1603761 - Crash in [@ cubeb_backend::capi::capi_enumerate_devices]

Status: VERIFIED FIXED

(Core :: Audio/Video: cubeb, defect, P2)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

This bug is for crash report bp-e90f9b75-d388-4899-8a30-acb330191213.

STRs: reproduces 100% for me in Nightly 2.0a1 (2019-11-11):

1. Wear earbuds (in my case Apple 3.5 mm jack)
2. Open https://webrtc.github.io/samples/src/content/devices/input-output/
3. Pick "External Microphone" in the Firefox permission prompt, and "Allow"
4. See and hear yourself
5. Unplug earbuds

Expected result: No audio, as audio track ends (spec) or reverts to default mic (Chrome)
Actual result: crash

Top 10 frames of crashing thread:

0 XUL gkrust_shared::panic_hook mfbt/Assertions.h:332
1 XUL core::ops::function::Fn::call src/libcore/ops/function.rs:69
2 XUL std::panicking::rust_panic_with_hook src/libstd/panicking.rs:481
3 XUL std::panicking::begin_panic src/libstd/panicking.rs:411
4 XUL cubeb_backend::capi::capi_enumerate_devices media/libcubeb/cubeb-coreaudio-rs/src/backend/mod.rs:1496
5 XUL cubeb_enumerate_devices media/libcubeb/src/cubeb.c:587
6 XUL mozilla::CubebDeviceEnumerator::EnumerateAudioDevices dom/media/webrtc/CubebDeviceEnumerator.cpp:227
7 XUL mozilla::CubebDeviceEnumerator::EnumerateAudioInputDevices dom/media/webrtc/CubebDeviceEnumerator.cpp:78
8 XUL mozilla::MediaEngineWebRTC::EnumerateDevices dom/media/webrtc/MediaEngineWebRTC.cpp:306
9 XUL mozilla::GetMediaDevices dom/media/MediaManager.cpp:1353

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

[Tracking Requested - why for this release]: I've verified it's not crashing in 71 release, which is consistent with the regressing bug 1581000 being a Nightly-only change. I wasn't sure how to set the tracking flags for this, but marked 73 as affected, in case there are plans for this to ride the train.

Regression range:

15:41.50 INFO: Narrowed inbound regression window from [8781f5f5, c116c122] (4 builds) to [c7e591ee, c116c122] (2 builds) (~1 steps left)
15:41.50 INFO: No more inbound revisions, bisection finished.
15:41.50 INFO: Last good revision: c7e591ee61c5368f31a48d6317d5521f16594db4
15:41.50 INFO: First bad revision: c116c122cc6f555d67ec4d863a347073a1f255bd
15:41.50 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c7e591ee61c5368f31a48d6317d5521f16594db4&tochange=c116c122cc6f555d67ec4d863a347073a1f255bd

Comment 2

[Alex Chronopoulos [:achronop]]

It seems to crash in the rust backend when the input devices are being enumerated. Chunmin, let me know if you want me to attempt to repro or to experiment with the c-api.

Comment 3

[C.M.Chang[:chunmin]]

It crashes when the input devices returns an empty list of the available rate range.

It susprises me that the system API returns an empty list without any error. Since get_ranges_of_device_sample_rate is just used for logging the device information, it's ok to remove that assertion. The rate won't be used other than logging so removing that assertion should be fine.

However, I wonder if that device constantly returns the empty list of the available rate range, or the empty list is just returned in a specific situation

Jan-Ivar Bruaroey, could you help me to check the following things?

1. What is the Rate of the "External Microphone" used in the test (the earbuds with 3.5 mm jack?) in the about:support.
   • It should crash when opening about:support if that device constantly returns a empty list of the available rate range
2. Check if this happen when using other 3.5 mm jack earbud or other headset

(In reply to Alex Chronopoulos [:achronop] from comment #2)

It seems to crash in the rust backend when the input devices are being enumerated. Chunmin, let me know if you want me to attempt to repro or to experiment with the c-api.

Alex, thanks! I guess C version will work fine since C version has a default rate range. It won't hit any assertion.

Comment 4

[C.M.Chang[:chunmin]]

Attachment: GitHub Pull Request

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

What is the Rate of the "External Microphone" used in the test (the earbuds with 3.5 mm jack?) in the about:support.

default: 48000, support: 44100 - 96000

same as internal one.

Comment 6

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Didn't crash disconnecting bluetooth mic, or USB mic. Unfortunately, I don't have a second 3.5mm jack mic to test with.

Comment 7

[C.M.Chang[:chunmin]]

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #5)

What is the Rate of the "External Microphone" used in the test (the earbuds with 3.5 mm jack?) in the about:support.

default: 48000, support: 44100 - 96000

So the empty list of the available rate range only returns when the devices are changed. I think this might need to be investigated a little bit but unfortunately I am not able to reprodue this.

It's weird to me that the device-collection-changed is fired when uplugginf the 3.5mm jack earbuds. The only thing changed is data source of the device, not device it's self.

Anyway, I'll remove that assertion.

Comment 8

[C.M.Chang[:chunmin]]

Attachment: Bug 1603761 - Update cubeb-coreaudio to 868d847. r?padenot

Pick commits:
b1ad8b9 - Minor cleanup (#27)
868d847 - Return error when get_ranges_of_device_sample_rate gives an empty vector (#30)

Comment 9

[Alex Chronopoulos [:achronop]]

The way I look at it, makes sense to signal on the data source change because the name of the built-in device changes, for example, from "Internal Speaker" to "External Speaker" when the earpods are connected. So we need the notification to update the list. However, this is a special case because only the name changes and not the device id. A component observing in the higher level can be easily confused and assumes that the device has been added or removed.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e778231b145d
Update cubeb-coreaudio to 868d847. r=padenot

Comment 11

[C.M.Chang[:chunmin]]

(In reply to Alex Chronopoulos [:achronop] from comment #9)

The way I look at it, makes sense to signal on the data source change because the name of the built-in device changes, for example, from "Internal Speaker" to "External Speaker" when the earpods are connected. So we need the notification to update the list.

Oh, I get it. That makes sense to me. Thanks for the explanation!

Comment 12

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6780eefc193f
Backed out changeset e778231b145d for causing firefox-ui test failures.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fbe78414b6ca
Update cubeb-coreaudio to 868d847. r=padenot

Comment 14

[Arthur Iakab [arthur_iakab]]

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

Comment 15

[Cristian Fogel [:cfogel]]

Was unable to reproduce this issue with our hardware setup (3.5 jack / USB typeC to USB headphones+mic).
Removing the qe+ flag because of this.

• there was no crash.

Comment 16

[Cristian Fogel [:cfogel]]

Jan-Ivar, any chance you could confirm if the issue is fixed on your setup?
Thanks in advance!

Comment 17

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

This was a regression in a nightly-only feature (from bug 1581000). I've verified the problem reproduces in

• https://download-origin.cdn.mozilla.net/pub/firefox/nightly/2019/12/2019-12-16-09-50-52-mozilla-central/firefox-73.0a1.en-US.mac.dmg

...and appears fixed in

• https://download-origin.cdn.mozilla.net/pub/firefox/nightly/2019/12/2019-12-19-09-50-06-mozilla-central/firefox-73.0a1.en-US.mac.dmg