1316650 - 7,900 instances of "CDM returned NoKeyErr" emitted from dom/media/gmp/GMPCDMProxy.cpp during linux64 debug testing

Status: RESOLVED FIXED

(Core :: Audio/Video: GMP, defect, P2)

Description

[Eric Rahm [:erahm]]

> 7949 WARNING: CDM returned NoKeyErr: file dom/media/gmp/GMPCDMProxy.cpp, line 747

This warning [1] shows up in the following test suites:

>   5291 - desktop-test-linux64/debug-mochitest-media mda
>   2658 - desktop-test-linux64/debug-mochitest-media-e10s mda

It shows up in 2 tests. A few of the most prevalent:

>   5291 -        dom/media/test/test_eme_non_mse_fails.html
>   2658 - [e10s] dom/media/test/test_eme_non_mse_fails.html

[1] https://hg.mozilla.org/mozilla-central/annotate/02876b8cf2b0/dom/media/gmp/GMPCDMProxy.cpp#l747

Comment 1

[Eric Rahm [:erahm]]

This one seems intermittent, but when it shows up it's by far the #1 most verbose warning during testing. It's hard to bisect as the m-media test isn't run as often on inbound and it only shows up in one test.

edwin and cpearce it looks like you were both involved in adding the test, any idea what's going on here?

Comment 2

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Hmm.

This should only occur if a sample is reaching the CDM. ISTR adding a check to prevent this from happening in the non-fragmented case.

Those checks are gone now; we just need to re-add a guard before calls to SetCDMProxy in HTMLMediaElement to make sure that mMediaSource exists.

Although, I'm assuming that we know at that point whether we have fragmented media -- is that still the case?

Comment 3

[Eric Rahm [:erahm]]

This continues to be the most verbose warning during testing.

(In reply to Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin] from comment #2)
> Hmm.
> 
> This should only occur if a sample is reaching the CDM. ISTR adding a check
> to prevent this from happening in the non-fragmented case.
> 
> Those checks are gone now; we just need to re-add a guard before calls to
> SetCDMProxy in HTMLMediaElement to make sure that mMediaSource exists.
> 
> Although, I'm assuming that we know at that point whether we have fragmented
> media -- is that still the case?

I'm not sure who this question is directed to. cpearce, Edwin no longer appears to be active, do you have any ideas?

Comment 4

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

I've been working on other things.

I suspect what's happening here is that because we only stop the load (when we detect EME being used in non-MSE content) after we've reached HTMLMediaElement::MetadataLoaded(), we'll have a CDMProxy attached, and have started the decode pipeline. We should be not trying to decode samples for which we don't have a key for (the SamplesWaitingForKey class manages that). So I don't know why we are trying (and subsequently failing) to decode samples; we should be assuming all keys are not usable and thus not trying to decode. For some reason we must be thinking the keys with which the samples are encrypted are usable, and trying to decode.

Jay: Do you want to look into this? Bryce may be able to mentor you; we did some work around our "waitingforkey" code, which is close to this area.

Comment 5

[jay.harris]

So it looks like this issue is due to the way we deal with encryption on unfragmented videos. We only parse CENC data from the video if the video is fragmented (`Index::Index#243`) which means that when we check to see if we need to wait for a key (`SamplesWaitingForKey::WaitIfKeyNotUsable#31`), `aSample->mCrypto.mValid` is false, so we don't wait. Speaking with Chris, we have two possible solutions:

1. To read the CENC information on unfragmented videos, so our existing guards know not to play the video
2. Detect encryption in non-fragmented videos and fail more pro-actively (we don't support it at all). This might involve detecting encryption atoms in the parser and reporting a failure.

Chris has suggested that the second option is probably the better of the two.

Comment 6

[jay.harris]

Attachment: Bug 1316650 - Changes the NS_WARNING to an EME_LOG

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

Comment 7

[jay.harris]

As we're going to be replacing the parser in the near future, I've changed the NS_WARNING to an EME_LOG, so you shouldn't see the warnings unless you enable the log. This should naturally be fixed with the new parser.

Comment 8

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

Comment on attachment 8832222 [details]
Bug 1316650 - Changes the NS_WARNING to an EME_LOG

https://reviewboard.mozilla.org/r/108574/#review109728

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29cf209dcba4
Changes the NS_WARNING to an EME_LOG r=cpearce

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/29cf209dcba4