1109987 - MSE: Crash in mp4 demuxer

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Jean-Yves Avenard [:jya]]

(lldb) bt
* thread #88: tid = 0x2412ea, 0x00000001035bb771 XUL`mp4_demuxer::Interval<long long>::Interval(this=0x000000013e286110, aStart=384307168180948, aEnd=0) + 113 at Interval.h:20, name = 'Media Decode #2', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x00000001035bb771 XUL`mp4_demuxer::Interval<long long>::Interval(this=0x000000013e286110, aStart=384307168180948, aEnd=0) + 113 at Interval.h:20
    frame #1: 0x00000001035b1115 XUL`mp4_demuxer::Interval<long long>::Interval(this=0x000000013e286110, aStart=384307168180948, aEnd=0) + 37 at Interval.h:21
  * frame #2: 0x0000000103598331 XUL`mp4_demuxer::Moof::ParseTrun(this=0x000000013e286420, aBox=0x000000013e286260, aTfhd=0x000000013e286298, aTfdt=0x000000013e286290, aMdhd=0x000000012216d928, aEdts=0x000000012216d968) + 1009 at MoofParser.cpp:213
    frame #3: 0x0000000103597e94 XUL`mp4_demuxer::Moof::ParseTraf(this=0x000000013e286420, aBox=0x000000013e286328, aTrex=0x000000012216d948, aMdhd=0x000000012216d928, aEdts=0x000000012216d968) + 388 at MoofParser.cpp:137
    frame #4: 0x0000000103597ccc XUL`mp4_demuxer::Moof::Moof(this=0x000000013e286420, aBox=0x000000013e286470, aTrex=0x000000012216d948, aMdhd=0x000000012216d928, aEdts=0x000000012216d968) + 236 at MoofParser.cpp:118
    frame #5: 0x0000000103597615 XUL`mp4_demuxer::Moof::Moof(this=0x000000013e286420, aBox=0x000000013e286470, aTrex=0x000000012216d948, aMdhd=0x000000012216d928, aEdts=0x000000012216d968) + 53 at MoofParser.cpp:121
    frame #6: 0x0000000103596c90 XUL`mp4_demuxer::MoofParser::RebuildFragmentedIndex(this=0x000000012216d900, aByteRanges=0x000000013e286558) + 320 at MoofParser.cpp:25
    frame #7: 0x0000000103596b46 XUL`mp4_demuxer::Index::UpdateMoofIndex(this=0x000000012a283b80, aByteRanges=0x000000013e286558) + 86 at Index.cpp:106
    frame #8: 0x0000000103599fad XUL`mp4_demuxer::MP4Demuxer::UpdateIndex(this=0x0000000121155660, aByteRanges=0x000000013e286558) + 141 at mp4_demuxer.cpp:203
    frame #9: 0x000000010625d07c XUL`mozilla::MP4Reader::UpdateIndex(this=0x0000000121607800) + 220 at MP4Reader.cpp:862
    frame #10: 0x000000010625e8d9 XUL`mozilla::MP4Reader::GetBuffered(this=0x0000000121607800, aBuffered=0x000000010040f400) + 105 at MP4Reader.cpp:884
    frame #11: 0x0000000106175d39 XUL`mozilla::SourceBufferDecoder::GetBuffered(this=0x000000012bbdcc40, aBuffered=0x000000010040f400) + 73 at SourceBufferDecoder.cpp:228
    frame #12: 0x0000000106175b75 XUL`mozilla::MediaSourceReader::SelectReader(this=0x000000012b9c4c00, aTarget=0, aTrackDecoders=0x000000011fb42438) + 245 at MediaSourceReader.cpp:300
    frame #13: 0x000000010617450d XUL`mozilla::MediaSourceReader::SwitchAudioReader(this=0x000000012b9c4c00, aTarget=0) + 157 at MediaSourceReader.cpp:321
    frame #14: 0x000000010617443f XUL`mozilla::MediaSourceReader::RequestAudioData(this=0x000000012b9c4c00) + 207 at MediaSourceReader.cpp:107
    frame #15: 0x000000010608ff35 XUL`mozilla::MediaDecoderStateMachine::DecodeAudio(this=0x000000012b9c9000) + 405 at MediaDecoderStateMachine.cpp:669
    frame #16: 0x00000001060e1b4a XUL`nsRunnableMethodImpl<void (this=0x000000012fcfd070)(), void, true>::Run() + 154 at nsThreadUtils.h:388
    frame #17: 0x00000001060bb6d9 XUL`mozilla::MediaTaskQueue::Runner::Run(this=0x0000000121618340) + 633 at MediaTaskQueue.cpp:230
    frame #18: 0x00000001036f5b27 XUL`nsThreadPool::Run(this=0x000000012bbc8cc0) + 967 at nsThreadPool.cpp:220
    frame #19: 0x00000001036f5c1c XUL`non-virtual thunk to nsThreadPool::Run(this=0x000000012bbc8cc8) + 28 at nsThreadPool.cpp:234
    frame #20: 0x00000001036f2588 XUL`nsThread::ProcessNextEvent(this=0x000000011e24de30, aMayWait=false, aResult=0x000000013e286c4e) + 2088 at nsThread.cpp:830
    frame #21: 0x000000010374a8e7 XUL`NS_ProcessNextEvent(aThread=0x000000011e24de30, aMayWait=false) + 151 at nsThreadUtils.cpp:265
    frame #22: 0x0000000103d76b43 XUL`mozilla::ipc::MessagePumpForNonMainThreads::Run(this=0x0000000129865240, aDelegate=0x000000013e642c90) + 691 at MessagePump.cpp:339
    frame #23: 0x0000000103d0a6b5 XUL`MessageLoop::RunInternal(this=0x000000013e642c90) + 117 at message_loop.cc:233
    frame #24: 0x0000000103d0a5c5 XUL`MessageLoop::RunHandler(this=0x000000013e642c90) + 21 at message_loop.cc:226
    frame #25: 0x0000000103d0a56d XUL`MessageLoop::Run(this=0x000000013e642c90) + 45 at message_loop.cc:200
    frame #26: 0x00000001036f0a16 XUL`nsThread::ThreadFunc(aArg=0x000000011e24de30) + 358 at nsThread.cpp:350
    frame #27: 0x0000000103371cff libnss3.dylib`_pt_root(arg=0x000000011fb42c60) + 463 at ptthread.c:212
    frame #28: 0x00007fff9b3022fc libsystem_pthread.dylib`_pthread_body + 131
    frame #29: 0x00007fff9b302279 libsystem_pthread.dylib`_pthread_start + 176
    frame #30: 0x00007fff9b3004b1 libsystem_pthread.dylib`thread_start + 13

Comment 1

[Jean-Yves Avenard [:jya]]

Crash is in Moof::ParseTrun
    sample.mCompositionRange = Interval<Microseconds>(
      aMdhd.ToMicroseconds(decodeTime + ctsOffset - aEdts.mMediaStart),
      aMdhd.ToMicroseconds(decodeTime + ctsOffset + sampleDuration - aEdts.mMediaStart));

here decodeTime = ctsOffset = 0, while aEdts.MediaStart = 1024
so decoderTime + ctsOffset - aEdts.MediaStart = 	384307168180948 (as Microseconds is a uint64_t)

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Ensure pts never goes below 0

Attempted fix. Not sure duration should be adjusted. This ensure however that the presentation time range is accurate. In that particular example, it gives the first frame a duration of 0

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: Use signed timestamps in mp4 demuxer

Use signed timestamps. Seems much better that way

Comment 4

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Comment on attachment 8534825 [details] [diff] [review]
Use signed timestamps in mp4 demuxer

Review of attachment 8534825 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/libstagefright/binding/MoofParser.cpp
@@ +185,5 @@
>  
>    uint64_t offset = aTfhd.mBaseDataOffset + (flags & 1 ? reader->ReadU32() : 0);
>    bool hasFirstSampleFlags = flags & 4;
>    uint32_t firstSampleFlags = hasFirstSampleFlags ? reader->ReadU32() : 0;
> +  int64_t decodeTime = aTfdt.mBaseMediaDecodeTime;

We still want this to be uint64_t but we have to make the conversion to microseconds signed.

@@ +323,5 @@
>    BoxReader reader(aBox);
>    uint32_t flags = reader->ReadU32();
>    uint8_t version = flags >> 24;
>    if (version == 0) {
> +    mBaseMediaDecodeTime = reader->Read32();

ISO/IEC 14496-12:2008/FDAM 

The spec defines this as unsigned int(32) baseMediaDecodeTime;

::: media/libstagefright/binding/include/mp4_demuxer/MoofParser.h
@@ +43,5 @@
>    {
>    }
>    explicit Mdhd(Box& aBox);
>  
> +  Microseconds ToMicroseconds(int64_t aTimescaleUnits)

I think this is the only change we want.

Comment 5

[Jean-Yves Avenard [:jya]]

Attachment: Use signed timestamps in mp4 demuxer

Use signed timestamps. Try #2

Comment 6

[Jean-Yves Avenard [:jya]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/78c1c54dd021

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/78c1c54dd021

Comment 8

[Ralph Giles (:rillian)]

Comment on attachment 8535301 [details] [diff] [review]
Use signed timestamps in mp4 demuxer

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Less consistent testing of video features, more likely to receive flash video.
[Describe test coverage new/current, TBPL]: Landed on m-c.
[Risks and why]: Low; this affects only MSE-specific code.
[String/UUID change made/needed]: None.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/fd499fac7d8b