RESOLVED FIXED191459
[GStreamer][EME] waitingforkey event should consider decryptors' waiting status
https://bugs.webkit.org/show_bug.cgi?id=191459
Summary [GStreamer][EME] waitingforkey event should consider decryptors' waiting status
Xabier Rodríguez Calvar
Reported 2018-11-09 01:40:51 PST
[GStreamer][EME] waitingforkey event should consider decryptors' waiting status
Attachments
Patch (12.98 KB, patch)
2018-11-09 01:52 PST, Xabier Rodríguez Calvar
no flags
Patch (14.72 KB, patch)
2018-11-09 04:40 PST, Xabier Rodríguez Calvar
no flags
Patch (13.06 KB, patch)
2018-11-12 03:25 PST, Xabier Rodríguez Calvar
no flags
Patch (13.05 KB, patch)
2018-11-12 03:32 PST, Xabier Rodríguez Calvar
no flags
Xabier Rodríguez Calvar
Comment 1 2018-11-09 01:52:26 PST
Philippe Normand
Comment 2 2018-11-09 02:26:32 PST
Comment on attachment 354321 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354321&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:1385 > + bool result { false }; > + if (m_pipeline) { Nit: Strange way to assign a variable? Also this condition could be transformed to an early return. > Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:345 > + return FALSE; No chaining to parent class?
Xabier Rodríguez Calvar
Comment 3 2018-11-09 02:52:11 PST
(In reply to Philippe Normand from comment #2) > Comment on attachment 354321 [details] Yep to all of it. Thx! Fixing the EWS as well. Weird that I am not getting that build failure myself...
Charlie Turner
Comment 4 2018-11-09 03:30:42 PST
Comment on attachment 354321 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354321&action=review Nice to get rid of those handlers in the decryptor sink handler :) I guess imported/w3c/web-platform-tests/encrypted-media/clearkey-mp4-waiting-for-a-key.https.html is still blocked on the event loop issue, just wondered if this affect the expectations of that test at all. > Source/WebCore/ChangeLog:10 > + going with the decryptors because events are reported only once going on with > Source/WebCore/ChangeLog:22 > + Query the pipeline, just a query after after pipeline is built and after after > Source/WebCore/ChangeLog:23 > + manual inspection during build. The optimal would be just the The query is optimal, but sometimes ...
Xabier Rodríguez Calvar
Comment 5 2018-11-09 04:40:14 PST
Charlie Turner
Comment 6 2018-11-12 01:58:57 PST
Informal r+ from me FWIW.
Xabier Rodríguez Calvar
Comment 7 2018-11-12 03:25:31 PST
Created attachment 354541 [details] Patch I fixed the concerns raised in this patch and remove the custom API for the iterator codepath as we can still use a query there.
Xabier Rodríguez Calvar
Comment 8 2018-11-12 03:32:44 PST
Created attachment 354542 [details] Patch Removed the WTF:: from boolForPrinting
Xabier Rodríguez Calvar
Comment 9 2018-11-12 04:19:15 PST
Phil, as you're on holidays, I asked Carlos to r+ this considering that I solved your concerns.
WebKit Commit Bot
Comment 10 2018-11-12 04:40:20 PST
Comment on attachment 354542 [details] Patch Clearing flags on attachment: 354542 Committed r238083: <https://trac.webkit.org/changeset/238083>
WebKit Commit Bot
Comment 11 2018-11-12 04:40:22 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 12 2018-11-12 04:41:46 PST
Note You need to log in before you can comment on or make changes to this bug.