1109861 - [EME] Handle decrypt errors due to MediaKeySession close/remove

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

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

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.

Comment 1

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

Attachment: Patch: Add delegate to manage waiting for the CDM to mark key usable.

* 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.

Comment 2

[Matthew Gregan [:kinetik]]

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.

Comment 3

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

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d7b01d38b023

Comment 4

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

WAE sucks:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=2c4ad1e5f82a

Comment 5

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

Btw, what is WAE?

Comment 6

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

Warnings As Errors, though in this case it was actually Unified Builds that broke my build... :P

Comment 7

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/8dbf60547db0

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 9

[Anshul]

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?

Comment 10

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

(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...

Comment 11

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

Mass update firefox-status to track EME uplift.