[Webkit-unassigned] [Bug 181990] [EME][GStreamer] Add support for the encrypted Caps in GStreamerUtilities

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=181990

Charlie Turner <cturner at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cturner at igalia.com

--- Comment #5 from Charlie Turner <cturner at igalia.com> ---
Comment on attachment 332388
  --> https://bugs.webkit.org/attachment.cgi?id=332388
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=332388&action=review

> Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:63
> +        if (!areVideoCaps(caps))

Can this check not be hoisted above the areEncryptedCaps? It's a valid pre-condition check for the whole function it seems. I know gst_video_info_from_caps does it's own validation in addition to this, but having it guarded by areEncryptedCaps doesn't immediately make sense to me.

> Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:76
> +

blanks lines before and after else, there's more in the patch, please remove.

> Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:98
> +    gint pixelAspectRatioNumerator, pixelAspectRatioDenominator;

I would assign these to 1 like in the original code, if the gst_structure_get_fraction call fails for some weird reason, we could get undefined behaviour in the height calculation.

> Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:101
> +        if (!areVideoCaps(caps))

Ditto for pre-condition check

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180202/ec0adc36/attachment.html>