Closed
Bug 1109861
Opened 9 years ago
Closed 9 years ago
[EME] Handle decrypt errors due to MediaKeySession close/remove
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: cpearce, Assigned: cpearce)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
31.79 KB,
patch
|
kinetik
:
review+
jwwang
:
feedback+
|
Details | Diff | Splinter Review |
I was getting test failures when closing MediaKeySessions because the CDM could have a pending decrypt operation, but run a session close/remove operation to drop keys, and then run the decrypt operation which would fail due to no longer having a key, and then we'd receive an error callback in the gecko process, and report that as an error to JS/HTML failing the mochitest. It's also a recoverable situation; the JS app may be able to re-request the key if it expired, or re-open the session. So I don't think we need to make this a fatal error. In the CDM-Decrypts case, we can handle the key becoming unusable by hanging onto the MP4Sample that failed to decrypt until the key becomes usable, or we flush/shutdown. In the CDM-decrypts-and-decodes case, I'm not sure what we can do, since we can't map the decode-failure to a specific frame. We can just ignore the error for now. The EME*Decoders won't send more samples until the keys are marked as usable again, and this should be a rare case, so I think just dropping the error for now is fine.
Assignee | ||
Comment 1•9 years ago
|
||
* Create a delegate to manage the process of waiting for a keyId to become usable inside the EME*Decoders so that we don't need to rewrite it again in every MediaDataDecoder interfacing with a GMP. * Report the GMPErr code if decrypting or decoding with a GMP fails, so we can handle GMPNoKeyErr. * Handle GMPNoKeyErr.
Attachment #8534595 -
Flags: review?(kinetik)
Attachment #8534595 -
Flags: feedback?(jwwang)
Assignee | ||
Updated•9 years ago
|
OS: Windows 8.1 → All
Hardware: x86_64 → All
Comment 2•9 years ago
|
||
Comment on attachment 8534595 [details] [diff] [review] Patch: Add delegate to manage waiting for the CDM to mark key usable. Review of attachment 8534595 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/eme/CDMCaps.cpp @@ +103,5 @@ > size_t i = 0; > while (i < waiters.Length()) { > auto& w = waiters[i]; > if (w.mKeyId == aKeyId) { > + waiters[i].mListener->NotifyUsable(aKeyId); s/waiters[i]/w/ ::: dom/media/fmp4/eme/EMEAudioDecoder.h @@ +115,4 @@ > Monitor mMonitor; > bool mFlushComplete; > + > + DebugOnly<bool> mIsShutdown; Use #ifdef DEBUG instead, DebugOnly members waste space in non-debug builds IIRC. ::: dom/media/fmp4/eme/EMEDecoderModule.cpp @@ +160,5 @@ > MediaDataDecoderCallback* mCallback; > nsRefPtr<MediaTaskQueue> mTaskQueue; > nsRefPtr<CDMProxy> mProxy; > + nsRefPtr<SamplesWaitingForKey> mSamplesWaitingForKey; > + DebugOnly<bool> mIsShutdown; Same deal with ifdef. ::: dom/media/fmp4/eme/EMEH264Decoder.h @@ +112,4 @@ > Monitor mMonitor; > bool mFlushComplete; > + > + DebugOnly<bool> mIsShutdown; Same deal with ifdef.
Attachment #8534595 -
Flags: review?(kinetik) → review+
Assignee | ||
Comment 3•9 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d7b01d38b023
Assignee | ||
Comment 4•9 years ago
|
||
WAE sucks: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=2c4ad1e5f82a
Comment 5•9 years ago
|
||
Btw, what is WAE?
Assignee | ||
Comment 6•9 years ago
|
||
Warnings As Errors, though in this case it was actually Unified Builds that broke my build... :P
Assignee | ||
Comment 7•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8dbf60547db0
Updated•9 years ago
|
Attachment #8534595 -
Flags: feedback?(jwwang) → feedback+
https://hg.mozilla.org/mozilla-central/rev/8dbf60547db0
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Chris, I am getting compilation error with your patch for firefox OS build as CDMCaps.h can not find SamplesWaitingForKey.h file. I filed a bug 1111033 to report the issue. Would you be able to help?
Flags: needinfo?(cpearce)
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to Anshul from comment #9) > Chris, I am getting compilation error with your patch for firefox OS build > as CDMCaps.h can not find SamplesWaitingForKey.h file. I filed a bug 1111033 > to report the issue. Would you be able to help? Followed up in bug 1111033...
Flags: needinfo?(cpearce)
Assignee | ||
Comment 11•9 years ago
|
||
Mass update firefox-status to track EME uplift.
You need to log in
before you can comment on or make changes to this bug.
Description
•