Created attachment 256232 [details] [review]
Enabling I420

Shortening the pad definition and adding I420.

Created attachment 256238 [details] [review]
Enabling I420 (2)

Updated:
C-style comments, using variable name instead of N_PLANES macro.

Review of attachment 256238 [details] [review]:

::: sys/applemedia/vtenc.c
@@ +800,3 @@
+        pixel_format_type = kCVPixelFormatType_420YpCbCr8Planar;
+      } else {
+        pixel_format_type = kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange;

For future extensibility please check the video format here :)

Also is it guaranteed that both are always supported? If not it would make sense to implement something to only return what actually is supported from the CAPS query.

Created attachment 256251 [details] [review]
Enabling I420 (3)

(In reply to comment #3)
> Review of attachment 256238 [details] [review]:
> 
> ::: sys/applemedia/vtenc.c
> @@ +800,3 @@
> +        pixel_format_type = kCVPixelFormatType_420YpCbCr8Planar;
> +      } else {
> +        pixel_format_type = kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange;
> 
> For future extensibility please check the video format here :)

Makes sense :-)

> Also is it guaranteed that both are always supported? If not it would make
> sense to implement something to only return what actually is supported from the
> CAPS query.

I have tested this on an iMac and and a Macbook pro - on these Mac OS systems both always work. I don't have any info on whether these are always supported, for neither of the formats. I can try to investigate that separately.

(on iOS for example, that is)

Please do that, my guess would be that it converts internally if necessary.


commit b002490ab24ea6d28eab1d7c6f6af9ec42dece14
Author: Dominik Röttsches <dominik.rottsches@intel.com>
Date:   Wed Oct 2 05:49:43 2013 +0300

    vtenc: Add support for I420
    
    https://bugzilla.gnome.org/show_bug.cgi?id=709241

(In reply to comment #5)
> (on iOS for example, that is)

An iOS this is not public API though :)