Bug 190471

Summary: [GStreamer] Fix EME build for GStreamer 1.14.x
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: PlatformAssignee: Philippe Normand <pnormand>
Status: RESOLVED FIXED    
Severity: Normal CC: bandou.yacine, calvaris, tsaunier, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch calvaris: review+

Philippe Normand
Reported 2018-10-11 08:30:28 PDT
.
Attachments
Patch (2.18 KB, patch)
2018-10-11 08:35 PDT, Philippe Normand 		no flags
DetailsFormatted DiffDiff
Patch (2.34 KB, patch)
2018-10-11 09:05 PDT, Philippe Normand 		no flags
DetailsFormatted DiffDiff
Patch (4.77 KB, patch)
2018-10-12 10:00 PDT, Philippe Normand 		calvaris: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Philippe Normand
Comment 1 2018-10-11 08:35:07 PDT
Created attachment 352046 [details] Patch
Yacine Bandou
Comment 2 2018-10-11 08:58:40 PDT
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
Comment 3 2018-10-11 09:05:27 PDT
Created attachment 352047 [details] Patch
Thibault Saunier
Comment 4 2018-10-11 13:19:36 PDT
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 5 2018-10-11 13:23:38 PDT
Comment on attachment 352047 [details] Patch Oops. Will update the patch. Thanks!
Philippe Normand
Comment 6 2018-10-12 10:00:29 PDT
Created attachment 352177 [details] Patch
Thibault Saunier
Comment 7 2018-10-12 10:07:10 PDT
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 8 2018-10-14 22:17:11 PDT
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
Comment 9 2018-10-15 04:44:49 PDT
Committed r237092: <https://trac.webkit.org/changeset/237092>
Radar WebKit Bug Importer
Comment 10 2018-10-15 04:45:42 PDT
<rdar://problem/45268472>
Thibault Saunier
Comment 11 2018-10-15 04:49:57 PDT
I am afraid that will actually make the patches that backported that feature in jhbuild useless.
Philippe Normand
Comment 12 2018-10-15 04:59:22 PDT
(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.
Note You need to log in before you can comment on or make changes to this bug.