RESOLVED FIXED 189616
[EME] Add support the waitingforkey event
https://bugs.webkit.org/show_bug.cgi?id=189616
Summary [EME] Add support the waitingforkey event
Xabier Rodríguez Calvar
Reported 2018-09-14 01:34:30 PDT
[EME] Add support the waitingforkey event
Attachments
Patch (7.28 KB, patch)
2018-09-14 01:36 PDT, Xabier Rodríguez Calvar
no flags
Patch for landing (7.26 KB, patch)
2018-09-14 07:59 PDT, Xabier Rodríguez Calvar
no flags
Xabier Rodríguez Calvar
Comment 1 2018-09-14 01:36:48 PDT
Philippe Normand
Comment 2 2018-09-14 02:13:37 PDT
Comment on attachment 349751 [details] Patch Surely there are some layout tests than can be unskipped/unflagged after this?
Xabier Rodríguez Calvar
Comment 3 2018-09-14 03:12:50 PDT
(In reply to Philippe Normand from comment #2) > Comment on attachment 349751 [details] > Patch > > Surely there are some layout tests than can be unskipped/unflagged after > this? When I land the patch for the bug depending on this one, yes.
Philippe Normand
Comment 4 2018-09-14 03:30:42 PDT
I'm talking about platform-agnostic tests.
Xabier Rodríguez Calvar
Comment 5 2018-09-14 03:37:02 PDT
(In reply to Philippe Normand from comment #4) > I'm talking about platform-agnostic tests. Me too, but this patch only implements the crossplatform part that requires the platform one to work.
Philippe Normand
Comment 6 2018-09-14 05:31:04 PDT
Comment on attachment 349751 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349751&action=review > Source/WebCore/html/HTMLMediaElement.cpp:2845 > + // The platform already suspends and resumes playback > + // automagically. Can you elaborate?
Xabier Rodríguez Calvar
Comment 7 2018-09-14 06:37:35 PDT
(In reply to Philippe Normand from comment #6) > Comment on attachment 349751 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=349751&action=review > > > Source/WebCore/html/HTMLMediaElement.cpp:2845 > > + // The platform already suspends and resumes playback > > + // automagically. > > Can you elaborate? In GStreamer the decryptors block until they can continue decrypting and once they can, they go on without needing intervention, playback is "naturally suspended". I don't know about other platforms but I guess we can talk about it when the time comes.
Xabier Rodríguez Calvar
Comment 8 2018-09-14 07:59:33 PDT
Created attachment 349760 [details] Patch for landing
WebKit Commit Bot
Comment 9 2018-09-14 08:37:21 PDT
Comment on attachment 349760 [details] Patch for landing Clearing flags on attachment: 349760 Committed r236006: <https://trac.webkit.org/changeset/236006>
WebKit Commit Bot
Comment 10 2018-09-14 08:37:23 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 11 2018-09-14 08:38:57 PDT
Jer Noble
Comment 12 2018-09-18 15:29:15 PDT
This patch broke a number of our internal EME tests, and the Modern EME feature. Unless the client calls waitingForKey(), it will never receive an attemptToDecrypt(). There is an existing client callback, initializationDataEncountered(), that could have been used for the same purpose as waitingForKey(), or this patch could have added waitingForKey() everywhere initializationDataEncountered() was already called.
Xabier Rodríguez Calvar
Comment 13 2018-09-18 23:41:31 PDT
(In reply to Jer Noble from comment #12) > This patch broke a number of our internal EME tests, and the Modern EME > feature. Unless the client calls waitingForKey(), it will never receive an > attemptToDecrypt(). > > There is an existing client callback, initializationDataEncountered(), that > could have been used for the same purpose as waitingForKey(), or this patch > could have added waitingForKey() everywhere initializationDataEncountered() > was already called. Hi Jer, Oh! Sorry to read that this patch broke your internal tests. About the spec and the wait for key algorithms, it is clear for me that they are different things and can be triggered in different situations: * initializationDataEncountered() happens when you're playing (or about to) something and you get the initialization data from the stream that you have to report to JS but unless you cannot play anymore because you are already getting encrypted data, they are separate things. * waitingForKey(), according to the spec means that you cannot play anymore because you are blocked and it doesn't necessarily correspond with the moment you get the initialization data For me, the case of initialization data encountered is clear, when you see the data, you report it. The case of the wait for key algorithm has the following cases: [1] https://www.w3.org/TR/encrypted-media/#htmlmediaelement-extensions When one of the following occurs while the decryption blocked waiting for key value is true, the user agent shall run the Wait for Key algorithm. * The user agent cannot advance the current playback position in the direction of playback. * The user agent cannot provide data for the current playback position. [2] https://www.w3.org/TR/encrypted-media/#media-may-contain-encrypted-blocks The Media Data May Contain Encrypted Blocks algorithm pauses playback if the user agent requires specification of a MediaKeys object before playing the media data. Requests to run this algorithm include a target HTMLMediaElement object. ... 2. If the media element's mediaKeys attribute is null and the implementation requires specification of a MediaKeys object before decoding potentially-encrypted media data, run the following steps: [3] https://www.w3.org/TR/encrypted-media/#attempt-to-decrypt NOTE Once the user agent has rendered the blocks preceding the block that cannot be decrypted (as much as it can, such as, all complete video frames), it will run the Wait for Key algorithm. [4] https://www.w3.org/TR/encrypted-media/#wait-for-key The algorithm itself. [5] https://www.w3.org/TR/encrypted-media/#resume-playback Attempt to Resume Playback If Necessary From [2] I get that you could even run the wait for key algorithm even before finding initialization data if you require it, which we (GStreamer) don't. From [1] and [3] I get that you might get initialization data but this algorithm should be run when you can't render anything else because you're blocked waiting for decryption. For example, in our tests with YouTube and other streams we have seen that playback begins unencrypted, you get the initialization data and fire it to JS which can trigger all the process until being able to decrypt without being blocked because you are ready to decrypt in advance. Actually, the wait to decrypt is a symptom that you're too late because you have to suspend playback, right? It's true that [5] bails out in step 2 and I thought that it could create issue so I paid attention to Mac the bots when landing but I saw no breakage so I was quiet in the end.
Jer Noble
Comment 14 2018-09-19 09:42:42 PDT
(In reply to Xabier Rodríguez Calvar from comment #13) > (In reply to Jer Noble from comment #12) > > This patch broke a number of our internal EME tests, and the Modern EME > > feature. Unless the client calls waitingForKey(), it will never receive an > > attemptToDecrypt(). > > > > There is an existing client callback, initializationDataEncountered(), that > > could have been used for the same purpose as waitingForKey(), or this patch > > could have added waitingForKey() everywhere initializationDataEncountered() > > was already called. > > Hi Jer, > > Oh! Sorry to read that this patch broke your internal tests. Yeah, it's really unfortunate that we can't run any /actual/ EME tests as part of the n normal LayoutTest process; there's an external server requirement. I've got a background task of trying to figure out a way to get EME tests running in an automated and public way. > About the spec > and the wait for key algorithms, it is clear for me that they are different > things and can be triggered in different situations... [snip] Thanks for clarifying. We'll try to update our own platform implementation match the spec as closely as possible. That said, I was really referring to restoring the status-quo-ante for ports which have not yet implemented waitingForKey(). Bug #189720 should fix that for the macOS and iOS ports.
Note You need to log in before you can comment on or make changes to this bug.