1533215 - Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Sample description entry reports sample is encrypted, but no sinf was parsed!), at src/dom/media/mp4/Index.cpp:136

Status: RESOLVED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.mp4

Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Sample description entry reports sample is encrypted, but no sinf was parsed!), at src/dom/media/mp4/Index.cpp:136

24|0|libxul.so|mozilla::SampleIterator::GetNext()|hg:hg.mozilla.org/mozilla-central:dom/media/mp4/Index.cpp:4ab143dde4dc3424cfedc74b3648fbf2e47fb7bf|148|0x0
24|1|libxul.so|mozilla::MP4TrackDemuxer::GetNextSample()|hg:hg.mozilla.org/mozilla-central:dom/media/mp4/MP4Demuxer.cpp:4ab143dde4dc3424cfedc74b3648fbf2e47fb7bf|424|0x10
24|2|libxul.so|mozilla::MP4TrackDemuxer::GetSamples(int)|hg:hg.mozilla.org/mozilla-central:dom/media/mp4/MP4Demuxer.cpp:4ab143dde4dc3424cfedc74b3648fbf2e47fb7bf|499|0xb
24|3|libxul.so|mozilla::detail::ProxyFunctionRunnable<mozilla::MediaFormatReader::DemuxerProxy::Wrapper::GetSamples(int)::{lambda()#1}, mozilla::MozPromise<RefPtr<mozilla::MediaTrackDemuxer::SamplesHolder>, mozilla::MediaResult, true> >::Run()|hg:hg.mozilla.org/mozilla-central:dom/media/MediaFormatReader.cpp:4ab143dde4dc3424cfedc74b3648fbf2e47fb7bf|781|0xd
24|4|libxul.so|mozilla::TaskQueue::Runner::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/TaskQueue.cpp:4ab143dde4dc3424cfedc74b3648fbf2e47fb7bf|199|0x15
24|5|libxul.so|nsThreadPool::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadPool.cpp:4ab143dde4dc3424cfedc74b3648fbf2e47fb7bf|241|0x15
24|6|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:4ab143dde4dc3424cfedc74b3648fbf2e47fb7bf|1166|0x15
24|7|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:4ab143dde4dc3424cfedc74b3648fbf2e47fb7bf|482|0x11
24|8|libxul.so|mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:4ab143dde4dc3424cfedc74b3648fbf2e47fb7bf|333|0xd
24|9|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:4ab143dde4dc3424cfedc74b3648fbf2e47fb7bf|315|0x17
24|10|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:4ab143dde4dc3424cfedc74b3648fbf2e47fb7bf|308|0x8
24|11|libxul.so|nsThread::ThreadFunc(void*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:4ab143dde4dc3424cfedc74b3648fbf2e47fb7bf|453|0x8
24|12|libnspr4.so|_pt_root|hg:hg.mozilla.org/mozilla-central:nsprpub/pr/src/pthreads/ptthread.c:4ab143dde4dc3424cfedc74b3648fbf2e47fb7bf|201|0x7
24|13|libpthread-2.23.so||||0x76ba
24|14|libc-2.23.so||||0x10741d

Comment 1

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Bryce, would you mind taking the first pass triage here?

Comment 2

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

This looks like my doing due to recent reworking on mp4 crypto passing. I'm going to hold the NI to remind me to look at this and confirm the below. I suspect what is happening is that we the presence of various data determine if structures are encrypted. It is possible, though not sane, to have no sinf box (crypto metadata), but then send other crypto structures. In which case the assertion should be replaced by some error handling.

Comment 3

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8d3541789cf7896970044b731cbd658e29c458c8

Comment 4

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

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

Comment 5

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Try runs with

• Just crashtest
• Crashtest + fix

Comment 6

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: Bug 1533215 - Add crashtest for MP4 with encrypted track with bad sinf. r?jya

Add an mp4 with a mangled tenc box to the crashtests. This file has been fuzzed
and appears to have had the tenc box for its second track changed to a '.enc'
box, which means it cannot be parsed. The tenc box is contained in the sinf box,
so this will bust the sinf parse. We should tolerate such bad files and not
assert and/or crash.

Driveby add a comment to another line in crashtests list to make it clear the
bug the file originally relates to.

Comment 7

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: Bug 1533215 - Remove assert that can be triggered by malformed MP4. r?jya

The assert removed here can be triggered by malformed mp4s that lack certain
crypto info (the sinf box), but that still indicate samples are encrypted via
sample description entries.

I plan to follow up the removal here by adding logs so we have some way to
detect this case. This will be done as part of bug 1547328.

Depends on D29692

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee6dcf8e69d6
Add crashtest for MP4 with encrypted track with bad sinf. r=jya
https://hg.mozilla.org/integration/autoland/rev/8432594d00d5
Remove assert that can be triggered by malformed MP4. r=jya

Comment 9

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/ee6dcf8e69d6
https://hg.mozilla.org/mozilla-central/rev/8432594d00d5