Closed Bug 1316650 Opened 5 years ago Closed 5 years ago
7,900 instances of "CDM returned No
Key Err" emitted from dom/media/gmp/GMPCDMProxy .cpp during linux64 debug testing
59 bytes, text/x-review-board-request
> 7949 WARNING: CDM returned NoKeyErr: file dom/media/gmp/GMPCDMProxy.cpp, line 747 This warning  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  https://hg.mozilla.org/mozilla-central/annotate/02876b8cf2b0/dom/media/gmp/GMPCDMProxy.cpp#l747
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?
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?
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?
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.
Flags: needinfo?(cpearce) → needinfo?(jharris)
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.
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 on attachment 8832222 [details] Bug 1316650 - Changes the NS_WARNING to an EME_LOG https://reviewboard.mozilla.org/r/108574/#review109728
Attachment #8832222 - Flags: review?(cpearce) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/29cf209dcba4 Changes the NS_WARNING to an EME_LOG r=cpearce
You need to log in before you can comment on or make changes to this bug.