1467882 - fix volume handling in sndio backend

Status: RESOLVED FIXED

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

Description

[Landry Breuil (:gaston)]

filing this one on behalf of alexandre, from https://marc.info/?l=openbsd-ports&m=152641946326955&w=2:

=====
Maybe you've noticed that somtimes sound volume in firefox doesn't
match the volume indicator until you touch it.

This is because the firefox audio API has no volume getter and assumes
the initial volume is 1, while sndio saves volumes and allows volume
to be controlled externally, which makes firefox use a wrong
representation of the actual volume.

The workaround is to do like alsa, pulseaudio and other backends: stop
using the native volume control and adjust the volume of the signal in
firefox.
=====

patch has been successfully tested by a bunch of users on OpenBSD - testing on Linux w/ sndio TBD..

Comment 1

[Landry Breuil (:gaston)]

Attachment: bug-1467882

I know i should do a PR in https://github.com/kinetiknz/cubeb/ but at least it's tracked in bugzilla, and i have some hope it gets to m-c at some point :) Can do it too if preferred..

Comment 2

[Alex Chronopoulos [:achronop]]

(In reply to Landry Breuil (:gaston) from comment #1)
> I know i should do a PR in https://github.com/kinetiknz/cubeb/ but at least
> it's tracked in bugzilla, and i have some hope it gets to m-c at some point
> :) Can do it too if preferred..

I'm afraid it's necessary to land it in github repo first. However, it might be faster since github gets the attention of more people. Moreover, you don't have to worry about this bug, once it is landed in github we take care the rest. I promise, the import to m-c will take place soon after the landing.

Comment 3

[Landry Breuil (:gaston)]

Testing this patch at runtime resulted in the initial volume (for the first start) in sndio to be zero, and the client being muted, while the controls in firefox showed it unmuted and at max volume. fixed by toggling mute/putting the volume at 100% via aucatctl, but im not sure this is the expected behaviour.

Comment 4

[Landry Breuil (:gaston)]

It seems the audio controls in sndio and in firefox are completely disconnected from each other - dunno if that's the core of the global issue...

Comment 5

[Alex Chronopoulos [:achronop]]

The patch has been merged in github repo and I could import it but I see that you are describing a problem and I wonder if this is ready. Do you need additional patches to fix it completely?

Comment 6

[Alex Chronopoulos [:achronop]]

I am copying here the answer from Alexander Ratchov. It seems he has an issue with his bugzilla account and he cannot post directly.

------------
Sorry, my bugzilla account is still disabled, meanwhile I reply by
e-mail to comments in the ticket:

Note https://bugzilla.mozilla.org/show_bug.cgi?id=1467882#c3 :
>
> Testing this patch at runtime resulted in the initial volume (for
> the first start) in sndio to be zero, and the client being muted,
> while the controls in firefox showed it unmuted and at max
> volume. fixed by toggling mute/putting the volume at 100% via
> aucatctl, but im not sure this is the expected behaviour.

Currently firefox volume knob adjusts the system volume, which is
persistent. The patch makes firefox adjust the volume of the signal
it's playing, then, the system applies its own the volume setting to
the result.

Landry, I guess, your system volume was set to 0, probably by a
previous (unpatched) instance of firefox. As the patched firefox
doesn't use the system volume control anymore, the system volume
remained to zero no matter firefox knob position.

Note https://bugzilla.mozilla.org/show_bug.cgi?id=1467882#c4 :
>
> It seems the audio controls in sndio and in firefox are completely
> disconnected from each other - dunno if that's the core of the
> global issue...

Yes, that's the intent of the patch. We "want" system and firefox
volumes to be disconnectet, ie the volume is:

        volume = firefox_volume * system_volume

This is because, system volume may be changed externally and there's
no way to notify firefox about volume changes in order to update
firefox volume indicator.

Note https://bugzilla.mozilla.org/show_bug.cgi?id=1467882#c5 :
>
> The patch has been merged in github repo and I could import it but I
> see that you are describing a problem and I wonder if this is
> ready. Do you need additional patches to fix it completely?
>

There are no other patches, AFAIU, the above "problem" is mostly
confusion caused by the fact that OpenBSD users expect knobs in
programs to affect the system volume, i.e.  the one visible in
aucatctl. Unefortulately this not allowed (yet?) by the cubeb API, as
explained in the ticket description.

[...]

If there are plans and/or discussions about extending cubeb to expose
the system volume in firefox, I'd be interested. FYI, programs using
sndio get asynchronous notifications (through a call-back) about
system volume changes, which would allow to update volume
indicators. Each stream has its own system volume setting, which seems
to be how firefox works. Furthermore this would save few CPU cycles,
as volume wouldn't be applied twice (once by cubeb and once by
sndiod).

Comment 7

[Landry Breuil (:gaston)]

Thanks alex, much clearer now ! No more patches needed so the github tip can be merged into m-c whenever you feel it :)

Comment 8

[Paul Adenot (:padenot)]

Comment on attachment 8984568 [details] [diff] [review]
bug-1467882

Review of attachment 8984568 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks Landry, this will land when we uplift cubeb so I'm removing the review request.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e4093890591
Update cubeb from upstream to 0677b30. r=kinetik

Comment 10

[Alex Chronopoulos [:achronop]]

The cubeb import above picked one commit: 0677b30 Fix volume handling in sndio backend

Comment 11

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/0e4093890591