1177452 - Assertion failure in MDSM::On*Decoded

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Jean-Yves Avenard [:jya]]

Was playing with start time adjustment enabled with MSE (I enabled it should the first sample be close to 0 but not exactly 0).

And I hit this assertion:
#01: mozilla::Maybe<long long>::ref()[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1ede0a0]
#02: mozilla::MediaDecoderStateMachine::StartTimeRendezvous::StartTime()[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x310ade5]
#03: mozilla::MediaDecoderStateMachine::StartTime()[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x306b294]
#04: mozilla::MediaDecoderStateMachine::OnAudioDecoded(mozilla::AudioData*)[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x302f389]
#05: mozilla::EnableIf<(ReturnTypeIs<void (mozilla::MediaDecoderStateMachine::*)(mozilla::AudioData*), void>::value) && (TakesArgument<void (mozilla::MediaDecoderStateMachine::*)(mozilla::AudioData*)>::value), already_AddRefed<mozilla::MediaPromise<nsRefPtr<moz\252[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x30c6cea]
#06: mozilla::MediaPromise<nsRefPtr<mozilla::AudioData>, mozilla::MediaDecoderReader::NotDecodedReason, true>::MethodThenValue<mozilla::MediaDecoderStateMachine, void (mozilla::MediaDecoderStateMachine::*)(mozilla::AudioData*), void (mozilla::MediaDecoderStateM\257[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x30c6b3f]
#07: mozilla::MediaPromise<nsRefPtr<mozilla::AudioData>, mozilla::MediaDecoderReader::NotDecodedReason, true>::ThenValueBase::DoResolveOrReject(mozilla::MediaPromise<nsRefPtr<mozilla::AudioData>, mozilla::MediaDecoderReader::NotDecodedReason, true>::ResolveOrRe\326[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x309c396]
#08: mozilla::MediaPromise<nsRefPtr<mozilla::AudioData>, mozilla::MediaDecoderReader::NotDecodedReason, true>::ThenValueBase::ResolveOrRejectRunnable::Run()[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x309c0dc]
#09: mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run()[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x300b119]
#10: mozilla::MediaTaskQueue::Runner::Run()[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x312050d]
#11: nsThreadPool::Run()[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x198d18]
#12: non-virtual thunk to nsThreadPool::Run()[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x198e1c]
#13: nsThread::ProcessNextEvent(bool, bool*)[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x195814]
#14: NS_ProcessNextEvent(nsIThread*, bool)[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x20bda7]
#15: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x844c9e]
#16: MessageLoop::RunInternal()[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7b19a5]
#17: MessageLoop::RunHandler()[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7b18b5]
#18: MessageLoop::Run()[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7b185d]
#19: nsThread::ThreadFunc(void*)[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x193d39]
#20: _pt_root[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/libnss3.dylib +0x371621]
#21: _pthread_body[/usr/lib/system/libsystem_pthread.dylib +0x3268]
#22: _pthread_body[/usr/lib/system/libsystem_pthread.dylib +0x31e5]
(lldb) 

Not sure how we get there: mAudioStartTime not being defined yet we reach MediaDecoderStateMachine::OnAudioDecoded.

Comment 1

[Jean-Yves Avenard [:jya]]

Step to reproduce:

modify dom/media/mediasource/MediaSourceDemuxer.h and modify ShouldComputeStartTime to return true.
starts FF, set: media.mediasource.format-reader to true
open: http://w3c-test.org/media-source/mediasource-seek-beyond-duration.html

Comment 2

[Jean-Yves Avenard [:jya]]

The reason is MDSM::FirstSampleRejected is rejected with WAITING_FOR_DATA which is a case not handled, this cause OnVideoDecoded to be later called directly.

Comment 3

[Bobby Holley (:bholley)]

Oh, I bet I know what's going on here - when we go into WaitingForData mode and come back out of it, we invoke RequestAudioData from EnsureAudioDecodeTaskQueued rather than from DecodeFirstFrame - the latter pipes things through the start time machinery, the former does not.

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: Ensure start time is always calculated on first frame decoding.

Should the first frame decoding got delayed, such as due to WAITING_FOR_DATA, we wouldn't calculate the start time properly.

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8635892 [details] [diff] [review]
Ensure start time is always calculated on first frame decoding.

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

::: dom/media/MediaDecoderStateMachine.cpp
@@ +1668,5 @@
> +  return NS_OK;
> +}
> +
> +void
> +MediaDecoderStateMachine::RequestAudioData()

Assert OnTaskQueue() and monitor is held so we won't use it wrongly.

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

I think we can have a wrapper around the original MediadecoderReader to handle the start time chores without worrying MDSM in the future. The wrapper should expose exactly the same interface as MediadecoderReader and delegate most of its jobs to the underlying MediadecoderReader.

Comment 7

[Jean-Yves Avenard [:jya]]

I'm not sure I see the advantages in doing so in the reader.
We had discussed this with bholley previously, and once all readers are converged to only use MediaFormatReader it would make sense.

But until then, it's much easier to do so in the MDSM.

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

What would make sense when MediaFormatReader is used?

Comment 9

[Jean-Yves Avenard [:jya]]

Because we would only have a single MediaDecoderReader.

When you have multiple MediaDecoderReader, you're looking at modifying all of them and duplicating code.

Comment 10

[JW Wang [:jwwang] [:jw_wang]]

That's why it is a wrapper around the reader. No readers need to be changed at all. It is something like StartTimeRendezvous exposing the reader interface.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4b0bb04601d

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b4b0bb04601d