Not sure if this is just an OOM or not. Please let me know if not and I will unhide the bug. This can take a few seconds to crashes. About 30 seconds on Linux mozglue!mozalloc_abort+0x2a [c:\mozilla-central\memory\mozalloc\mozalloc_abort.cpp @ 33] mozglue!mozalloc_handle_oom+0x61 [c:\mozilla-central\memory\mozalloc\mozalloc_oom.cpp @ 46] mozglue!moz_xrealloc+0x24 [c:\mozilla-central\memory\mozalloc\mozalloc.cpp @ 134] xul!nsTArrayInfallibleAllocator::Realloc+0xd [c:\mozilla-central\objdir-ff\dist\include\nstarray.h @ 182] xul!nsTArray_base<nsTArrayInfallibleAllocator,nsTArray_CopyWithMemutils>::EnsureCapacity<nsTArrayInfallibleAllocator>+0xb9 [c:\mozilla-central\objdir-ff\dist\include\nstarray-inl.h @ 183] xul!nsTArray_Impl<signed char,nsTArrayInfallibleAllocator>::AppendElement<signed char &,nsTArrayInfallibleAllocator>+0x19 [c:\mozilla-central\objdir-ff\dist\include\nstarray.h @ 2038] xul!mp4_demuxer::Saiz::Saiz+0x118 [c:\mozilla-central\media\libstagefright\binding\moofparser.cpp @ 856] xul!mp4_demuxer::Moof::ParseTraf+0x113 [c:\mozilla-central\media\libstagefright\binding\moofparser.cpp @ 490] xul!mp4_demuxer::Moof::Moof+0xda [c:\mozilla-central\media\libstagefright\binding\moofparser.cpp @ 367] xul!mp4_demuxer::MoofParser::RebuildFragmentedIndex+0xe6 [c:\mozilla-central\media\libstagefright\binding\moofparser.cpp @ 51] xul!mp4_demuxer::MoofParser::RebuildFragmentedIndex+0x25 [c:\mozilla-central\media\libstagefright\binding\moofparser.cpp @ 35] xul!mozilla::MP4TrackDemuxer::EnsureUpToDateIndex+0x49 [c:\mozilla-central\dom\media\fmp4\mp4demuxer.cpp @ 270] xul!mozilla::MP4TrackDemuxer::MP4TrackDemuxer+0xb4 [c:\mozilla-central\dom\media\fmp4\mp4demuxer.cpp @ 240] xul!mozilla::MP4Demuxer::GetTrackDemuxer+0xae [c:\mozilla-central\dom\media\fmp4\mp4demuxer.cpp @ 172] xul!mozilla::MediaFormatReader::OnDemuxerInitDone+0xc1 [c:\mozilla-central\dom\media\mediaformatreader.cpp @ 291] xul!mozilla::MozPromise<enum nsresult,enum mozilla::DemuxerFailureReason,1>::InvokeCallbackMethod+0xb [c:\mozilla-central\objdir-ff\dist\include\mozilla\mozpromise.h @ 457] xul!mozilla::MozPromise<enum nsresult,enum mozilla::DemuxerFailureReason,1>::MethodThenValue<mozilla::MediaFormatReader,void (__thiscall mozilla::MediaFormatReader::*)(enum nsresult),void (__thiscall mozilla::MediaFormatReader::*)(enum mozilla::DemuxerFailureReason)>::DoResolveOrRejectInternal+0x15 [c:\mozilla-central\objdir-ff\dist\include\mozilla\mozpromise.h @ 508] xul!mozilla::MozPromise<enum nsresult,enum mozilla::DemuxerFailureReason,1>::ThenValueBase::DoResolveOrReject+0x7c [c:\mozilla-central\objdir-ff\dist\include\mozilla\mozpromise.h @ 407] xul!mozilla::MozPromise<enum nsresult,enum mozilla::DemuxerFailureReason,1>::ThenValueBase::ResolveOrRejectRunnable::Run+0x6a [c:\mozilla-central\objdir-ff\dist\include\mozilla\mozpromise.h @ 324] xul!mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run+0x53 [c:\mozilla-central\objdir-ff\dist\include\mozilla\taskdispatcher.h @ 195] xul!mozilla::TaskQueue::Runner::Run+0x85 [c:\mozilla-central\xpcom\threads\taskqueue.cpp @ 173] xul!nsThreadPool::Run+0x2f3 [c:\mozilla-central\xpcom\threads\nsthreadpool.cpp @ 229] xul!nsThread::ProcessNextEvent+0x233 [c:\mozilla-central\xpcom\threads\nsthread.cpp @ 1058] xul!NS_ProcessNextEvent+0x26 [c:\mozilla-central\xpcom\glue\nsthreadutils.cpp @ 290] xul!mozilla::ipc::MessagePumpForNonMainThreads::Run+0xee [c:\mozilla-central\ipc\glue\messagepump.cpp @ 338] xul!MessageLoop::RunInternal+0x8 [c:\mozilla-central\ipc\chromium\src\base\message_loop.cc @ 232] xul!MessageLoop::RunHandler+0x53 [c:\mozilla-central\ipc\chromium\src\base\message_loop.cc @ 226] xul!MessageLoop::Run+0x19 [c:\mozilla-central\ipc\chromium\src\base\message_loop.cc @ 206] xul!nsThread::ThreadFunc+0xb4 [c:\mozilla-central\xpcom\threads\nsthread.cpp @ 461] nss3!_PR_NativeRunThread+0xc1 [c:\mozilla-central\nsprpub\pr\src\threads\combined\pruthr.c @ 419] nss3!pr_root+0xb [c:\mozilla-central\nsprpub\pr\src\md\windows\w95thred.c @ 95] ucrtbase!_crt_at_quick_exit+0x104 KERNEL32!BaseThreadInitThunk+0x24 ntdll!__RtlUserThreadStart+0x2f ntdll!_RtlUserThreadStart+0x1b

Another variation

Similar to bug 1296473, this is causing an OOM by trying to allocate billions of bytes (adding the same byte one at a time to an infallible array, that's why it takes a little while to happen). The 2nd log is similar, but the first big allocation worked and it's a later one (based on the first set of numbers) that failed. Unhiding bug as it's not a security threat.

Anthony, is it possible that this is related to the black-screen issues we've been seeing with Facebook Live video streams on Windows (which use MP4 containers). If so I'd suggest a higher prioritization.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #4) > Anthony, is it possible that this is related to the black-screen issues > we've been seeing with Facebook Live video streams on Windows (which use MP4 > containers). If so I'd suggest a higher prioritization. The bug here happens for ill-formatted videos (with impossibly-high number-of-samples), and results in an OOM crash from an infallible allocation. So I doubt it would "only" cause black-screen issues. In any case I'm already working on it (it was a low-hanging fruit for me), here's a WIP try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2dee6b668ba9042700a332e3eaa3d6bb7ed8fc36 I'm hoping to get it reviewed & pushed in the next day or so. We'll see then if that helps with Facebook issues...

(In reply to Benjamin Smedberg [:bsmedberg] from comment #4) > Anthony, is it possible that this is related to the black-screen issues > we've been seeing with Facebook Live video streams on Windows (which use MP4 > containers). If so I'd suggest a higher prioritization. No. The black video issues are caused by Flash.

Comment on attachment 8785058 [details] Bug 1296532 - Optionally allow crypto in stagefright gtest - https://reviewboard.mozilla.org/r/74392/#review72212

Comment on attachment 8785059 [details] Bug 1296532 - Fix MoofParser tests - https://reviewboard.mozilla.org/r/74394/#review72214

Comment on attachment 8785060 [details] Bug 1296532 - Added test case to stagefright gtest - https://reviewboard.mozilla.org/r/74396/#review72216

Comment on attachment 8785061 [details] Bug 1296532 - Use fallible arrays in MoofParser - https://reviewboard.mozilla.org/r/74398/#review72218 ::: media/libstagefright/binding/MoofParser.cpp:547 (Diff revision 1) > LOG(Moof, "Incomplete Box (missing sampleCount)"); > return false; > } > uint32_t sampleCount = reader->ReadU32(); > if (sampleCount == 0) { > + reader->DiscardRemaining(); This is starting to get messy/bloated. Please extract the AutoByteReader https://dxr.mozilla.org/mozilla-central/source/dom/media/flac/FlacFrameParser.cpp?q=AutoByteReader&redirect_type=direct#16 And place it in ByteReader.h. You've already started modifying ByteReader.h with utility class, mays as well continue so you can remove all the DiscardRemaining ::: media/libstagefright/binding/MoofParser.cpp:912 (Diff revision 1) > mAuxInfoType = reader->ReadU32(); > mAuxInfoTypeParameter = reader->ReadU32(); > } > size_t count = reader->ReadU32(); > need = (version ? sizeof(uint64_t) : sizeof(uint32_t)) * count; > - if (reader->Remaining() < count) { > + if (reader->Remaining() < need) { I would be anal enough to say that this should be on a bug on its own and uplifted. as I think it's the most serious problem of all as it could occur with valid files (the others typically won't) ::: media/libstagefright/binding/include/mp4_demuxer/MoofParser.h (Diff revision 1) > MediaByteRange FirstCompleteMediaHeader(); > > mozilla::MediaByteRange mInitRange; > RefPtr<Stream> mSource; > uint64_t mOffset; > - nsTArray<uint64_t> mMoofOffsets; this is out of scope of this bug. Fly by fixes need their own patch

Comment on attachment 8785061 [details] Bug 1296532 - Use fallible arrays in MoofParser - https://reviewboard.mozilla.org/r/74398/#review72226 This needs to be split up. It's good overall, so not a real r-, but I'd like to see the followup fixes thank you

Comment on attachment 8785061 [details] Bug 1296532 - Use fallible arrays in MoofParser - https://reviewboard.mozilla.org/r/74398/#review72218 > This is starting to get messy/bloated. > > Please extract the AutoByteReader https://dxr.mozilla.org/mozilla-central/source/dom/media/flac/FlacFrameParser.cpp?q=AutoByteReader&redirect_type=direct#16 > > And place it in ByteReader.h. You've already started modifying ByteReader.h with utility class, mays as well continue > > so you can remove all the DiscardRemaining Good idea, thanks. I'll do that in a separate bug. > I would be anal enough to say that this should be on a bug on its own and uplifted. > as I think it's the most serious problem of all as it could occur with valid files (the others typically won't) I'll open a new bug for that. > this is out of scope of this bug. > > Fly by fixes need their own patch I won't bother opening a new bug for this, I think it's too trivial to merit all the extra paperwork. I suppose it can stay until we remove stagefright completely one day.

Comment on attachment 8785061 [details] Bug 1296532 - Use fallible arrays in MoofParser - https://reviewboard.mozilla.org/r/74398/#review72268 ::: media/libstagefright/binding/MoofParser.cpp (Diff revision 2) > > // Sometimes audio streams don't properly mark their samples as keyframes, > // because every audio sample is a keyframe. > sample.mSync = !(sampleFlags & 0x1010000) || aIsAudio; > > - // FIXME: Make this infallible after bug 968520 is done. technically, that comment is still valid. The allocation with SetCapacity was made fallible, we know AppendElement will always be true.

Comment on attachment 8785061 [details] Bug 1296532 - Use fallible arrays in MoofParser - https://reviewboard.mozilla.org/r/74398/#review72268 Thank you for the review. > technically, that comment is still valid. > > The allocation with SetCapacity was made fallible, we know AppendElement will always be true. Bug 968520 has landed in 43, and I didn't see an obvious way to do an infallible AppendElement on a FallibleTArray, that's why I removed the comment. But I'll put it back, for someone else to deal with it another time...

Pushed by gsquelart@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8c218f4870cf Optionally allow crypto in stagefright gtest - r=jya https://hg.mozilla.org/integration/autoland/rev/4552565382c0 Fix MoofParser tests - r=jya https://hg.mozilla.org/integration/autoland/rev/328a5b31fa52 Added test case to stagefright gtest - r=jya https://hg.mozilla.org/integration/autoland/rev/19b4bf02c0bd Use fallible arrays in MoofParser - r=jya

https://hg.mozilla.org/mozilla-central/rev/8c218f4870cf https://hg.mozilla.org/mozilla-central/rev/4552565382c0 https://hg.mozilla.org/mozilla-central/rev/328a5b31fa52 https://hg.mozilla.org/mozilla-central/rev/19b4bf02c0bd