Closed Bug 1307019 Opened 3 years ago Closed 3 years ago

Crash in mozilla::EMEDecoderModule::SupportsMimeType

Categories

(Core :: Audio/Video: Playback, defect, P1, critical)

x86
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox50 --- fixed
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

(Blocks 1 open bug)

Details

(Keywords: crash)

Crash Data

Attachments

(3 files)

Attached file missing-pssh.zip
This bug was filed from the Socorro interface and is 
report bp-c37f65e1-7a6f-472c-89a3-b2af22161003.
=============================================================

Crash with attached testcase, which tries to play an encrypted file which doesn't have a PSSH without first negotiating a ClearKey license.

We're doing a null pointer deref.
Assignee: nobody → cpearce
Turns out this was fixed by bug 1300069. Given that I was trying to write a test case for bug 1300069, so I guess I've succeeded!
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1300069
Not quite true. This is actually a regression from bug 1300069, which seems to be timing dependent. The patch in that bug introduced an inconsistency between what the MediaDecoderStateMachine and the MediaFormatReader consider an encrypted stream. The MDSM considered a stream encrypted if mInfo.IsEncrypted() is true, and that only takes into account the PSSH. Whereas the MFR only considers the presence of a TENC box to indicate encryptedness. This would cause the MDSM to not wait for the CDM before trying to start decoding. So if you setup the MediaSource before setting the MediaKeys on the MediaElement, you'll hit this crash.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Comment on attachment 8797030 [details]
Bug 1307019 - Ensure MDSM and MFR have consistent view of what counts as encrypted.

https://reviewboard.mozilla.org/r/82666/#review81376
Attachment #8797030 - Flags: review?(jyavenard) → review+
Comment on attachment 8797031 [details]
Bug 1307019 - Testcase for encrypted MP4 without PSSH and MDSM waiting-for-CDM.

https://reviewboard.mozilla.org/r/82668/#review81378

::: dom/media/test/test_eme_missing_pssh.html:75
(Diff revision 1)
> +          sourceBuffer = mediaSource.addSourceBuffer(type);
> +          var req = new XMLHttpRequest();
> +          req.open("GET", url);
> +          req.responseType = "arraybuffer";
> +          req.addEventListener("load", function() {
> +            sourceBuffer.addEventListener("updateend", resolve);

You should use a promise (like with the once utility found in mediasource.js) here rather than an event listnener.. or at least remove the event handler as the later call to endOfStream will cause updateend to be fired again.

otherwise you may get intermittent errors due to an invalid state

::: dom/media/test/test_eme_missing_pssh.html:84
(Diff revision 1)
> +        });
> +      }
> +
> +      function LoadMSE() {
> +        var ms = new MediaSource();
> +        audio.src = URL.createObjectURL(ms);

You could have made this mochitest must simpler if it was possible to share some of the scripts found in the mediasource/test directory.
Attachment #8797031 - Flags: review?(jyavenard) → review+
(In reply to Jean-Yves Avenard [:jya] from comment #7)
> Comment on attachment 8797031 [details]
> Bug 1307019 - Testcase for encrypted MP4 without PSSH and MDSM
> waiting-for-CDM.
> [...]
> You could have made this mochitest must simpler if it was possible to share
> some of the scripts found in the mediasource/test directory.

I manually imported once() (with a minor change to pass the event argument to the callback) and fetchWithXHR() into eme.js. Using the dom/media/mediasource/test/mediasource.js script wasn't working, or at least, it isn't obvious how to make it work from the normal dom/media/test directory.
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/61d63bbd649f
Ensure MDSM and MFR have consistent view of what counts as encrypted. r=jya
https://hg.mozilla.org/integration/autoland/rev/8f201cbda968
Testcase for encrypted MP4 without PSSH and MDSM waiting-for-CDM. r=jya
Must remember to uplift...
Flags: needinfo?(cpearce)
https://hg.mozilla.org/mozilla-central/rev/61d63bbd649f
https://hg.mozilla.org/mozilla-central/rev/8f201cbda968
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment on attachment 8797030 [details]
Bug 1307019 - Ensure MDSM and MFR have consistent view of what counts as encrypted.

Approval Request Comment
[Feature/regressing bug #]: bug 1300069, EME Video Playback
[User impact if declined]: Crashes if sites setup streaming sources before setting up EME. I missed a case in my patch for bug 1300069, and we crash in that case. :(
[Describe test coverage new/current, TreeHerder]: Testcase included in next patch in this bug. That testcase also covers bug 1300069 which was uplifted a few days ago.
[Risks and why]: Low, fairly simple fix, with testcase.
[String/UUID change made/needed]: None.
Flags: needinfo?(cpearce)
Attachment #8797030 - Flags: approval-mozilla-beta?
Attachment #8797030 - Flags: approval-mozilla-aurora?
Comment on attachment 8797031 [details]
Bug 1307019 - Testcase for encrypted MP4 without PSSH and MDSM waiting-for-CDM.

Approval Request Comment
[Feature/regressing bug #]: bug 1300069, EME Video Playback
[User impact if declined]: Test coverage for crash fix, and bug 1300069.
[Describe test coverage new/current, TreeHerder]: This is the testcase.
[Risks and why]: Test, so should be no user risk!
[String/UUID change made/needed]: None.
Attachment #8797031 - Flags: approval-mozilla-beta?
Attachment #8797031 - Flags: approval-mozilla-aurora?
Comment on attachment 8797030 [details]
Bug 1307019 - Ensure MDSM and MFR have consistent view of what counts as encrypted.

Fix a crash, taking it
Shoud be in 50 beta 5
Attachment #8797030 - Flags: approval-mozilla-beta?
Attachment #8797030 - Flags: approval-mozilla-beta+
Attachment #8797030 - Flags: approval-mozilla-aurora?
Attachment #8797030 - Flags: approval-mozilla-aurora+
Attachment #8797031 - Flags: approval-mozilla-beta?
Attachment #8797031 - Flags: approval-mozilla-beta+
Attachment #8797031 - Flags: approval-mozilla-aurora?
Attachment #8797031 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.