1228461 - [EME] Implement "keyids" initData type for ClearKey

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

The EME spec has a "keyids" initData type which is used in ClearKey. We should support that so that we match the spec.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch 1: Don't assert we must have keys for keyIds we've seen in ~ClearKeySession()

In ~ClearKeySession() we assert that when we're releasing the ClearKeyDecryptionManager's ClearKeyDecryptor's that we have a key for the decryptor's we're releasing. That's not a valid assumption, as a license may not have been sent to the CDM, but the CDM may still have encountered initData causing us to expect the key, and thus create a ClearKeyDecryptor for it.

Without this change, we'll hit the assert in ~ClearKeySession if we create a clearkey MediaKeys, send it initData which contains keyIds, and refresh the page without sending a license to the CDM.

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch 2: Implement keyids initData type for ClearKey

* Plumb up "keyids" initDataType in Gecko and gmp-clearkey.
* Split ClearKeyUtils::ParseInitData() into CENC and KeyIds parts.
* Implement a JSON keyIds parser, similar to the existing JSON based parser for license data.
* Add mochitest, in which all the test cases pass in Chrome too (outside of mochitest).
* Remove spaces from our license-request message, to make the test easier to write.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f002dff8e709

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8692711 [details] [diff] [review]
Patch 2: Implement keyids initData type for ClearKey

Review of attachment 8692711 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with comments:

::: dom/media/eme/MediaKeySystemAccess.cpp
@@ +443,5 @@
>      for (const nsString& candidate : aCandidate.mInitDataTypes.Value()) {
>        if (candidate.EqualsLiteral("cenc")) {
>          initDataTypes.AppendElement(candidate);
>        }
> +      if (candidate.EqualsLiteral("keyids") && aKeySystem.EqualsLiteral("org.w3.clearkey")) {

Nit: I'd put an 'else' there, to show that this test and the previous one can't both be true.

@@ +444,5 @@
>        if (candidate.EqualsLiteral("cenc")) {
>          initDataTypes.AppendElement(candidate);
>        }
> +      if (candidate.EqualsLiteral("keyids") && aKeySystem.EqualsLiteral("org.w3.clearkey")) {
> +        initDataTypes.AppendElement( candidate);

Nit: Remove space before 'candidate'.

::: dom/media/test/test_eme_key_ids_initdata.html
@@ +84,5 @@
> +          session.addEventListener("message", function(event) {
> +            is(event.messageType, "license-request", "'" + test.name + "' MediaKeyMessage type should be license-request.");
> +            var text = new TextDecoder().decode(event.message);
> +            is(text, test.expectedRequest, "'" + test.name + "' got expected response.");
> +            is(text == test.expectedRequest, test.expectPass, "'" + test.name + "' expected to pass");

"expected to pass" is incorrect if test.expectPass is false, you could probably do: |"' expected to " + (test.expectPass ? "pass" : "fail")|.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/05c5396b7230
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec6611006fd0

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/05c5396b7230
https://hg.mozilla.org/mozilla-central/rev/ec6611006fd0