1641585 - Assertion failure: mChannels && mFrames && mSampleRate (Mix not called for this cycle?), at /builds/worker/checkouts/gecko/dom/media/AudioMixer.h:54

Status: RESOLVED FIXED

(Core :: Audio/Video: MediaStreamGraph, defect)

Description

[Jason Kratzer [:jkratzer]]

Testcase found while fuzzing mozilla-central rev cfa4bd8e6f78 (built with --enable-debug).

Assertion failure: mChannels && mFrames && mSampleRate (Mix not called for this cycle?), at /builds/worker/checkouts/gecko/dom/media/AudioMixer.h:54

rax = 0x00007f9f5324525c   rdx = 0x0000000000000000
rcx = 0x0000557cd4d70a58   rbx = 0x00007f9f4011b690
rsi = 0x00007f9f6437d8b0   rdi = 0x00007f9f6437c680
rbp = 0x00007f9f4011b640   rsp = 0x00007f9f4011b600
r8 = 0x00007f9f6437d8b0    r9 = 0x00007f9f4011d700
r10 = 0x0000000000000002   r11 = 0x0000000000000000
r12 = 0x0000000000000000   r13 = 0x0000557cd750a540
r14 = 0x0000000000000000   r15 = 0x0000557cd6e05f48
rip = 0x00007f9f4cfadc41
OS|Linux|0.0.0 Linux 5.3.0-51-generic #44~18.04.2-Ubuntu SMP Thu Apr 23 14:27:18 UTC 2020 x86_64
CPU|amd64|family 6 model 94 stepping 3|8
GPU|||
Crash|SIGSEGV|0x0|36
36|0|libxul.so|mozilla::AudioMixer::FinishMixing()|hg:hg.mozilla.org/mozilla-central:dom/media/AudioMixer.h:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|53|0x3f
36|1|libxul.so|mozilla::AudioCaptureTrack::ProcessInput(long, long, unsigned int)|hg:hg.mozilla.org/mozilla-central:dom/media/AudioCaptureTrack.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|100|0x12
36|2|libxul.so|mozilla::MediaTrackGraphImpl::Process(mozilla::AudioMixer*)|hg:hg.mozilla.org/mozilla-central:dom/media/MediaTrackGraph.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|1278|0x3b
36|3|libxul.so|mozilla::MediaTrackGraphImpl::OneIterationImpl(long, long, mozilla::AudioMixer*)|hg:hg.mozilla.org/mozilla-central:dom/media/MediaTrackGraph.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|1402|0xf
36|4|libxul.so|mozilla::GraphRunner::Run()|hg:hg.mozilla.org/mozilla-central:dom/media/GraphRunner.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|114|0x20
36|5|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|1211|0x11
36|6|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|501|0xc
36|7|libxul.so|mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|332|0x13
36|8|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|315|0x17
36|9|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|290|0x8
36|10|libxul.so|nsThread::ThreadFunc(void*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|444|0x8
36|11|libnspr4.so|_pt_root|hg:hg.mozilla.org/mozilla-central:nsprpub/pr/src/pthreads/ptthread.c:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|201|0x7
36|12|libpthread.so.0||||0x76db
36|13|libc.so.6||||0x12188f

Comment 1

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Failed to identify testcase.  Please ensure that the testcase meets the requirements identified here: https://github.com/MozillaSecurity/bugmon#testcase-identification
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Comment 2

[Jason Kratzer [:jkratzer]]

Attachment: testcase.html

Comment 3

[Jason Kratzer [:jkratzer]]

Attachment: 6ecd0a5ace7c998c7c9b4d7fcad6e8630efbf2f2.mp4

Comment 4

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20200528032513-cfa4bd8e6f78.
Failed to bisect testcase (Start build crashes!):
> Start: 73c98da145a7c0ef518404493b23a979f328768e (20190530034755)
> End: cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f (20200528032513)
> BuildFlags: BuildFlags(asan=False, tsan=False, debug=True, fuzzing=False, coverage=False, valgrind=False)

Comment 5

[Tyson Smith [:tsmith]]

Andreas: I was able to reproduce this issue with that attached test case but I was unable to reproduce it under rr.

Comment 6

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Unable to reproduce bug 1641585 using build mozilla-central 20201205093858-7ce95b6cde26. Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 7

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Unable to reproduce bug 1641585 using build mozilla-central 20201205093858-7ce95b6cde26. Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 8

[Andreas Pehrson [:pehrsons]]

gUM and audioCapture are involved. karl, since you've been working in gUM-land could you have fixed this by accident?

Comment 9

[Karl Tomlinson (:karlt)]

The use of getUserMedia() in the testcase now requires a recent user gesture, so the reported crash bug may still exist.

Comment 10

[Karl Tomlinson (:karlt)]

This would also have been affected by removing the "media.getusermedia.audiocapture.enabled" pref from fuzzing.

Comment 11

[Karl Tomlinson (:karlt)]

close()ing the AudioContext when test_getUserMedia_audioCapture.html is enabled on Android triggers this assertion failure on Pixel5 Aarch64.

AudioCaptureTrack::ProcessInput() is getting aTo == aFrom and so AudioSegment::Mix() does not aMixer.Mix().

ProduceDataForTracksBlockByBlock() does not call ProcessInput() when mProcessedTime < mStateComputedTime, but MediaTrackGraphImpl::Process() does when there are no AudioNodeTracks.
close()ing the AudioContext removes the last AudioNodeTrack.

Comment 12

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/mozilla-central/rev/6f33a2fcd373 chose to run OneIteration() even when no frames need to be rendered.

Andreas, I didn't understand all the reasoning. Was this so that control messages get run or were you wanting ProcessInput() to also run?
Was the "next patch in this stack" https://phabricator.services.mozilla.com/D102578?
That was subsequently abandoned.

It seems advantageous to run control messages while awake, but less obviously so to ProcessInput() with no frames.
Do you think we still need to do that?

Comment 13

[Andreas Pehrson [:pehrsons]]

(In reply to Karl Tomlinson (back Nov 29 :karlt) from comment #12)

https://hg.mozilla.org/mozilla-central/rev/6f33a2fcd373 chose to run OneIteration() even when no frames need to be rendered.

Andreas, I didn't understand all the reasoning. Was this so that control messages get run or were you wanting ProcessInput() to also run?
Was the "next patch in this stack" https://phabricator.services.mozilla.com/D102578?

I think so! And for its sake I think we needed ProcessInput() to also run.

That was subsequently abandoned.

That's good. The code it was overtaken by is more direct. Note that since then I've also added gtests that need empty iterations to work (empty callbacks even).

It seems advantageous to run control messages while awake, but less obviously so to ProcessInput() with no frames.
Do you think we still need to do that?

I think where the mentioned new gtest says Process the CrossGraphTransmitter on the primary graph. it just means that it needs to process the ControlMessage that adds the CrossGraphTransmitter on the graph thread, so you might get away with this optimization.

On the other hand I haven't seen evidence that iterating the graph with 0 frames happens in the wild at any frequency other than on rare occasions, so in that light optimizing away ProcessInput for empty iterations might be a case of premature optimization. Or do you have data that suggests otherwise?

Comment 14

[Karl Tomlinson (:karlt)]

Key preferences for the testcase attached here are:

user_pref("media.autoplay.default", 0);
user_pref("media.getusermedia.audiocapture.enabled", true);
user_pref("media.navigator.permission.disabled", true);

with which, the testcase reproduces on Linux desktop in pernosco.

"Effective latency in frames" is 1200, but we are getting data callbacks from the audio stream with fewer frames, sometimes as low as 20.

Paul, do you know if this is expected?
Could this be related to bug 1864010?

Comment 15

[Paul Adenot (:padenot)]

(In reply to Karl Tomlinson (back Nov 29 :karlt) from comment #14)

Key preferences for the testcase attached here are:

user_pref("media.autoplay.default", 0);
user_pref("media.getusermedia.audiocapture.enabled", true);
user_pref("media.navigator.permission.disabled", true);

with which, the testcase reproduces on Linux desktop in pernosco.

"Effective latency in frames" is 1200, but we are getting data callbacks from the audio stream with fewer frames, sometimes as low as 20.

Paul, do you know if this is expected?
Could this be related to bug 1864010?

The latency number is a hint at what is needed for a particular use-case.

Especially on Linux, we can't have any guarantees about the size of the buffer that is passed to the data callback. Bug 1864010 changes what is requested, to be more performant especially on Android.

But it can also be that the current setup (software / hardware) cannot satisfy what has been requested, and so cubeb attempt to do something it feels is best. For example, on some OSes, with Bluetooth devices, it will attempt to use the lowest latency that is "safe" to avoid constant under-running, even if the latency number is smaller than what happens on reality.

Comment 16

[Intermittent Failures Robot]

1 failures in 3791 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-central: 1

Platform and build breakdown:

• windows11-32-2009-qr: 1
  • debug: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1641585&startday=2024-03-18&endday=2024-03-24&tree=all

Comment 17

[Karl Tomlinson (:karlt)]

(In reply to Andreas Pehrson [:pehrsons] from comment #13)

On the other hand I haven't seen evidence that iterating the graph with 0 frames happens in the wild at any frequency other than on rare occasions, so in that light optimizing away ProcessInput for empty iterations might be a case of premature optimization. Or do you have data that suggests otherwise?

The advantage would not be so much in the optimization, but in having the code behave consistently regardless of whether there is an AudioNodeTrack in the graph and in not having to ensure that each ProcessInput() implementation handles this rare case.

Comment 18

[Karl Tomlinson (:karlt)]

Attachment: Bug 1641585 Skip ProcessInput() calls when mStateComputedTime has not advanced r?pehrsons

and so there are no frames to processed.

This is consistent with ProduceDataForTracksBlockByBlock(), so that behavior
is consistent regardless of whether there is an AudioNodeTrack in the graph.

This saves individual ProcessInput() implementations from needing to handle
this rare case.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/97576780052a
Skip ProcessInput() calls when mStateComputedTime has not advanced r=pehrsons

Comment 20

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/97576780052a