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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37
Tracking Status
firefox37 --- fixed
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
* 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)
OS: Windows 8.1 → All
Hardware: x86_64 → All
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+
Btw, what is WAE?
Warnings As Errors, though in this case it was actually Unified Builds that broke my build... :P
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)
(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)
Mass update firefox-status to track EME uplift.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: