1633769 - Too many recording channels requested

Status: RESOLVED FIXED

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

Description

[Alexandre Ratchov]

Attachment: media_libcubeb_src_cubeb_sndio.patch

User Agent: Mozilla/5.0 (X11; OpenBSD amd64; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

Start firefox with sndio support enabled (default on OpenBSD). Go to a web site which needs to record (allow microphone). Start recording and start monitoring the audio sub-system.

Actual results:

Firefox always records 16 channels, which is the maximum supported channels by the libcubeb sndio backend. The underlying audio sub-system up-samples whatever number of channels it has to 16. This consumes more resources and most importantly breaks certain websites.

For instance https://whereby.com, enters an infinite start/stop loop.

The play number of channel uses another logic: only the number of channels needed is requested, for instance 2 in most cases, even if libcubeb reports supporting a maximum of 16.

Expected results:

The expected result is firefox to request the number of channel needed only.

Patching the libcubeb sndio backend (see attached file) to report 2 as maximum number of channels fixes broken websites. The patch is only a workaround to illustrate the problem.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Matthew Gregan [:kinetik]]

It seems weird that we're always opening input streams with whatever cubeb_enumerate_devices reports as max_channels for that device. Alex or Paul, does that sound right to you?

Aside from that, cubeb_sndio is only reporting one "fake" input/output device via cubeb_enumerate_devices. It'd be better if it reported something with realistic values there. cubeb_get_max_channel_count reports a hardcoded value of 8, which is inconsistent with the device reported via enumerate.

Comment 3

[Paul Adenot (:padenot)]

Sites can decide what to use and inspect what the user has, but apparently don't.

The spec says that code like this:

navigator.mediaDevices.getUserMedia({audio: true}).then((stream) => { ... })

opens the default number of channels of the device, but it's possible get the number of channels like so:

stream.getAudioTracks()[0].getSettings().channelCount

but this requires opening the device somehow. Then it can be changed to stereo, for example:

stream.getAudioTracks()[0].applyConstraints({channelCount: 2});

jib, is there a way to get the channel count of a device without attempting to open it? In any case, yes, having a virtual device that has 16 input channels regardless of what the hardware has is sub optimal considering the web model and the fingerprinting protection it mandates.

Comment 4

[Paul Adenot (:padenot)]

Also the output side is per spec: it always open a stereo out (sensible I think), but reports the maximum channel count (AudioContext.destination.maxChannelCount). More importantly it's possible to set things up without opening an system-level audio stream.

Comment 5

[Alexandre Ratchov]

FWIW, the sndio API has no global limit of the number of channels (upper bound for all devices), that's why there's a hard-coded value in sndio_get_max_channel_count().

The API has no per-device "maximum channels" concept. Basically the program requests whatever number of channels it needs and the audio sub-system handles it (it may down-mix or up-mix if necessary, depending of user preferences). The (hardware) number of channels in use may change dynamically, if the user switches between devices. For instance he may start with a simple stereo USB headset, then pull the USB cable and switch seamlessly to its 7.1 channel sound-card.

That's why in sndio_enumerate_devices() reports a "reasonable" upper bound large enough to not restrict the caller. Note that this seems to work for playback, as most of the time firefox creates stereo streams.

Another approach for sndio_enumerate_devices() would be to open the device, get current hardware parameters and close it and return the current number of channels. Do you think it would be better?

I agree that sndio_get_max_channel_count() and sndio_enumerate_devices() should report the same value, I'll post a patch to fix it after few tests.

Comment 6

[Alexandre Ratchov]

BTW, I just noticed that setting in about:config media.getusermedia.channels=1 fixes the recording problem for which I opened the ticket.

Comment 7

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

(In reply to Paul Adenot (:padenot) from comment #3)

The spec says that code like this ... opens the default number of channels of the device,

The spec does not mandate defaults in any way. It says: "The decision of which track to choose from the finalSet is completely up to the User Agent". Constraints, for better or worse, externalize requirements to clients: if clients care about a property at all, constrain it. I think that's what Paul means.

But conversely, browsers are not required to take defaults of underlying drivers into account. Firefox is free to do what we think is best, for users, fingerprint-mitigations, & web compat. E.g. for camera we default to 640x480 resolution if available, then 1280x720 across the board.

If we want to do something like that for channelCount (e.g. 2 if available, then 1), we could. I admit I naively assumed we already did. Maybe look at other browsers too. Alex, do you recall our thinking here?

jib, is there a way to get the channel count of a device without attempting to open it?

I don't know. Do we need the count before opening it?

Comment 8

[Paul Adenot (:padenot)]

(In reply to Alexandre Ratchov from comment #5)

The API has no per-device "maximum channels" concept. Basically the program requests whatever number of channels it needs and the audio sub-system handles it (it may down-mix or up-mix if necessary, depending of user preferences). The (hardware) number of channels in use may change dynamically, if the user switches between devices. For instance he may start with a simple stereo USB headset, then pull the USB cable and switch seamlessly to its 7.1 channel sound-card.

That's why in sndio_enumerate_devices() reports a "reasonable" upper bound large enough to not restrict the caller. Note that this seems to work for playback, as most of the time firefox creates stereo streams.

Yes, output is different, default is 2 for an AudioContext (but changeable at run-time) or the number of channels of the media being played for an HTMLMediaElement.

Another approach for sndio_enumerate_devices() would be to open the device, get current hardware parameters and close it and return the current number of channels. Do you think it would be better?

It might be better because it would match what all the other audio API do.

I agree that sndio_get_max_channel_count() and sndio_enumerate_devices() should report the same value, I'll post a patch to fix it after few tests.

Thanks. get_max_channel_count is used for AudioContext.destination.maxChannelCount, on our side.

Comment 9

[Alex Chronopoulos [:achronop]]

Regarding the input devices, we do not preset anything. At the point we set the channel count we have enumerated the devices and we know what they can offer. The default is to ask the device for the max number of channels that it is capable of. If a specific channel count has been requested we clamp it t the max number channel count.

Comment 10

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

Is "max number of channels" as default POLA, or do we want to cap it (e.g. at stereo)? What do other browsers do here?

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:achronop, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 12

[Landry Breuil (:gaston)]

:padenot, :jib, fwiw we've been using and shipping this patch on OpenBSD since more than a year (cf http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/www/mozilla-firefox/patches/patch-media_libcubeb_src_cubeb_sndio_c?rev=1.9&content-type=text/x-cvsweb-markup), what should be done to get this commited ? Is it reviewable/acceptable as such ?

Comment 13

[Paul Adenot (:padenot)]

Probably yes, just send it here: https://github.com/mozilla/cubeb/ and I'll get it merged and then cubeb will be updated in Firefox -- or I can do it, if it's just this particular one-liner.

Note that the URL changed, it used to be under kinetiknz and got moved to the Mozilla org (if you have an older repo lying around).

Comment 14

[Landry Breuil (:gaston)]

https://github.com/mozilla/cubeb/pull/649 has the patch we've been using - thanks !

Comment 15

[Landry Breuil (:gaston)]

from my understanding bug #1720293 integrated the changes from the PR, and those have been backported to beta (the local patch i had didnt apply anymore on 91.0b9) so i think this bug can be safely closed.

Comment 16

[Paul Adenot (:padenot)]

That's correct, thanks!