Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting volume low requires unexpectedly low values #74

Open
andrewmcguinness opened this issue Dec 23, 2021 · 6 comments · May be fixed by #75
Open

Setting volume low requires unexpectedly low values #74

andrewmcguinness opened this issue Dec 23, 2021 · 6 comments · May be fixed by #75

Comments

@andrewmcguinness
Copy link

From BMO 1746799 https://bugzilla.mozilla.org/show_bug.cgi?id=1746799

Per https://freedesktop.org/software/pulseaudio/doxygen/volume.html (my italics)

The volumes in PulseAudio are cubic in nature and applications shouldn't perform calculations with them directly. Instead, they should be converted to and from either dB or a linear scale:

dB - pa_sw_volume_from_dB() / pa_sw_volume_to_dB()
Linear - pa_sw_volume_from_linear() / pa_sw_volume_to_linear()

For simple multiplication, pa_sw_volume_multiply() and pa_sw_cvolume_multiply() can be used.

It's often unknown what scale hardware volumes relate to. Don't use the above functions on sink and source volumes, unless the sink or source in question has the PA_SINK_DECIBEL_VOLUME or PA_SOURCE_DECIBEL_VOLUME flag set. The conversion functions are rarely needed anyway, most of the time it's sufficient to treat all volumes as opaque with a range from PA_VOLUME_MUTED (0%) to PA_VOLUME_NORM (100%).

cubeb does use pa_sw_volume_from_linear() on source volumes, and the result is that even a very low setting of volume through cubeb (e.g 0.02) results in a quite loud volume.

In contrast, pulseaudio's own volume control does just map from PA_VOLUME_MUTED to PA_VOLUME_NORM without using volume_from_linear() -- https://gitlab.freedesktop.org/pulseaudio/pavucontrol/-/blob/master/src/channelwidget.cc#L42

andrewmcguinness added a commit to andrewmcguinness/cubeb-pulse-rs that referenced this issue Dec 23, 2021
@andrewmcguinness andrewmcguinness linked a pull request Dec 23, 2021 that will close this issue
@kinetiknz
Copy link
Contributor

Thanks for investigating this and proposing a fix! The fix looks reasonable from my understanding, but I want to investigate further - I'll take a closer look at this again after the holiday season.

@andrewmcguinness
Copy link
Author

Some more research:

Most other backends are also using linear volume -- cubeb itself multiplies the samples by the volume value. So does this code, if flat volumes are in use. I think they are wrong too. This isn't a recently introduced error, it has always been this way.

Significantly, Windows is different. It does not use linear volume. The volume value is passed on to the WASAPI function, and the Microsoft documentation explains how and why it uses a logarithmic scale:

https://docs.microsoft.com/en-us/windows/win32/coreaudio/audio-tapered-volume-controls

So my change, if I'm right, is bringing the pulseaudio backend approximately into line with Windows (it's a cube rather than a logarithm, but the principle is the same), but different from most other backends.

I've written an article https://www.vsbadminton.co.uk/randoms/volumecontrol.html about the issue.

@padenot
Copy link
Collaborator

padenot commented Mar 8, 2022

I've just remembered this while looking for something else, sorry about forgetting. I checked macOS behave similarly as current Linux, Windows is different as you note.

I think I like Windows' way the best (and it does make sense physically). We could trivially move macOS to this curve (or something similar like real log or whatever) as well, in addition to Linux.

I tested all other browsers on all OSes, and they really tend to do what Firefox on Linux or macOS currently do, so I'm kind of worried about a compatibility issue. But then again, maybe nobody cares, and we can make it the way we like.

@andrewmcguinness
Copy link
Author

I'm not easily able to test outside my own environment setup, but I've been using nightly with my PR #75 all year and to me it is an improvement.

@andrewmcguinness
Copy link
Author

bump... @padenot ?

@pallaswept
Copy link

This patch is a great relief, thank you @andrewmcguinness .

It needed a minor rebase:
@@ -757,8 +757,8 @@
becomes
@@ -765,8 +765,8 @@

And now when I press keys or click sliders to change volume in firefox, I see the same volume value (percentage) reflected in the player in firefox, pipewire/pulse, my DE's popups, connected mobile devices, etc.

Similarly, at any given percentage of volume, firefox will produce the same volume (audible and value) as all other clients at that same volume. For example, firefox at 50%, qmplay (pulse) at 50%, and haruna (pipewire native) at 50%, all playing the same source, all say 50%, and all sound the same. Without this patch, all other clients would be the same, but firefox would be audibly far louder, and the volume value would be shown as 79% everywhere else, when the html media player shows it is 50%.

Especially, the experience of "My volume jumps from silent, to nearly half, in the first click of the volume slider" is gone, and it is now possible to set a 'quiet' volume even if using 5% or 10% increments - without the patch we have 0%, 46%!, 58%, with the patch we have 0%, 10%, 20%.

Likewise, the upper regions of volume sliders are now far more effective, there is no more "the top three notches of volume are three degrees of too loud" - without the patch, they are 100%, 97% and 92%, with it, they are 100%, 90% and 80%.

I have tested this patch on phone and TV speakers, 5 sets of high-end headphones and a cheap set, and two 5.1 surround systems. I also tested volume controls from two keyboards, two headsets, and an android phone. It behaved as expected in all cases.

I think it would be extremely good to merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants