1185407 - Have AudioSink::Init return a promse to indicate the end of audio playback

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

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

There are several benefits:
1. we can remove MDSM::DispatchOnAudioSink{Complete,Error} to reduce the bi-directional dependency between MDSM and AudioSink
2. Bug 1153344 doesn't really fix the problem. There are chances that MDSM::DispatchOnAudioSinkComplete is called after MDSM::Reset depending on the thread contention.
3. We can disconnect a promise if we don't care about the result anymore.

Comment 1

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

Bug 1184742 comment 11 is caused by problem 2 listed above.

Stack:
05:16:56 INFO - Assertion failure: !mAudioCompleted, at /builds/slave/m-cen-m64-d-000000000000000000/build/src/dom/media/MediaDecoderStateMachine.cpp:3083
05:17:33 INFO - #01: mozilla::MediaDecoderStateMachine::DonePrerollingAudio() [dom/media/MediaDecoderStateMachine.h:1109]
05:17:33 INFO - #02: mozilla::MediaDecoderStateMachine::StartDecoding() [dom/media/MediaDecoderStateMachine.cpp:1330]
05:17:33 INFO - #03: mozilla::MediaDecoderStateMachine::SeekCompleted() [dom/media/MediaDecoderStateMachine.cpp:2172]
05:17:33 INFO - #04: mozilla::MediaDecoderStateMachine::OnAudioDecoded(mozilla::AudioData*) [xpcom/base/nsRefPtr.h:66]
05:17:33 INFO - #05: mozilla::MozPromise<nsRefPtr<mozilla::AudioData>, mozilla::MediaDecoderReader::NotDecodedReason, true>::MethodThenValue<mozilla::MediaDecoderStateMachine, void (mozilla::MediaDecoderStateMachine::*)(mozilla::AudioData*), void (mozilla::MediaDecoderStateMachine::*)(mozilla::MediaDecoderReader::NotDecodedReason)>::DoResolveOrRejectInternal(mozilla::MozPromise<nsRefPtr<mozilla::AudioData>, mozilla::MediaDecoderReader::NotDecodedReason, true>::ResolveOrRejectValue const&) [dom/media/MozPromise.h:433]
05:17:33 INFO - #06: mozilla::MozPromise<nsRefPtr<mozilla::AudioData>, mozilla::MediaDecoderReader::NotDecodedReason, true>::ThenValueBase::DoResolveOrReject(mozilla::MozPromise<nsRefPtr<mozilla::AudioData>, mozilla::MediaDecoderReader::NotDecodedReason, true>::ResolveOrRejectValue const&) [mfbt/AlreadyAddRefed.h:116]
05:17:33 INFO - #07: mozilla::MozPromise<nsRefPtr<mozilla::AudioData>, mozilla::MediaDecoderReader::NotDecodedReason, true>::ThenValueBase::ResolveOrRejectRunnable::Run() [xpcom/base/nsRefPtr.h:50]
05:17:33 INFO - #08: mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() [mfbt/ThreadLocal.h:104]
05:17:33 INFO - #09: mozilla::TaskQueue::Runner::Run() [dom/media/TaskQueue.h:139]
05:17:33 INFO - #10: nsThreadPool::Run() [xpcom/glue/nsCOMPtr.h:389]
05:17:33 INFO - #11: _ZThn8_N12nsThreadPool3RunEv [obj-firefox/xpcom/threads/Unified_cpp_xpcom_threads0.cpp:242]
05:17:33 INFO - #12: nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:867]
05:17:33 INFO - #13: NS_ProcessNextEvent(nsIThread*, bool) [xpcom/glue/nsThreadUtils.cpp:277]
05:17:33 INFO - #14: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) [ipc/glue/MessagePump.cpp:326]
05:17:33 INFO - #15: MessageLoop::RunInternal() [ipc/chromium/src/base/message_loop.cc:235]
05:17:33 INFO - #16: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:520]
05:17:33 INFO - #17: nsThread::ThreadFunc(void*) [xpcom/threads/nsThread.cpp:362]
05:17:35 INFO - #18: _pt_root [nsprpub/pr/src/pthreads/ptthread.c:215]
05:17:35 INFO - #19: libSystem.B.dylib + 0x39fd6

Comment 2

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

Attachment: 1185407_part1_AudioSink_Init_promise-v1.patch

Have AudioSink::Init return a promise.

Comment 3

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

Attachment: 1185407_part2_cleanup_code-v1.patch

Remove unused code.

Comment 4

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

Attachment: 1185407_part3_dont_shutdown_AudioStream_prematurely-v1.patch

Don't shutdown AudioStream prematurely.

Comment 5

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

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8146c51e8435

Comment 6

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

Need to rebase.

Comment 7

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

Attachment: 1185407_part1_AudioSink_Init_promise-v2.patch

Have AudioSink::Init return a promise.

Comment 8

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

Attachment: 1185407_part2_cleanup_code-v2.patch

Remove unused code.

Comment 9

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

Attachment: 1185407_part3_dont_shutdown_AudioStream_prematurely-v2.patch

Don't shutdown AudioStream prematurely.

Comment 10

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

Thanks for the review!

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0757d9048472
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd41024575c5
https://hg.mozilla.org/integration/mozilla-inbound/rev/564e2565cf83

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0757d9048472
https://hg.mozilla.org/mozilla-central/rev/bd41024575c5
https://hg.mozilla.org/mozilla-central/rev/564e2565cf83