Bug 189951 – MediaPlayer should have mediaPlayerWaitingForKeyChanged() / bool waitingForKey() accessor

• Component: Media
• Status: RESOLVED FIXED

[Jer Noble]

Currently, HTMLMediaElement assumes that the MediaPlayer is no longer waiting for a key after attemptToDecrypt() is called. It elides the "If the user agent can advance the current playback position in the direction of playback:" step.

MediaPlayer should provide a "bool waitingForKey() const" accessor so that HTMLMediaElement can query whether it's still waiting for a key after attemptToDecrypt(), as well as a mediaPlayerWaitingForKeyChanged() client notification when this value changes. This would replace the existing mediaPlayerWaitingForKey() notification.

[Jer Noble]

I'll post a patch making this change, but I'll wait till I hear back from Igalia people before landing, as this will definitely have the potential to break ports.

[Jer Noble]

I'll post a patch making this change, but I'll wait till I hear back from Igalia people before landing, as this will definitely have the potential to break ports.

[Philippe Normand]

Thanks Jer.

[Jer Noble]

Created attachment 350766 [details]
Patch

[Xabier Rodríguez Calvar]

I'd appreciate if you can wait until Monday since I am going on holidays.

[Eric Carlson]

Comment on attachment 350766 [details]
Patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:2807
> +        setReadyState(m_player->readyState());

Nit: this should be a noop because the player always updates readyState when it changes.

[Xabier Rodríguez Calvar]

Comment on attachment 350766 [details]
Patch

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

>> Source/WebCore/html/HTMLMediaElement.cpp:2807
>> +        setReadyState(m_player->readyState());
> 
> Nit: this should be a noop because the player always updates readyState when it changes.

I am not sure of this. IIRC, when I wrote the first patch I updated the readyState in the element without syncing it to the player, so this would force it to be reset to what the player had.

> Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp:311
> +            gst_element_post_message(GST_ELEMENT(self), gst_message_new_element(GST_OBJECT(self), gst_structure_new_empty("drm-key-received")));

Wow, I am still amazed by how you solved this and your ability to send GStreamer events in one side and catch them at the other :) . First I thought this couldn't work but then I realized that it actually can, at least on paper and no (more) tests are broken. First I thought that now that we track if we can play or not, you should remove the code below this that resends the waitingforkey event. Then I realized that we are not tracking that properly because you only consider one decryptor being blocked and you can have more than one. Then I realized that the code below would fix that as it would resend the waitingforkey event.

I also thought this wouldn't work because the attempt to decrypt is not connected but our decryptors are quite autonomous and if they are waiting, they will carry on once they get the key, which will put things in motion again.

This said, I made an "interesting" interpretation of the ability to continue playback, which is something I can't know unless I query the all decryptors so I assume yes and let the algorithm stop again once I "can't continue decrypting things on the encrypted blocks awaiting decryption list". I guess your approach is better but I'll keep that as a fix me for the future.

r=me on the GStreamer part as no tests are broken and solution seems sensible. I don't know if Phil has some more comments on this.

PS: You can still come to the hackfest, btw! :)

[Philippe Normand]

No concerns from my side. A last iteration seems needed though to turn EWS green.

[Jer Noble]

Created attachment 350871 [details]
Patch for landing

[Jer Noble]

Created attachment 350874 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 350874 [details]
Patch for landing

Clearing flags on attachment: 350874

Committed r236572: <https://trac.webkit.org/changeset/236572>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44844394>