1355252 - [EME] Consider configurations passed to navigator.requestMediaKeySystemAccess() without both audioCapabilities and videoCapabilities as unsupported

Status: RESOLVED WONTFIX

(Core :: Audio/Video: Playback, enhancement, P3)

Description

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

The EME spec requires us to consider configurations passed to navigator.requestMediaKeySystemAccess() that don't specify at least one of either audioCapabilites or videoCapabilities as being not supported.

Step 15 of this algorithm:
https://w3c.github.io/encrypted-media/#get-supported-configuration-and-consent

Chrome are implementing this:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/aG_QGiPErlE

I left a TODO in the code to remind me that we need to come back and add this:

https://dxr.mozilla.org/mozilla-central/rev/45692c884fdd5136a64fb2f8a61a0c8183b69331/dom/media/eme/MediaKeySystemAccess.cpp#941

Comment 1

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

We also shouldn't be assuming that WebM and MP4 normatively implies a specific set of codecs and codec constraints, as that violates step 10 of the "Get Supported Capabilities for Audio/Video Type" algorithm. So we should fix this branch:

https://dxr.mozilla.org/mozilla-central/rev/45692c884fdd5136a64fb2f8a61a0c8183b69331/dom/media/eme/MediaKeySystemAccess.cpp#664

Comment 2

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

Blake: Do you have someone who can look at this too? :)

Comment 3

[Blake Wu [:bwu][:blakewu]]

Alfredo，
Can you help on this?

Comment 4

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

I'll take this as discussed with Alfredo.

Comment 5

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

Attachment: Bug 1355252 - Don't assume default codecs for MP4 and WebM in EME MediaCapabilities

Step 10 of EME's "Get Supported Capabilities for Audio/Video Type" algorithm
says we can assume default codecs only if a container normatively implies a
specific set of codec and codec constraints. Our code assumes that WebM implies
Vorbis/VP8 and MP4 implies AAC/H.264, but those aren't actually normatively
required by either of these containers' specifications. So we shouldn't assume
these containers imply those codecs.

Review commit: https://reviewboard.mozilla.org/r/139978/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139978/

Comment 6

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

Attachment: Bug 1355252 - Reject navigator.requestMediaKeySystemAccess if no MediaCapabilities supplied.

As required by step 15 of EME's Get Supported Configuration and Consent algorithm,
https://www.w3.org/TR/encrypted-media/#get-supported-configuration-and-consent

Review commit: https://reviewboard.mozilla.org/r/139980/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139980/

Comment 7

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

Comment on attachment 8868389 [details]
Bug 1355252 - Don't assume default codecs for MP4 and WebM in EME MediaCapabilities

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139978/diff/1-2/

Comment 8

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

Comment on attachment 8868390 [details]
Bug 1355252 - Reject navigator.requestMediaKeySystemAccess if no MediaCapabilities supplied.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139980/diff/1-2/

Comment 9

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8868390 [details]
Bug 1355252 - Reject navigator.requestMediaKeySystemAccess if no MediaCapabilities supplied.

https://reviewboard.mozilla.org/r/139980/#review143796

Comment 10

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8868389 [details]
Bug 1355252 - Don't assume default codecs for MP4 and WebM in EME MediaCapabilities

https://reviewboard.mozilla.org/r/139978/#review143798

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c19d32e2a710
Don't assume default codecs for MP4 and WebM in EME MediaCapabilities r=jwwang
https://hg.mozilla.org/integration/autoland/rev/ddb3969de046
Reject navigator.requestMediaKeySystemAccess if no MediaCapabilities supplied. r=jwwang

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c19d32e2a710
https://hg.mozilla.org/mozilla-central/rev/ddb3969de046

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a95271d1180
Backed out changeset ddb3969de046. r=backout
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f1c4b1bac37
Backed out changeset c19d32e2a710. r=backout

Comment 14

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

This broke Netflix and potentially other sites, so I backed it out.

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8a95271d1180

Comment 16

[James Cheng[:JamesCheng]]

https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-10-24&keys=__none__!__none__!__none__&max_channel_version=release%252F56&measure=MEDIA_EME_REQUEST_DEPRECATED_WARNINGS&min_channel_version=beta%252F54&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-09-26&table=0&trim=1&use_submission_date=0

2017/09/21 to 2017/11/12
Nightly 58 ==> 18.16K 

2017/09/25 to 2017/11/12
Beta 57    ==> 427.61K

2017/09/26 to 2017/10/24
Release 56 ==> 443.77K


One-day samples
2017/11/10
Beta 58    ==> 14
2017/11/13
Nightly 59 ==> 37
No data in Release 57


I don't know how to test if this patch impact on the Netflix or other side.

I apply the patch and run Netflix, Shaka Demo Player and Amazon, those work just fine on my Linux.

Hi cpearce,

How did you test comment 14, or Netflix just change their script to fulfill the spec requirement?

And by the telemetry, there are still around 400K users so I don't think I should remove it right now...

But what do you think how low the sample should be that I can land this patch, or any other criteria we should meet?

Thanks.

Comment 17

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

(In reply to James Cheng[:JamesCheng] from comment #16)
> How did you test comment 14, or Netflix just change their script to fulfill
> the spec requirement?

I ran with the patch applied, and checked whether Netflix worked. So it sounds like you've tested it.

> And by the telemetry, there are still around 400K users so I don't think I
> should remove it right now...
> 
> But what do you think how low the sample should be that I can land this
> patch, or any other criteria we should meet?

This is a good question. How about we wait for the MediaEMENoCodecsDeprecatedWarning level to be at or below 0.5% before landing this?

Comment 18

[Daniel Bodea [:danibodea]]

I have seen a similar issue reproduce and I am not sure if this is actually the same or not, so I will need some feedback.

This is what I observed:

1. Reached the https://view.yahoo.com/ and opened a video.
   For example:
   https://view.yahoo.com/show/chicago-med/episode/61295024/we-hold-these-truths

Expected:
An error is displayed, notifying the user that the video can only be streamed within the United States. (For locations other than US)
OR
The video is played correctly. (For locations inside the US)

Actual:
The whole player appears as a black area. No video, no sound, no video controls are being displayed. The rest of the page is displayed correctly.

Error displayed in Browser Console:
MediaKeySystemAccess::GetKeySystemStatus(com.widevine.alpha) result=cdm-not-supported msg=''
navigator.requestMediaKeySystemAccess promise rejected 0x80530009 ''
navigator.requestMediaKeySystemAccess promise rejected 0x80530009 'Key system is unsupported'

NOTE: This issue ONLY occurs on the aarch64 builds (Nightly v67.0a1 from 2019-02-20 and Beta v66.0b10) on Lenovo Yoga, ARM laptop. In the case of the non-aarch64 build (tested on win32 non-aarch build on Lenovo Yoga, ARM laptop) notification appears displayed in the player area and the following error is displayed in Browser Console:
MediaKeySystemAccess::GetKeySystemStatus(com.widevine.alpha) result=available msg=''

Do you think this issue is not likely to be included/related to bug 1515208?
Is THIS bug not fixed? Or is it a new issue?

Thank you.

Comment 19

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

Widevine isn't working on aarch64 yet, so what you're seeing should be fixed once bug 1515208 and its dependencies are resolved.

Comment 20

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

We had telemetry on this, but it expired in 63.

From the telemetry data from beta 62, we can see that ~4% of EME requests would have been rejected if we conformed to the spec here:

https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2018-08-30&include_spill=0&keys=__none__!__none__!__none__&max_channel_version=beta%252F62&measure=MEDIA_EME_REQUEST_DEPRECATED_WARNINGS&min_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_by_value=0&sort_keys=submissions&start_date=2018-06-25&table=0&trim=1&use_submission_date=0

Bryce: I recommend you re-enable the MEDIA_EME_REQUEST_DEPRECATED_WARNINGS and monitor it, and make the change suggested here once sites stop requesting KeySystemAccess without any capabilities as frequently. Above I suggested dropping support once fewer than 0.5% of all requests are non-conformant to the spec. It's probably also worth checking up on what Chrome is doing here.

Comment 21

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Bug 1356046 removed the telemetry involved above. I'll restore it so we can reason about this change. Holding NI.

Comment 22

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

I had some WIP to restore the telemetry around this (bug 1672213) , and then planned to finish this out, but am afraid it fell on the back burner. Passing NI.