Closed
Bug 1301061
Opened 8 years ago
Closed 8 years ago
Background video decode will cause issue when use with EME
Categories
(Core :: Audio/Video: Playback, defect, P3)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
INVALID
People
(Reporter: jya, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
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 hidden (mozreview-request) |
Comment 2•8 years ago
|
||
mozreview-review |
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
Attachment #8788903 -
Flags: review?(kaku)
Comment hidden (mozreview-request) |
Reporter | ||
Updated•8 years ago
|
Attachment #8790100 -
Flags: review?(cpearce)
Reporter | ||
Comment 4•8 years ago
|
||
mozreview-review |
Comment on attachment 8790100 [details]
Bug 1301061: Ensure EME events are propagated when decoding is suspended.
https://reviewboard.mozilla.org/r/78056/#review76508
LGTM
Attachment #8790100 -
Flags: review?(jyavenard) → review+
Updated•8 years ago
|
Priority: -- → P3
Comment 5•8 years ago
|
||
@jya, I am going to take a long PTO, so, could you help to land this bug if :cpearce also approve the patch?
Flags: needinfo?(jyavenard)
Comment 6•8 years ago
|
||
mozreview-review |
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.
Attachment #8790100 -
Flags: review?(cpearce) → review-
Comment 7•8 years ago
|
||
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•8 years ago
|
||
You mean "waitingforkey" events? The "encrypted" events are supposed to be fired by the MediaFormatReader/demuxers.
Reporter | ||
Comment 10•8 years ago
|
||
(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.
Flags: needinfo?(jyavenard)
Reporter | ||
Updated•8 years ago
|
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•