If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[EME] Implement Navigator.requestMediaKeySystemAccess()

RESOLVED FIXED in Firefox 37

Status

()

Core
Audio/Video
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: cpearce, Assigned: cpearce)

Tracking

(Blocks: 2 bugs, {dev-doc-needed})

unspecified
mozilla36
dev-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox37 fixed, firefox38 fixed, firefox39 fixed)

Details

(URL)

Attachments

(2 attachments, 2 obsolete attachments)

The way to create a MediaKeys object has changed recently; now we're supposed to create it using these:

partial interface Navigator {
    Promise<MediaKeySystemAccess> requestMediaKeySystemAccess (DOMString keySystem, optional sequence<MediaKeySystemOptions> supportedConfigurations);
};

interface MediaKeySystemAccess {
    readonly    attribute DOMString keySystem;
    Promise<MediaKeys> createMediaKeys ();
};

dictionary MediaKeySystemOptions {
    DOMString            initDataType = "";
    DOMString            audioType = "";
    DOMString            audioCapability = "";
    DOMString            videoType = "";
    DOMString            videoCapability = "";
    MediaKeysRequirement uniqueidentifier = "optional";
    MediaKeysRequirement stateful = "optional";
};

enum MediaKeysRequirement {
    "required",
    "optional",
    "disallowed"
};
Created attachment 8521013 [details] [diff] [review]
Patch

Remove MediaKeys.create and MediaKeys.isTypeSupported, and add navigator.requestMediaKeySystemAccess() as per current draft spec.

The capability checks are only wired up for clearkey, since we haven't discussed with Adobe what capability strings they want to expose.

Note: the members of MediaKeySystemOptions are still in flux, so there will likely be some changes to the members of MediaKeySystemOptions in future.
Attachment #8521013 - Flags: review?(edwin)
Attachment #8521013 - Flags: review?(bzbarsky)
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=06fd931f2d06
Comment on attachment 8521013 [details] [diff] [review]
Patch

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

Sweet. Couple small nits.

::: dom/media/fmp4/MP4Decoder.cpp
@@ +105,5 @@
>  MP4Decoder::CanHandleMediaType(const nsACString& aType,
> +                               const nsAString& aCodecs,
> +                               bool& aOutContainsAAC,
> +                               bool& aOutContainsH264,
> +                               bool& aOutContainsMP3)

:(

I have no suggestion here; just... :(

::: dom/media/test/eme.js
@@ +199,5 @@
> +      }
> +    ];
> +    navigator.requestMediaKeySystemAccess(KEYSYSTEM_TYPE, options).then(
> +      function(keySystemAccess) {
> +        keySystemAccess.createMediaKeys().then(function(mediaKeys) {

nit: |return keySystemAccess.createMediaKeys();| and move this .then chain to the end of requestMediaKeySystemAccess.

::: dom/media/test/test_eme_requestKeySystemAccess.html
@@ +34,5 @@
> +      });
> +  });
> +}
> +
> +const clearkey = 'org.w3.clearkey';

nit: CLEARKEY_ID or something like that. Something uppercase.
Attachment #8521013 - Flags: review?(edwin) → review+
Created attachment 8521729 [details] [diff] [review]
Patch v2

With Edwin's review comments addressed... and with `hg add dom/webidl/MediaKeySystemAccess.webidl`...
Attachment #8521013 - Attachment is obsolete: true
Attachment #8521013 - Flags: review?(bzbarsky)
Attachment #8521729 - Flags: review?(bzbarsky)
This bug is necessary for basic playback of EME video on Windows.
Blocks: 1098156
Comment on attachment 8521729 [details] [diff] [review]
Patch v2

>+++ b/dom/media/eme/MediaKeySystemAccess.cpp
>+MediaKeySystemAccess::IsSupported(const nsAString& aKeySystem,

Why does this not depend on initDataType, uniqueidentifier, stateful?  At leat document why those don't matter here, please.

>+++ b/dom/media/eme/MediaKeySystemAccess.h
>+struct JSContext;

Including js/TypeDecls.h might be better.

I didn't review the tests very carefully.

r=me.  Sorry for the lag.  :(
Attachment #8521729 - Flags: review?(bzbarsky) → review+
(In reply to Please do not ask for reviews for a bit [:bz] from comment #6)
> Comment on attachment 8521729 [details] [diff] [review]
> Patch v2
> 
> >+++ b/dom/media/eme/MediaKeySystemAccess.cpp
> >+MediaKeySystemAccess::IsSupported(const nsAString& aKeySystem,
> 
> Why does this not depend on initDataType, uniqueidentifier, stateful?  At
> leat document why those don't matter here, please.

MediaKeySystemAccess::IsSupported() checks options.mInitDataType. 

Our sandbox provides an origin specific uniqueidentifier, and the ability to persist data. We don't yet have a way to turn those off and on for specific GMPs/CDMs, and I don't know if we want to, so I just left that unimplemented.

I'll add a comment.
https://hg.mozilla.org/integration/mozilla-inbound/rev/54f79521758b
Backed out again due to m3 failures:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ee39194ac67

4175 INFO TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_interfaces.html | If this is failing: DANGER, are you sure you want to expose the new interface MediaKeySystemAccess to all webpages as a property on the window (XBL: false)? Do not make a change to this file without a review from a DOM peer for that specific change!!! (or a JS peer for changes to ecmaGlobals) - expected PASS

I bet this is because I forgot the pref=media.eme.enabled on MediaKeySystemAccess.
Er, yes, exactly.  Sorry, I should have caught that.  :(
But that adding that pref annotation to MediaKeySystemAccess didn't fix the error, I still see the error:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=61d2569643a4
Because navigator.requestMediaKeySystemAccess is still visible somehow...
Ah, I need to add MediaKeySystemAccess to interfaceNamesInGlobalScope in dom/tests/mochitest/general/test_interfaces.html for the test to pass... And I'm told repeatedly that I need DOM Peer review to change that.
Created attachment 8524388 [details] [diff] [review]
Patch v3

Added Pref=media.eme.enabled to MediaKeySystemAccess.
Attachment #8521729 - Attachment is obsolete: true
Attachment #8524388 - Flags: review+
Created attachment 8524389 [details] [diff] [review]
Patch: Add MediaKeySystemAccess to DOM interfaceNamesInGlobalScope test

MediaKeySystemAccess needs to be in interfaceNamesInGlobalScope for dom/tests/mochitest/general/test_interfaces.html to pass.

It says repeatedly that I need DOM peer review to change that list... So I guess I'd better do that...
Attachment #8524389 - Flags: review+
Thanks for the review peterv!

I rolled the test_interfaces.html fix into the main patch and re-landed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/7fa6291d952d
sory chris had to back this out since we are seeing test failures like https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3984801&repo=mozilla-inbound starting with this push and  releng investigations found no change in the infrastructure that could explain that failures for a infrastructure reason.
Flags: needinfo?(cpearce)
Chris, is the pref set during the test run or something?
Ah, yes, it is.  See testing/profiles/prefs_general.js
(In reply to Carsten Book [:Tomcat] from comment #17)
> sory chris had to back this out since we are seeing test failures like
> https://treeherder.mozilla.org/ui/logviewer.
> html#?job_id=3984801&repo=mozilla-inbound starting with this push and 
> releng investigations found no change in the infrastructure that could
> explain that failures for a infrastructure reason.

I still see this test failure happening in revisions after my push, and I don't see how my patch can have caused it.
Flags: needinfo?(cpearce)
Discussed with KWierso on IRC, and he OK'd me to reland:

https://hg.mozilla.org/integration/mozilla-inbound/rev/40711e8b703d

We're hitting bug 1101133.
Keywords: dev-doc-needed
https://hg.mozilla.org/mozilla-central/rev/40711e8b703d
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
No longer blocks: 1098156
Mass update firefox-status to track EME uplift.
status-firefox37: --- → fixed
status-firefox38: --- → fixed
status-firefox39: --- → fixed
You need to log in before you can comment on or make changes to this bug.