[PR #685] [MERGED] Optimize volume control logic #1017

New issue

Closed

opened 2026-02-27 20:00:50 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 20:00:50 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/librespot-org/librespot/pull/685
Author: @roderickvd
Created: 3/31/2021
Status: ✅ Merged
Merged: 5/26/2021
Merged by: @roderickvd

Base: dev ← Head: log-volume-ctrl-optimisations

📝 Commits (3)

eca505c Improve volume controls
9efd886 Describe new mixer-card getopts behavior
3a2455d Merge branch 'dev' into log-volume-ctrl-optimisations

📊 Changes

10 files changed (+673 additions, -427 deletions)

View changed files

📝 CHANGELOG.md (+23 -9)
📝 connect/src/spirc.rs (+24 -73)
📝 core/src/config.rs (+2 -28)
📝 playback/src/config.rs (+46 -6)
📝 playback/src/mixer/alsamixer.rs (+227 -181)
➕ playback/src/mixer/mappings.rs (+163 -0)
📝 playback/src/mixer/mod.rs (+18 -13)
📝 playback/src/mixer/softmixer.rs (+25 -14)
📝 playback/src/player.rs (+12 -12)
📝 src/main.rs (+133 -91)

📄 Description

Well, what started as a couple of improvements to the log mapping, got me on the road to do a major refactoring of the volume control code after some inspiration by @ashthespy. I think there's quite a bit in here.

Connect:

Move volume computations from connect to playback crate. For connect it's only relevant if there's any sort of volume control so it can enable or disable volume sliders in Connect clients.
Fix volume step size on volume up/down events. Controlling an official Connect device with an official Connect client, I verified the number of steps (64) to be correct. Likewise the volume range from 0..65535 is correct. The volume step size then needs to be 1024 instead of 4096. The fact that this didn't bit us in the tail before, must be that these events are deprecated and official clients now first query the volume, then set the step size themselves.
Remove unused mixer started/stopped logic. They are no-ops for all of our mixers, so unless some downstream crate depends on this with a custom mixer, this should be safe. (At least Spotifyd doesn't.)

Playback:

Provide all four volume controls cubic, fixed, linear and log to both softvol and alsamixer. Previously, cubic was available only on alsamixer.
Fix log and cubic controls to be actually mute at zero setting (they weren't; exp(0) != 0).
Add a command line option --volume-range to set the dB range for cubic and log mappings. Defaults to 60 for softvol and 0 for alsamixer meaning to let Alsa query the device. This behavior can be overridden by specifying a custom volume range. Edit: the default in dev for the log volume control already is 60.
The above point also means the log mapping is a bit more precise even for the default case.
Completely rewrite alsamixer for much more legible code. ~~The new VolumeMapping made it so we could largely eliminate dealing with both dB and raw values.~~ Edit: dB API reintroduced because not all cards scale linearly.
Edit: Make --volume-ctrl {linear|cubic} work as expected on alsamixer without the --mixer-linear-volume cruft.
Edit: Get the initial volume from Alsa, if not specified otherwise.
Edit: Fix the cubic mapping for cards that report their minimum volume is mute, instead of their actual lowest dB setting before that.
The new alsamixer also supports querying dB ranges for Alsa softvol controls (previously only hardware controls were supported). Edit: the use for this is to match the curves of the log and cubic mappings with what was configured for Alsa softvol.
Edit: If no --mixer-card is set on the command line, then alsamixer now first uses the device name that was configured for the backend, or 'default' otherwise (previously it did not use the backend device name).
Our own softvol now stores normalized values in f32 so we spare the CPU cycles of normalizing for every packet.
Some minor code cleanups.

Main:

Improved consistency of the getopts descriptions. The parameters themselves remain unchanged other than removing the Alsa-only --mixer-linear-volume and adding the optional --volume-range.
Some code cleanups, in particular moving the struct variables above the instantiation.

Special thanks to: @JasonLG1979 for extensive testing and suggestions on Alsa

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/librespot-org/librespot/pull/685 **Author:** [@roderickvd](https://github.com/roderickvd) **Created:** 3/31/2021 **Status:** ✅ Merged **Merged:** 5/26/2021 **Merged by:** [@roderickvd](https://github.com/roderickvd) **Base:** `dev` ← **Head:** `log-volume-ctrl-optimisations` --- ### 📝 Commits (3) - [`eca505c`](https://github.com/librespot-org/librespot/commit/eca505c3873e3e0a87b7dd4d56ab25d5d5d67cf2) Improve volume controls - [`9efd886`](https://github.com/librespot-org/librespot/commit/9efd886e9132c18667372ed0eaae80eff1656c17) Describe new `mixer-card` getopts behavior - [`3a2455d`](https://github.com/librespot-org/librespot/commit/3a2455d68652288f69f2ef7557ab02a343aef4a4) Merge branch 'dev' into log-volume-ctrl-optimisations ### 📊 Changes **10 files changed** (+673 additions, -427 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+23 -9) 📝 `connect/src/spirc.rs` (+24 -73) 📝 `core/src/config.rs` (+2 -28) 📝 `playback/src/config.rs` (+46 -6) 📝 `playback/src/mixer/alsamixer.rs` (+227 -181) ➕ `playback/src/mixer/mappings.rs` (+163 -0) 📝 `playback/src/mixer/mod.rs` (+18 -13) 📝 `playback/src/mixer/softmixer.rs` (+25 -14) 📝 `playback/src/player.rs` (+12 -12) 📝 `src/main.rs` (+133 -91) </details> ### 📄 Description Well, what started as a couple of improvements to the log mapping, got me on the road to do a major refactoring of the volume control code after some inspiration by @ashthespy. I think there's quite a bit in here. **Connect:** * Move volume computations from `connect` to `playback` crate. For `connect` it's only relevant if there's any sort of volume control so it can enable or disable volume sliders in Connect clients. * Fix volume step size on volume up/down events. Controlling an official Connect device with an official Connect client, I verified the number of steps (64) to be correct. Likewise the volume range from 0..65535 is correct. The volume step size then needs to be 1024 instead of 4096. The fact that this didn't bit us in the tail before, must be that these events are deprecated and official clients now first query the volume, then set the step size themselves. * Remove unused mixer started/stopped logic. They are no-ops for all of our mixers, so unless some downstream crate depends on this with a custom mixer, this should be safe. (At least Spotifyd doesn't.) **Playback:** * Provide all four volume controls cubic, fixed, linear and log to both `softvol` and `alsamixer`. Previously, cubic was available only on `alsamixer`. * Fix log and cubic controls to be actually mute at zero setting (they weren't; `exp(0) != 0`). * Add a command line option `--volume-range` to set the dB range for cubic and log mappings. Defaults to 60 for `softvol` and 0 for `alsamixer` meaning to let Alsa query the device. This behavior can be overridden by specifying a custom volume range. _Edit: the default in `dev` for the log volume control already is 60._ * The above point also means the log mapping is a bit more precise even for the default case. * Completely rewrite `alsamixer` for much more legible code. ~~The new `VolumeMapping` made it so we could largely eliminate dealing with both dB and raw values.~~ _Edit: dB API reintroduced because not all cards scale linearly._ * _Edit:_ Make `--volume-ctrl {linear|cubic}` work as expected on `alsamixer` without the `--mixer-linear-volume` cruft. * _Edit:_ Get the initial volume from Alsa, if not specified otherwise. * _Edit:_ Fix the cubic mapping for cards that report their minimum volume is mute, instead of their actual lowest dB setting before that. * The new `alsamixer` also supports querying dB ranges for Alsa softvol controls (previously only hardware controls were supported). _Edit: the use for this is to match the curves of the log and cubic mappings with what was configured for Alsa softvol._ * _Edit:_ If no `--mixer-card` is set on the command line, then `alsamixer` now first uses the device name that was configured for the backend, or 'default' otherwise (previously it did not use the backend device name). * Our own `softvol` now stores normalized values in `f32` so we spare the CPU cycles of normalizing for every packet. * Some minor code cleanups. **Main:** * Improved consistency of the getopts descriptions. The parameters themselves remain unchanged other than removing the Alsa-only `--mixer-linear-volume` and adding the optional `--volume-range`. * Some code cleanups, in particular moving the struct variables above the instantiation. Special thanks to: @JasonLG1979 for extensive testing and suggestions on Alsa --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

kerem

2026-02-27 20:00:50 +03:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/librespot#1017

No description provided.

Rows
Columns