1301061 - Background video decode will cause issue when use with EME

Status: RESOLVED INVALID

(Core :: Audio/Video: Playback, defect, P3)

Description

[Jean-Yves Avenard [:jya]]

When a video goes into the background, its decoding is suspended.

This is done by substituting the current decoder with a BlankDecoder.

However, this replacement is done regardless of EME being used.

If video is encrypted, then the blankdecoder must be wrapped with the EME PDM

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1301061: Ensure EME events are propagated when decoding is suspended.

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

Comment 2

[Tzuhao Kuo [:kaku]]

Comment on attachment 8788903 [details]
Bug 1301061: Ensure EME events are propagated when decoding is suspended.

https://reviewboard.mozilla.org/r/77228/#review75794

Thanks :jw for clarifying the EME and GMP concept for me!

I think that while using EMEPDM to create a video decoder for an invisible video element, we should not use the GMP decoder but use a EMEDecryptor comparing with a BlankVideoDecoder. In specific, we should check | if (SupportsMimeType(aParams.mConfig.mMimeType, nullptr) && !aParams.mUseBlankDecoder) | here: http://searchfox.org/mozilla-central/source/dom/media/platforms/agnostic/eme/EMEDecoderModule.cpp#246

Comment 3

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1301061: Ensure EME events are propagated when decoding is suspended.

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

Comment 4

[Jean-Yves Avenard [:jya]]

Comment on attachment 8790100 [details]
Bug 1301061: Ensure EME events are propagated when decoding is suspended.

https://reviewboard.mozilla.org/r/78056/#review76508

LGTM

Comment 5

[Tzuhao Kuo [:kaku]]

@jya, I am going to take a long PTO, so, could you help to land this bug if :cpearce also approve the patch?

Comment 6

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

Comment on attachment 8790100 [details]
Bug 1301061: Ensure EME events are propagated when decoding is suspended.

https://reviewboard.mozilla.org/r/78056/#review77960

The CDM can operate in two modes; decrypt mode or decrypt-and-decode mode. In decrypt mode, the CDM decrypts and returns decrypted samples to Gecko for Gecko to decode with non-robust decoders. In decrypt-and-decode mode, the CDM decrypts and decodes with its own decoders, and returns uncompressed samples back to Gecko to render.

What you're doing here means that if the decoder is suspended due to being in a background tab, we'll switch from doing decrypt-and-decode in the CDM to doing decrypt only in the CDM with insecure decode in Gecko. This will likely violate the CDM's robustness rules, and the CDM is unlikely to cooperate with us switching to non-robust decode mid-stream. Indeed I built your patch and found that Netflix fails when the decoder is shutdown this way; decryption returns a "no key" error.

So we can't take this patch as-is.

Interestingly, Nightly seems to work fine without your patch. I suspect that what's happening is that since we're creating a blank decoder, we're not trying to decrypt the media samples, and so we don't trigger the robustness rules. Why can't we keep using the blank decoder unwrapped? That seems to work, and means we won't trip the CDM's robustness rules.

Comment 7

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

IIUC, the reason for comment 0 is that the blank decoder doesn't decrypt at all and will not fire encrypt events correctly. However, come to think about it again, it should fire encrypt events anyway when switching back to the gmp decoder.

Comment 8

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

You mean "waitingforkey" events? The "encrypted" events are supposed to be fired by the MediaFormatReader/demuxers.

Comment 9

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

So what's our concern in comment 0?

Comment 10

[Jean-Yves Avenard [:jya]]

(In reply to JW Wang [:jwwang] [:jw_wang] from comment #9)
> So what's our concern in comment 0?

maybe there aren't any...

better safe than sorry.