Open Bug 1584959 Opened 6 months ago Updated 16 days ago

Crash in [@ mozilla::AudioConverter::AudioConverter]

Categories

(Core :: Audio/Video: Playback, defect, P3, critical)

Unspecified
Linux
defect

Tracking

()

Tracking Status
firefox-esr68 --- wontfix
firefox72 --- wontfix
firefox73 --- wontfix
firefox74 --- wontfix
firefox75 --- affected
firefox76 --- affected

People

(Reporter: gsvelto, Assigned: achronop, NeedInfo)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, crash, testcase)

Crash Data

Attachments

(2 files)

This bug is for crash report bp-8cd1ffcf-1710-4e4e-b527-93b5e0190929.

Top 10 frames of crashing thread:

0 libxul.so mozilla::AudioConverter::AudioConverter dom/media/AudioConverter.cpp:27
1 libxul.so mozilla::AudioSink::NotifyAudioNeeded mfbt/UniquePtr.h:617
2 libxul.so mozilla::detail::RunnableMethodImpl<mozilla::detail::Listener<RefPtr<mozilla::AudioData> >*, void  xpcom/threads/nsThreadUtils.h:1176
3 libxul.so mozilla::TaskQueue::Runner::Run xpcom/threads/TaskQueue.cpp:199
4 libxul.so nsThreadPool::Run xpcom/threads/nsThreadPool.cpp:256
5 libxul.so non-virtual thunk to nsThreadPool::Run xpcom/threads/nsThreadPool.cpp
6 libxul.so nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1225
7 libxul.so <name omitted> xpcom/threads/nsThreadUtils.cpp:486
8 libxul.so mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:333
9 libxul.so MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290

Not a new crash since reports go back to 67.0 beta. The raw crash reason is:

MOZ_DIAGNOSTIC_ASSERT(aOut.Channels() <= 2 || aIn.Channels() == aOut.Channels()) (Only down/upmixing to mono or stereo is supported at this stage)

Since this is a diagnostic assertion the volume should be limited to nightly and beta. The assertion was introduced in bug 1542097 so CC'ing :jyavenard and :bryce.

The assert suggests that we should never end up with output channels > 2 unless we passing though. Our failing of the assert does rather suggest that it's possible to arrive at such a config though.

Searchfox is down right now, NI myself to look more at this once it's back up.

Flags: needinfo?(bvandyk)
Priority: -- → P3

Based on the crash stacks this is the where the converter is being created with invalid args. For this to happen we must be creating the converter with inputChannels != outputChannels and outputChannels > 2. I.e. data->mChannels != mOutputChannels && mOutputChannels > 2 at that site.

mOutputChannels is set here, and tracing further back, the origin appears to be in the MDSM from after we've read metadata and following getting the first frame. I figure the audio callback is what is pushing data into the audio queue that we're using here (the data var).

My educated guess would be that we have a mismatch between the number of channels we're expecting based on our metadata, and the number of channels in the data we're getting from the audio callback, and that in the cases where the callback returns > 2 channel audio, we run into the assert.

Paul, does that sound plausible to you?

Flags: needinfo?(bvandyk) → needinfo?(padenot)

Alex, do you have time to look at this?

Flags: needinfo?(padenot) → needinfo?(achronop)

Sure, I keep the NI to myself to check later today.

The number of channels in data is being read by the third-party decoder during initialization. For example, for the (multichannel) sample [1] the channel info is set during initialization [2] and it is used later during decoding to create the AudioData [3]. Can the number of channels in metadata (mOutputChannels) and this one be different numbers?

[1] https://www2.iis.fraunhofer.de/AAC/ChID-BLITS-EBU-Narration.mp4
[2] https://searchfox.org/mozilla-central/rev/01d1011ca4a460f751da030d455d35c267c3e210/dom/media/platforms/ffmpeg/FFmpegDataDecoder.cpp#99
[3] https://searchfox.org/mozilla-central/rev/01d1011ca4a460f751da030d455d35c267c3e210/dom/media/platforms/ffmpeg/FFmpegAudioDecoder.cpp#223,246

Flags: needinfo?(achronop) → needinfo?(bvandyk)

I don't know off the top of my head. I would have to delve the code to get an idea, which I don't think I have time to do in the near future.

I expect it will require a certain multi channel output setup to repro, but it would be useful if we had media that was triggering this.

Flags: needinfo?(bvandyk)
Attached video testcase.mp4

This test case reproduces the issue on Windows.

Blocks: grizzly
Flags: in-testsuite?
Keywords: assertion, testcase

Is bug 1376714 a dupe?

The testcase.mp4 file reproduced the following crash on windows 10:
https://crash-stats.mozilla.org/report/index/56440815-6139-4c14-aa7c-bda150191210
My attached device is the built-in one, stereo. It does not crash on Linux, it fails with a decoder error.

(In reply to Tyson Smith [:tsmith] from comment #9)

Is bug 1376714 a dupe?

Yeah, it looks similar.

Duplicate of this bug: 1376714

The attached file from comment 7 is crashing in [1] because the aIn.Channels() returns 2 and aOut.Channels() returns 3. AudioConverter supports upmix/downmix to mono/stereo.

[1] https://searchfox.org/mozilla-central/rev/690e903ef689a4eca335b96bd903580394864a1c/dom/media/AudioConverter.cpp#27-29

The stack is different though:

xul.dll!mozilla::AudioConverter::AudioConverter(const mozilla::AudioConfig & aIn, const mozilla::AudioConfig & aOut) Line 27
	at c:\Users\achro\repos\mozilla-central\dom\media\AudioConverter.cpp(27)
[Inline Frame] xul.dll!mozilla::MakeUnique(mozilla::AudioConfig && aArgs, mozilla::AudioConfig && aArgs) Line 617
	at c:\Users\achro\repos\mozilla-central\obj-x86_64-pc-mingw32\dist\include\mozilla\UniquePtr.h(617)
xul.dll!mozilla::AudioSink::NotifyAudioNeeded() Line 379
	at c:\Users\achro\repos\mozilla-central\dom\media\mediasink\AudioSink.cpp(379)
xul.dll!mozilla::AudioSink::Init(const mozilla::MediaSink::PlaybackParams & aParams, RefPtr<mozilla::MozPromise<bool,nsresult,0>> & aEndedPromise) Line 85
	at c:\Users\achro\repos\mozilla-central\dom\media\mediasink\AudioSink.cpp(85)
xul.dll!mozilla::AudioSinkWrapper::Start(const mozilla::media::TimeUnit & aStartTime, const mozilla::MediaInfo & aInfo) Line 170
	at c:\Users\achro\repos\mozilla-central\dom\media\mediasink\AudioSinkWrapper.cpp(170)
xul.dll!mozilla::VideoSink::Start(const mozilla::media::TimeUnit & aStartTime, const mozilla::MediaInfo & aInfo) Line 283
	at c:\Users\achro\repos\mozilla-central\dom\media\mediasink\VideoSink.cpp(283)
xul.dll!mozilla::MediaDecoderStateMachine::StartMediaSink() Line 3201
	at c:\Users\achro\repos\mozilla-central\dom\media\MediaDecoderStateMachine.cpp(3201)
xul.dll!mozilla::MediaDecoderStateMachine::MaybeStartPlayback() Line 2846
	at c:\Users\achro\repos\mozilla-central\dom\media\MediaDecoderStateMachine.cpp(2846)
xul.dll!mozilla::MediaDecoderStateMachine::DecodingState::Step() Line 2337
	at c:\Users\achro\repos\mozilla-central\dom\media\MediaDecoderStateMachine.cpp(2337)
[Inline Frame] xul.dll!mozilla::detail::RunnableMethodArguments<>::applyImpl(nsMemoryReporterManager * o, nsresult(nsMemoryReporterManager::*)() m, mozilla::Tuple<> & args, std::integer_sequence<unsigned long long>) Line 1124
	at c:\Users\achro\repos\mozilla-central\obj-x86_64-pc-mingw32\dist\include\nsThreadUtils.h(1124)
[Inline Frame] xul.dll!mozilla::detail::RunnableMethodArguments<>::apply(nsMemoryReporterManager * o, nsresult(nsMemoryReporterManager::*)() m) Line 1130
	at c:\Users\achro\repos\mozilla-central\obj-x86_64-pc-mingw32\dist\include\nsThreadUtils.h(1130)
xul.dll!mozilla::detail::RunnableMethodImpl<nsMemoryReporterManager *,nsresult (nsMemoryReporterManager::*)(),1,mozilla::RunnableKind::Standard>::Run() Line 1179
	at c:\Users\achro\repos\mozilla-central\obj-x86_64-pc-mingw32\dist\include\nsThreadUtils.h(1179)
xul.dll!mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() Line 200
	at c:\Users\achro\repos\mozilla-central\obj-x86_64-pc-mingw32\dist\include\mozilla\TaskDispatcher.h(200)
xul.dll!mozilla::TaskQueue::Runner::Run() Line 203
	at c:\Users\achro\repos\mozilla-central\xpcom\threads\TaskQueue.cpp(203)
xul.dll!nsThreadPool::Run() Line 306
	at c:\Users\achro\repos\mozilla-central\xpcom\threads\nsThreadPool.cpp(306)
xul.dll!nsThread::ProcessNextEvent(bool aMayWait, bool * aResult) Line 1251
	at c:\Users\achro\repos\mozilla-central\xpcom\threads\nsThread.cpp(1251)
xul.dll!NS_ProcessNextEvent(nsIThread * aThread, bool aMayWait) Line 486
	at c:\Users\achro\repos\mozilla-central\xpcom\threads\nsThreadUtils.cpp(486)
xul.dll!mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate * aDelegate) Line 333
	at c:\Users\achro\repos\mozilla-central\ipc\glue\MessagePump.cpp(333)
[Inline Frame] xul.dll!MessageLoop::RunInternal() Line 315
	at c:\Users\achro\repos\mozilla-central\ipc\chromium\src\base\message_loop.cc(315)
xul.dll!MessageLoop::RunHandler() Line 309
	at c:\Users\achro\repos\mozilla-central\ipc\chromium\src\base\message_loop.cc(309)
xul.dll!MessageLoop::Run() Line 291
	at c:\Users\achro\repos\mozilla-central\ipc\chromium\src\base\message_loop.cc(291)
xul.dll!nsThread::ThreadFunc(void * aArg) Line 460
	at c:\Users\achro\repos\mozilla-central\xpcom\threads\nsThread.cpp(460)
nss3.dll!_PR_NativeRunThread(void * arg) Line 421
	at c:\Users\achro\repos\mozilla-central\nsprpub\pr\src\threads\combined\pruthr.c(421)
nss3.dll!pr_root(void * arg) Line 140
	at c:\Users\achro\repos\mozilla-central\nsprpub\pr\src\md\windows\w95thred.c(140)
[External Code]
[Inline Frame] mozglue.dll!mozilla::interceptor::FuncHook<mozilla::interceptor::WindowsDllInterceptor<mozilla::interceptor::VMSharingPolicyShared<mozilla::interceptor::MMPolicyInProcess,1>>,void (*)(int, void *, void *)>::operator()(int &) Line 141
	at c:\Users\achro\repos\mozilla-central\obj-x86_64-pc-mingw32\dist\include\nsWindowsDllInterceptor.h(141)
mozglue.dll!patched_BaseThreadInitThunk(int aIsInitialThread, void * aStartAddress, void * aThreadParam) Line 565
	at c:\Users\achro\repos\mozilla-central\mozglue\dllservices\WindowsDllBlocklist.cpp(565)
[External Code]
Assignee: nobody → achronop

The patch above avoids calling the converter if the conversion is not possible. This fixes the crash. However, I am not sure if we want to avoid the crash using that or prevent the error somewhere deeper in the decoder before getting here. I NI Bryce in case he knows better.

Flags: needinfo?(bvandyk)
Duplicate of this bug: 1525108
Crash Signature: [@ mozilla::AudioConverter::AudioConverter] → [@ mozilla::AudioConverter::AudioConverter] [@ mozilla::AudioConverter::AudioConverter(mozilla::AudioConfig const &,mozilla::AudioConfig const &)]
Crash Signature: [@ mozilla::AudioConverter::AudioConverter] [@ mozilla::AudioConverter::AudioConverter(mozilla::AudioConfig const &,mozilla::AudioConfig const &)] → [@ mozilla::AudioConverter::AudioConverter] [@ mozilla::AudioConverter::AudioConverter(mozilla::AudioConfig const &,mozilla::AudioConfig const &)]
You need to log in before you can comment on or make changes to this bug.