Crash in mozilla::MP4Demuxer::NotifyDataArrived

RESOLVED FIXED in Firefox 54

Status

()

defect
P1
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: marcia, Assigned: jya)

Tracking

(Blocks 1 bug, {crash, regression})

55 Branch
mozilla55
Unspecified
Linux
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox53 unaffected, firefox54+ fixed, firefox55+ fixed)

Details

(crash signature)

Attachments

(2 attachments, 5 obsolete attachments)

This bug was filed from the Socorro interface and is 
report bp-631c3f32-056d-4d31-a121-d40c92170417.
=============================================================

Seen while looking at nightly crash stats - earliest crashes seem to be in 20170331030216: http://bit.ly/2ppvgx2. Crashes affect Win, Linux and Android on nightly but the majority affect Linux. This crash is also present on Aurora.

ni on :gerald since it appears he worked in this area of code and might have some ideas.
Flags: needinfo?(gsquelart)

Comment 1

2 years ago
Bughunter can reproduce this on an NSFW url on Linux. It appears to be a null ptr access. Crashes opt and asserts in debug Nightly.

Assertion failure: mRawPtr != nullptr (You can't dereference a NULL RefPtr with operator->().), at /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/RefPtr.h:315
#01: mozilla::MP4Demuxer::NotifyDataArrived [dom/media/fmp4/MP4Demuxer.cpp:624]
#02: mozilla::detail::ProxyFunctionRunnable<mozilla::MediaFormatReader::DemuxerProxy::NotifyDataArrived()::<lambda()>, mozilla::MozPromise<bool, mozilla::MediaResult, true> >::Run [dom/media/MediaFormatReader.cpp:1060]
#03: mozilla::TaskQueue::Runner::Run [xpcom/threads/TaskQueue.cpp:232]
#04: nsThreadPool::Run [xpcom/threads/nsThreadPool.cpp:226]
#05: nsThread::ProcessNextEvent [mfbt/Maybe.h:445]
#06: NS_ProcessNextEvent [xpcom/threads/nsThreadUtils.cpp:389]
#07: mozilla::ipc::MessagePumpForNonMainThreads::Run [ipc/glue/MessagePump.cpp:339]
#08: MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:239]
#09: MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:505]
#10: nsThread::ThreadFunc [xpcom/threads/nsThread.cpp:503]
#11: _pt_root [nsprpub/pr/src/pthreads/ptthread.c:219]
#12: libpthread.so.0 + 0x76ba
#13: libc.so.6 + 0x10682d
#14: ??? (???:???)
ExceptionHandler::GenerateDump cloned child 16003
ExceptionHandler::SendContinueSignalToChild sent continue signal to child
ExceptionHandler::WaitForContinueSignal waiting for continue signal...

[15940] WARNING: Decoder=7f89ae029c00 state=DECODING_METADATA Decode metadata failed, shutting down decoder: file /home/worker/workspace/build/src/dom/media/MediaDecoderStateMachine.cpp, line 405

This is basically

<video  id='...' class='crt-video crt-skin-default' poster='...' preload='none' muted data-crt-video data-crt-options='{"autoheight":null,"duration":95,"hdUrl":false,"filmstrip":{"url":"...","width":"200","height":"112"}}' ><source src="..." type="video/mp4"></video>

[15940] WARNING: Decoder=7f89ae029c00 Decode error: file /home/worker/workspace/build/src/dom/media/MediaDecoderStateMachine.cpp, line 3399
Blocks: 532972
There are older reports, but not as frequent before early March.
(And the spike on 11 April is happening on only 2 installs.)

There are similar (but fewer) crashes in WebMBufferedState::NotifyDataArrived, so maybe the issue is at a higher level.

I don't have useful ideas just yet; in past experiences it has been some race between shutdown (killing the demuxer) and new data arriving into that demuxer.

But Jean-Yves would know best about all this, and/or possibly JW?

(I'll have a deeper look if I find the time...)
Flags: needinfo?(jyavenard)
Flags: needinfo?(jwwang)

Comment 3

2 years ago
it's crashing in a codepath that was last touched in bug 1341454.
Blocks: 1341454
Keywords: regression
Priority: -- → P1
Assign to blake to find owner.
Assignee: nobody → bwu
http://searchfox.org/mozilla-central/rev/313e5199bf58200f158c6fcbe193e41b88ed58a6/dom/media/MediaFormatReader.cpp#2899

The |!mDemuxerInitDone && !mDemuxerInitRequest.Exists()| check is suspicious to me. I think it should be just |!mDemuxerInitDone|.

Hi Jya,
Do you have any idea about the check?
Flags: needinfo?(jwwang)
Assignee

Comment 6

2 years ago
it does indeed look suspicious...

I can't recall why it was done that way
Flags: needinfo?(jyavenard)
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Assignee: bwu → jyavenard
Thank you for looking into this, Jean-Yves.
Flags: needinfo?(gsquelart)

Comment 9

2 years ago
mozreview-review
Comment on attachment 8861408 [details]
Bug 1357040: P1. Abort if demuxer initialisation isn't complete.

https://reviewboard.mozilla.org/r/133400/#review136546
Attachment #8861408 - Flags: review?(jwwang) → review+

Comment 10

2 years ago
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a5cb0de6bdb2
Abort if demuxer initialisation isn't complete. r=jwwang
See Also: → 1360092
Assignee

Comment 12

2 years ago
so it turned out that NotifyDataArrived is required for the MediaSourceDemuxer to resolve its promise, hence why we had the |!mDemuxerInitDone && !mDemuxerInitRequest.Exists()|

So it's definitely due to bug 1341454, the track demuxers are no longer created as it used to...
Flags: needinfo?(jyavenard)
Assignee

Comment 13

2 years ago
A tad puzzled with the stack trace.

So we attempt to deference a null pointer.
However, https://hg.mozilla.org/mozilla-central/annotate/ce69b6e1773e/dom/media/MediaFormatReader.cpp#l1054 test that such pointer isn't null...

So there is a race between the test of the pointer value and accessing it. I don't see however how that could be possible.. the Data::mDemuxer is only ever accessed on its own taskqueue.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 16

2 years ago
mozreview-review
Comment on attachment 8862527 [details]
Bug 1357040: P2. Don't rely on NotifyDataArrived to resolve MSE init promise.

https://reviewboard.mozilla.org/r/134418/#review137456
Attachment #8862527 - Flags: review?(gsquelart) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Comment 20

2 years ago
:jwwang

I'm getting time out following P2 changed with the test dom/media/mediasource/test/test_EndedEvent.html

It seeks to currentTime = duration, and wait for the ended event which never comes:

however:
GECKO(30251) | [MediaPlayback #1]: D/MediaFormatReader MediaFormatReader(0x129f2a000)::OnDemuxFailed: Failed to demux video, failure:2154692616
GECKO(30251) | [MediaPlayback #1]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::ScheduleUpdate: SchedulingUpdate(Video)
GECKO(30251) | [MediaPlayback #1]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::Update: Processing update for Video
GECKO(30251) | [MediaPlayback #1]: D/MediaFormatReader MediaFormatReader(0x129f2a000)::DrainDecoder: Requesting Video decoder to drain
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::NotifyNewOutput: Received new Video sample time:3967000 duration:34000
GECKO(30251) | [MediaPlayback #2]: D/MediaFormatReader MediaFormatReader(0x129f2a000)::NotifyNewOutput: Done processing new Video samples
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::ScheduleUpdate: SchedulingUpdate(Video)
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::Update: Processing update for Video
GECKO(30251) | [MediaPlayback #2]: D/MediaFormatReader MediaFormatReader(0x129f2a000)::ReturnOutput: Resolved data promise for Video [3967000, 4001000]
GECKO(30251) | [MediaPlayback #2]: D/MediaFormatReader MediaFormatReader(0x129f2a000)::DrainDecoder: Requesting Video decoder to drain
GECKO(30251) | [MediaPlayback #3]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::ScheduleUpdate: SchedulingUpdate(Video)
GECKO(30251) | [MediaPlayback #3]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::Update: Processing update for Video
GECKO(30251) | [MediaPlayback #3]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::Update: Update(Video) ni=0 no=0 in:12 out:12 qs=0 decoding:0 flushing:0 desc:ffvpx video decoder pending:0 waiting:0 sid:0
GECKO(30251) | [MediaPlayback #3]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::Update: No need for additional input (pending:0)
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::RequestVideoData: RequestVideoData(0, 4000000)
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::ScheduleUpdate: SchedulingUpdate(Video)
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::Update: Processing update for Video
GECKO(30251) | [MediaPlayback #2]: D/MediaFormatReader MediaFormatReader(0x129f2a000)::Update: Rejecting Video promise: EOS
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::Update: Update(Video) ni=0 no=1 in:12 out:12 qs=0 decoding:0 flushing:0 desc:ffvpx video decoder pending:0 waiting:0 sid:0
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::Update: No need for additional input (pending:0)
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::ReleaseResources:
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::ShutdownDecoder: Audio
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::ShutdownDecoder: Video
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::ScheduleUpdate: SchedulingUpdate(Video)
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::Update: Processing update for Video
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::Update: Update(Video) ni=0 no=0 in:0 out:0 qs=0 decoding:0 flushing:0 desc:shutdown pending:0 waiting:0 sid:0
GECKO(30251) | [MediaPlayback #2]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::Update: No need for additional input (pending:0)
GECKO(30251) | [Main Thread]: D/MediaSource MediaSourceResource(0x12a57c080:application/x.mediasource)::IsSuspendedByCache: UNIMPLEMENTED FUNCTION at /Users/jyavenard/Work/Mozilla/mozilla-central/dom/media/mediasource/MediaSourceResource.h:49
GECKO(30251) | --DOMWINDOW == 15 (0x18d9c8800) [pid = 30251] [serial = 15] [outer = 0x0] [url = about:blank]
GECKO(30251) | --DOMWINDOW == 14 (0x18d907000) [pid = 30251] [serial = 14] [outer = 0x0] [url = about:blank]
GECKO(30251) | --DOMWINDOW == 13 (0x185a5e000) [pid = 30251] [serial = 8] [outer = 0x0] [url = about:blank]
GECKO(30251) | --DOMWINDOW == 12 (0x12ac6b800) [pid = 30251] [serial = 11] [outer = 0x0] [url = about:blank]
Failed to retrieve MOZ_UPLOAD_DIR env var
4 INFO TEST-UNEXPECTED-FAIL | dom/media/mediasource/test/test_EndedEvent.html | Test timed out. 
    reportError@SimpleTest/TestRunner.js:121:7
    TestRunner._checkForHangs@SimpleTest/TestRunner.js:142:7
    setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5
    setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5
    setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5
    setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5
    setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5
    setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5
    setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5
    setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5
    setTimeout handler*TestRunner._checkForHangs@SimpleTest/TestRunner.js:163:5
    TestRunner.runTests@SimpleTest/TestRunner.js:380:5
    RunSet.runtests@SimpleTest/setup.js:194:3
    RunSet.runall@SimpleTest/setup.js:173:5
    hookupTests@SimpleTest/setup.js:266:5
EventHandlerNonNull*getTestManifest@http://mochi.test:8888/manifestLibrary.js:45:3
    hookup@SimpleTest/setup.js:246:5
EventHandlerNonNull*@http://mochi.test:8888/tests?autorun=1&consoleLevel=INFO&manifestFile=tests.json&dumpOutputDirectory=%2Fvar%2Ffolders%2Fgd%2Fdg_9jpsd44118kp5cns1rp5c0000gn%2FT:11:1
GECKO(30251) | [Main Thread]: D/MediaSource MediaSourceDecoder(0x10d85e800)::Shutdown: Shutdown
GECKO(30251) | [Main Thread]: D/MediaSource MediaSource(0x12a7eeb40)::Detach: mDecoder=0x10d85e800 owner=0x12a04e890
GECKO(30251) | [Main Thread]: D/MediaSource MediaSource(0x12a7eeb40)::SetReadyState: SetReadyState(aState=0) mReadyState=2
GECKO(30251) | [Main Thread]: D/MediaSource MediaSource(0x12a7eeb40)::QueueAsyncSimpleEvent: Queuing event 'sourceclose'
GECKO(30251) | [Main Thread]: D/MediaSource SourceBuffer(0x1357cb680:video/webm)::Detach: Detach
GECKO(30251) | [Main Thread]: D/MediaSource TrackBuffersManager(0x129893000:video/webm)::Detach:
GECKO(30251) | [Main Thread]: D/MediaSource SourceBufferList(0x1296dc300)::QueueAsyncSimpleEvent: Queue event 'removesourcebuffer'
GECKO(30251) | [Main Thread]: D/MediaSource SourceBuffer(0x1357cb680:video/webm)::Detach: Detach
GECKO(30251) | [Main Thread]: D/MediaSource SourceBuffer(0x1357cb680:video/webm)::Detach: Already detached
GECKO(30251) | [Main Thread]: D/MediaSource SourceBufferList(0x1296dc280)::QueueAsyncSimpleEvent: Queue event 'removesourcebuffer'
GECKO(30251) | MEMORY STAT vsizeMaxContiguous not supported in this build configuration.
GECKO(30251) | MEMORY STAT | vsize 5747MB | residentFast 503MB | heapAllocated 114MB
GECKO(30251) | [MediaPlayback #4]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::ResetDecode:
GECKO(30251) | [MediaPlayback #4]: D/MediaFormatReader MediaFormatReader(0x129f2a000)::Reset: Reset(Video) BEGIN
GECKO(30251) | [MediaPlayback #4]: D/MediaFormatReader MediaFormatReader(0x129f2a000)::Reset: Reset(Video) END
GECKO(30251) | Decoder=10d85e800 MediaDecoder State: channels=0 rate=0 hasAudio=0 hasVideo=1 mPlayState=PAUSED mdsm=129fa9000
GECKO(30251) | reader data:
GECKO(30251) | Audio Decoder: unavailable
GECKO(30251) | Audio Frames Decoded: 0
GECKO(30251) | Video Decoder: shutdown
GECKO(30251) | Hardware Video Decoding: disabled
GECKO(30251) | Video Frames Decoded: 12 (skipped=0)
GECKO(30251) | Video State: ni=0 no=0 wp=0 demuxr=0 demuxq=0 decoder=0 tt=-1.0 tths=-1 in=0 out=0 qs=0 pending:0 wfd=0 wfk=0 sid=0
GECKO(30251) | Dumping Data for Demuxer: 128e53000
GECKO(30251) | 	Dumping Video Track Buffer(video/webm; codecs=vp8): mLastVideoTime=4.001000
GECKO(30251) | 		Video Track Buffer Details: NumSamples=120 Size=250502 Evictable=226862 NextGetSampleIndex=120 NextInsertionIndex=120
GECKO(30251) | 		Video Track Buffered: ranges=[(0.000000, 4.001000)]
GECKO(30251) | [MediaPlayback #5]: D/MediaSource TrackBuffersManager(0x129893000:video/webm)::Seek: Keyframe  found at 0 @ 0
GECKO(30251) | [MediaPlayback #4]: D/MediaFormatReader MediaFormatReader(0x129f2a000)::Shutdown:
GECKO(30251) | Decoder=10d85e800 MediaDecoderStateMachine State: GetMediaTime=4000000 GetClock=-1 mMediaSink=10b057c90 state=COMPLETED mPlayState=2 mSentFirstFrameLoadedEvent=1 IsPlaying=0 mAudioStatus=idle mVideoStatus=idle mDecodedAudioEndTime=0 mDecodedVideoEndTime=4001000mAudioCompleted=0 mVideoCompleted=0
GECKO(30251) | VideoSink Status: IsStarted=0 IsPlaying=0 VideoQueue(finished=1 size=1) mVideoFrameEndTime=0 mHasVideo=0 mVideoSinkEndRequest.Exists()=0 mEndPromiseHolder.IsEmpty()=1
GECKO(30251) | [MediaPlayback #4]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::ShutdownDecoder: Video
GECKO(30251) | [MediaPlayback #5]: D/MediaSource TrackBuffersManager(0x129893000:video/webm)::Seek: Keyframe  found at 0 @ 0
GECKO(30251) | [MediaPlayback #4]: V/MediaFormatReader MediaFormatReader(0x129f2a000)::ReleaseResources:
GECKO(30251) | [Main Thread]: D/MediaSource MediaSource(0x12a7eeb40)::DispatchSimpleEvent: Dispatch event 'sourceclose'
GECKO(30251) | [Main Thread]: D/MediaSource SourceBufferList(0x1296dc300)::DispatchSimpleEvent: Dispatch event 'removesourcebuffer'
GECKO(30251) | [Main Thread]: D/MediaSource SourceBufferList(0x1296dc280)::DispatchSimpleEvent: Dispatch event 'removesourcebuffer'
5 INFO TEST-OK | dom/media/mediasource/test/test_EndedEvent.html | took 319753ms


This shows that the MFR has decoded the last video frame and returned EOS.
Yet, the MDSM shows mVideoCompleted=0 with state=COMPLETED

Per spec:
we are to reach ended playback:
https://html.spec.whatwg.org/multipage/embedded-content.html#ended-playback
"When the current playback position reaches the end of the media resource when the direction of playback is forwards, then the user agent must follow these steps"

we have reached the end of the media resource and the direction of playback is forward:
https://html.spec.whatwg.org/multipage/embedded-content.html#direction-of-playback
"If the element's playbackRate is positive or zero, then the direction of playback is forwards"
Flags: needinfo?(jwwang)

Comment 21

2 years ago
mozreview-review
Comment on attachment 8862663 [details]
Bug 1357040: P3. Don't expect that all data has been processed after metadata.

https://reviewboard.mozilla.org/r/134528/#review137496
Attachment #8862663 - Flags: review?(gsquelart) → review+
I am able to repro the issue on my desktop with
`MOZ_CHAOSMODE=0./mach mochitest dom/media/mediasource/test/test_EndedEvent.html --run-until-failure --repeat 10000`.

http://searchfox.org/mozilla-central/rev/068e6f292503df13515a52321fc3844e237bf6a9/dom/media/MediaDecoderStateMachine.cpp#2391

The if statement is expected to be true for the test but sometimes it fails with newCurrentTime=4000000 and mMaster->Duration()=4001000.

It looks like the duration (4000000) is updated again (to 4001000) after the 'loadedmetadata' event is fired.
Flags: needinfo?(jwwang)
Assignee

Comment 23

2 years ago
oh I see what's happening....

the metadata states a 4s video, but the last video frame is 4.00100

yet, we got EOS, so shouldn't we fire ended anyway?
No, 'ended' is fired when playback reaches the end of stream, not when decoder reaches EOS. In this case, the playback position after seeking is 4s, and there is still 0.001s to go before reaching the end.
Assignee

Comment 25

2 years ago
I understand that. However in webm the duration of the last video frame is estimated. So there's often a discrepancy between the time reported in the metadata and the end time of the last frame.

Maybe I could add a workaround with MSE that if we reach the last frame of the container and the duration in the metadata is close. Then round up to duration.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Comment 31

2 years ago
Gerald, could you review P2 again...

I was afraid there was a race in accessing the MediaSourceDemuxer object.

MediaSourceDecoder::GetDemuxer() returns a raw pointer of its mDemuxer value. This mDemuxer can be reset on the main thread and we were accessing on the MediaSourceDemuxer taskqueue.

So I now store a copy of the pointers and protect it via a mutex. Should the sourcebuffer be detached, that pointer is cleared and we test that value.

I thought of doing something like:

RefPtr<MediaSourceDemuxer> msDemuxer = mParentDecoder->GetDemuxer();

however, I was afraid that on the event MediaSourceDecoder::mDemuxer is cleared, right when having msDemuxer set, we could end up with a cycle (the MediaSourceDemuxer holds a strong reference to the TrackBufferManager)
Flags: needinfo?(gsquelart)

Comment 32

2 years ago
mozreview-review
Comment on attachment 8862890 [details]
Bug 1357040: P4. Don't assume the duration found when reading metadata will be the final one.

https://reviewboard.mozilla.org/r/134764/#review137806
Attachment #8862890 - Flags: review?(gsquelart) → review+

Comment 33

2 years ago
mozreview-review
Comment on attachment 8862891 [details]
Bug 1357040: P5. Remove unecessary code.

https://reviewboard.mozilla.org/r/134766/#review137808
Attachment #8862891 - Flags: review?(gsquelart) → review+

Comment 34

2 years ago
mozreview-review
Comment on attachment 8862892 [details]
Bug 1357040: P6. Don't assume that when stalling all data has been loaded.

https://reviewboard.mozilla.org/r/134768/#review137810
Attachment #8862892 - Flags: review?(gsquelart) → review+
(In reply to Jean-Yves Avenard [:jya] from comment #31)
> Gerald, could you review P2 again...

Done, still r+.

I agree it's better to just avoid cycles where possible. So having this raw pointer protected by a mutex seems better to me.
Flags: needinfo?(gsquelart)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 49

2 years ago
mozreview-review
Comment on attachment 8864053 [details]
Bug 1357040: P7. Only use sourcebuffer again once previous operation completed.

https://reviewboard.mozilla.org/r/135768/#review139008
Attachment #8864053 - Flags: review?(gsquelart) → review+
Jya, 
This bug caused many crashes. Can we land it now?
Flags: needinfo?(jyavenard)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Attachment #8862663 - Attachment is obsolete: true
Assignee

Updated

2 years ago
Attachment #8862890 - Attachment is obsolete: true
Assignee

Updated

2 years ago
Attachment #8862891 - Attachment is obsolete: true
Assignee

Updated

2 years ago
Attachment #8862892 - Attachment is obsolete: true
Assignee

Updated

2 years ago
Attachment #8864053 - Attachment is obsolete: true
Assignee

Comment 53

2 years ago
Moving all the mochitests changes to bug 1362165
Flags: needinfo?(jyavenard)

Comment 54

2 years ago
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f0e461394f7e
P1. Abort if demuxer initialisation isn't complete. r=jwwang
https://hg.mozilla.org/integration/autoland/rev/23327af4c4c7
P2. Don't rely on NotifyDataArrived to resolve MSE init promise. r=gerald

Comment 55

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f0e461394f7e
https://hg.mozilla.org/mozilla-central/rev/23327af4c4c7
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Track 54+/55+ as the volume of crashes in 54 is a bit high.
Assignee

Comment 57

2 years ago
Comment on attachment 8861408 [details]
Bug 1357040: P1. Abort if demuxer initialisation isn't complete.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1341454
[User impact if declined]: Crashes
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: it's an intermittent failure. It's also hard to say as all the crashes lately have been in 54 not 55.
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: both patches are required
[Is the change risky?]: no
[Why is the change risky/not risky?]: we abort an operation early if not initialised.
[String changes made/needed]: none
Attachment #8861408 - Flags: approval-mozilla-aurora?
Comment on attachment 8861408 [details]
Bug 1357040: P1. Abort if demuxer initialisation isn't complete.

54 is on Beta.
Attachment #8861408 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Comment on attachment 8861408 [details]
Bug 1357040: P1. Abort if demuxer initialisation isn't complete.

Fix a crash. Beta54+. Should be in 54 beta 8.
Attachment #8861408 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Jean-Yves Avenard [:jya] from comment #57)
> [Is this code covered by automated tests?]: yes
> [Has the fix been verified in Nightly?]: it's an intermittent failure. It's
> also hard to say as all the crashes lately have been in 54 not 55.
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on jya's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-

Comment 62

2 years ago
hi, there are still crashes with this signature in 54.0b8 after the patch landed there:
https://crash-stats.mozilla.com/signature/?product=Firefox&version=54.0b8&signature=mozilla%3A%3AMP4Demuxer%3A%3ANotifyDataArrived#reports
Flags: needinfo?(jyavenard)
Assignee

Updated

2 years ago
Blocks: 1366208
Assignee

Comment 63

2 years ago
(In reply to [:philipp] from comment #62)
> hi, there are still crashes with this signature in 54.0b8 after the patch
> landed there:
> https://crash-stats.mozilla.com/signature/?product=Firefox&version=54.
> 0b8&signature=mozilla%3A%3AMP4Demuxer%3A%3ANotifyDataArrived#reports

fixed in bug 1366208 (hopefully)
Flags: needinfo?(jyavenard)

Comment 64

2 years ago
thanks :))
Assignee

Updated

2 years ago
Duplicate of this bug: 1360092
You need to log in before you can comment on or make changes to this bug.