Bug 190471 – [GStreamer] Fix EME build for GStreamer 1.14.x

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Philippe Normand]

.

[Philippe Normand]

Created attachment 352046 [details]
Patch

[Yacine Bandou]

Comment on attachment 352046 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:168
>          }

If it is necessary to add the flag, I suggest like this:

            gst_structure_set_name(outgoingStructure.get(),
#if GST_CHECK_VERSION(1, 15, 0)
                !g_strcmp0(klass->protectionSystemId, GST_PROTECTION_UNSPECIFIED_SYSTEM_ID) ? "application/x-webm-enc" : "application/x-cenc");
#else
                "application/x-cenc");
#endif

[Philippe Normand]

Created attachment 352047 [details]
Patch

[Thibault Saunier]

Comment on attachment 352047 [details]
Patch

There is another occurence of `GST_PROTECTION_UNSPECIFIED_SYSTEM_ID` in MediaPlayerPrivateGStreamerBase.cpp - I guess you missed a part of your patch.

[Philippe Normand]

Comment on attachment 352047 [details]
Patch

Oops. Will update the patch. Thanks!

[Philippe Normand]

Created attachment 352177 [details]
Patch

[Thibault Saunier]

Comment on attachment 352177 [details]
Patch

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

Lgtm, though I believe that code should be reworked :-)

Informal r+

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:1305
> +        if (eventKeySystemUUID == GST_PROTECTION_UNSPECIFIED_SYSTEM_ID)

I know this patch doesn't change the semantics but the assumption that `GST_PROTECTION_UNSPECIFIED_SYSTEM_ID` implies `webm` sounds dangerous/not future proof.

[Xabier Rodríguez Calvar]

Comment on attachment 352177 [details]
Patch

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

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:1305
>> +        if (eventKeySystemUUID == GST_PROTECTION_UNSPECIFIED_SYSTEM_ID)
> 
> I know this patch doesn't change the semantics but the assumption that `GST_PROTECTION_UNSPECIFIED_SYSTEM_ID` implies `webm` sounds dangerous/not future proof.

I agree and I already commented it on the original bug. I'll buy it for the moment as in cenc the sys id is mandatory and we only suppory either webm or cenc.

[Philippe Normand]

Committed r237092: <https://trac.webkit.org/changeset/237092>

[Radar WebKit Bug Importer]

<rdar://problem/45268472>

[Thibault Saunier]

I am afraid that will actually make the patches that backported that feature in jhbuild useless.

[Philippe Normand]

(In reply to Thibault Saunier from comment #11)
> I am afraid that will actually make the patches that backported that feature
> in jhbuild useless.

Yes you're right. See also https://bugs.webkit.org/show_bug.cgi?id=189238#c9 and following comments.