Crash in [@ mozilla::MediaPipelineReceiveAudio::PipelineListener::NotifyPullImpl]
Categories
(Core :: WebRTC: Audio/Video, defect, P2)
Tracking
()
People
(Reporter: pehrsons, Assigned: dminor)
References
(Regression)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
This bug is for crash report bp-5e627f49-f8ff-4289-ab07-345430190411.
Top 10 frames of crashing thread:
0 xul.dll void mozilla::MediaPipelineReceiveAudio::PipelineListener::NotifyPullImpl media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp:1655
1 xul.dll bool mozilla::SourceMediaStream::PullNewData dom/media/MediaStreamGraph.cpp:2588
2 xul.dll void mozilla::MediaStreamGraphImpl::UpdateGraph dom/media/MediaStreamGraph.cpp:1196
3 xul.dll mozilla::MediaStreamGraphImpl::OneIteration dom/media/MediaStreamGraph.cpp:1395
4 xul.dll long mozilla::AudioCallbackDriver::DataCallback dom/media/GraphDriver.cpp:892
5 xul.dll passthrough_resampler<float>::fill media/libcubeb/src/cubeb_resampler.cpp:76
6 xul.dll static long `anonymous namespace'::refill media/libcubeb/src/cubeb_wasapi.cpp:477
7 xul.dll static bool `anonymous namespace'::refill_callback_duplex media/libcubeb/src/cubeb_wasapi.cpp
8 xul.dll static unsigned int `anonymous namespace'::wasapi_stream_render_loop media/libcubeb/src/cubeb_wasapi.cpp
9 ucrtbase.dll o_strcat_s
DivideByZero in code added in [1], aka bug 901633. It's likely that something else triggered this later on. According to crash-stats it started in 65, and with this being a pull of a receive-stream, I suspect webrtc.org and the 64 update, bug 1376873. Supposedly AudioSessionConduit::GetAudioFrame()
could have changed, in a way such that there is a new condition in which it gives no error but too few samples. Marking the webrtc.org 64 update as the regressor for now.
Comment 2•5 years ago
|
||
While this has been noted in Windows, we're seeing this on all platforms -- Windows, Mac, and Linux, running Version 70.0
I can get a crash nearly every time at this spot from this website:
https://cb.virtualairwaves.com/
This WebRTC site works fine on Chrome and Safari.
Assignee | ||
Comment 3•5 years ago
|
||
Thank you for letting us know, I can get a crash consistently while changing between channels on that site. I'll have a look.
Assignee | ||
Comment 4•5 years ago
|
||
In the crash I'm looking at now, we're getting samplesLength = 71, samplesPer10ms = 441, so we're getting less than a full frame of data, leading to us calculating a channelCount of zero which leads to the divide by zero and crash.
Comment 5•5 years ago
|
||
Shouldn't channel count always be a minimum of "1" even if you should pick up a short frame?
Assignee | ||
Comment 6•5 years ago
|
||
The number of channels is available in mAudioFrame in GetAudioFrame so
there is no reason to calculate it after the fact in MediaPipeline.
Comment 8•5 years ago
|
||
bugherder |
Comment 9•5 years ago
|
||
Doesn't look like the volume is high enough to worry about ESR68 uplift, but did you want to nominate this for Beta?
Comment 10•5 years ago
|
||
I'd love to see this fixed ASAP so we can get our users on Firefox.
Assignee | ||
Comment 11•5 years ago
|
||
Comment on attachment 9104901 [details]
Bug 1543622 - Make number of channels out param of GetAudioFrame; r=pehrsons!
Beta/Release Uplift Approval Request
- User impact if declined: Crashes with WebRTC audio.
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): This is a low risk change because it is grabbing the channel count directly from the audio frame rather than relying upon calculating it from other parameters, so very little code has changed.
- String changes made/needed: None
Comment 12•5 years ago
|
||
Comment on attachment 9104901 [details]
Bug 1543622 - Make number of channels out param of GetAudioFrame; r=pehrsons!
Low risk crash fix for a non-frequent crash but we are still in early betas, uplift approved for 71 beta 6, thanks.
Comment 13•5 years ago
|
||
bugherder uplift |
Comment 15•5 years ago
|
||
Tested in Firefox 71 beta. It's fixed! Thanks a lot.
Updated•3 years ago
|
Description
•