From 6bdc0eb3128d7e86eb2a11c74f70d70e6cfefbff Mon Sep 17 00:00:00 2001 From: "Scott S. McCoy" Date: Thu, 1 May 2025 22:19:47 +0100 Subject: [PATCH] spirc: Configurable volume control steps (#1498) * spirc: Configurable volume control steps Allow the volume control steps to be configured via the `--volume-steps` command line parameter. The author personally found the default volume steps of `1024` to be completely unusable, and is presently using `128` as his configuration. Perhaps consider this as a more reasonable default. Additionally, reduce the delay in volume update from a wopping two seconds to 500ms, again for usability. Also clean up the seemingly unnecessary use of a pattern match on whether or not `--initial-volume` was supplied. * fixup! spirc: Configurable volume control steps * fixup! spirc: Configurable volume control steps * fixup! spirc: Configurable volume control steps * fixup! spirc: Configurable volume control steps * fixup! spirc: Configurable volume control steps * fixup! spirc: Configurable volume control steps --------- Co-authored-by: Scott S. McCoy --- CHANGELOG.md | 1 + connect/src/spirc.rs | 12 ++++++---- connect/src/state.rs | 10 ++++++-- src/main.rs | 54 +++++++++++++++++++++++++++++++------------- 4 files changed, 54 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf3b3057..84b34578 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- [connect] Add command line parameter for setting volume steps. - [connect] Add support for `seek_to`, `repeat_track` and `autoplay` for `Spirc` loading - [connect] Add `pause` parameter to `Spirc::disconnect` method (breaking) - [connect] Add `volume_steps` to `ConnectConfig` (breaking) diff --git a/connect/src/spirc.rs b/connect/src/spirc.rs index ee192aaf..95b48a8c 100644 --- a/connect/src/spirc.rs +++ b/connect/src/spirc.rs @@ -133,7 +133,7 @@ enum SpircCommand { const CONTEXT_FETCH_THRESHOLD: usize = 2; // delay to update volume after a certain amount of time, instead on each update request -const VOLUME_UPDATE_DELAY: Duration = Duration::from_secs(2); +const VOLUME_UPDATE_DELAY: Duration = Duration::from_millis(500); // to reduce updates to remote, we group some request by waiting for a set amount of time const UPDATE_STATE_DELAY: Duration = Duration::from_millis(200); @@ -1514,16 +1514,16 @@ impl SpircTask { } fn handle_volume_up(&mut self) { - let volume_steps = self.connect_state.device_info().capabilities.volume_steps as u16; + let volume = (self.connect_state.device_info().volume as u16) + .saturating_add(self.connect_state.volume_step_size); - let volume = (self.connect_state.device_info().volume as u16).saturating_add(volume_steps); self.set_volume(volume); } fn handle_volume_down(&mut self) { - let volume_steps = self.connect_state.device_info().capabilities.volume_steps as u16; + let volume = (self.connect_state.device_info().volume as u16) + .saturating_sub(self.connect_state.volume_step_size); - let volume = (self.connect_state.device_info().volume as u16).saturating_sub(volume_steps); self.set_volume(volume); } @@ -1639,6 +1639,8 @@ impl SpircTask { } fn set_volume(&mut self, volume: u16) { + debug!("SpircTask::set_volume({})", volume); + let old_volume = self.connect_state.device_info().volume; let new_volume = volume as u32; if old_volume != new_volume || self.mixer.volume() != volume { diff --git a/connect/src/state.rs b/connect/src/state.rs index 047f3c8c..64a1e14a 100644 --- a/connect/src/state.rs +++ b/connect/src/state.rs @@ -87,7 +87,7 @@ pub struct ConnectConfig { pub initial_volume: u16, /// Disables the option to control the volume remotely (default: false) pub disable_volume: bool, - /// The steps in which the volume is incremented (default: 1024) + /// Number of incremental steps (default: 64) pub volume_steps: u16, } @@ -99,7 +99,7 @@ impl Default for ConnectConfig { is_group: false, initial_volume: u16::MAX / 2, disable_volume: false, - volume_steps: 1024, + volume_steps: 64, } } } @@ -127,10 +127,15 @@ pub(super) struct ConnectState { /// a context to keep track of the autoplay context autoplay_context: Option, + + /// The volume adjustment per step when handling individual volume adjustments. + pub volume_step_size: u16, } impl ConnectState { pub fn new(cfg: ConnectConfig, session: &Session) -> Self { + let volume_step_size = u16::MAX.checked_div(cfg.volume_steps).unwrap_or(1024); + let device_info = DeviceInfo { can_play: true, volume: cfg.initial_volume.into(), @@ -195,6 +200,7 @@ impl ConnectState { }), ..Default::default() }, + volume_step_size, ..Default::default() }; state.reset(); diff --git a/src/main.rs b/src/main.rs index b14a57a4..6169fac5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -276,6 +276,7 @@ fn get_setup() -> Setup { const VERSION: &str = "version"; const VOLUME_CTRL: &str = "volume-ctrl"; const VOLUME_RANGE: &str = "volume-range"; + const VOLUME_STEPS: &str = "volume-steps"; const ZEROCONF_PORT: &str = "zeroconf-port"; const ZEROCONF_INTERFACE: &str = "zeroconf-interface"; const ZEROCONF_BACKEND: &str = "zeroconf-backend"; @@ -291,6 +292,7 @@ fn get_setup() -> Setup { const DEVICE_SHORT: &str = "d"; const VOLUME_CTRL_SHORT: &str = "E"; const VOLUME_RANGE_SHORT: &str = "e"; + const VOLUME_STEPS_SHORT: &str = ""; // no short flag const DEVICE_TYPE_SHORT: &str = "F"; const FORMAT_SHORT: &str = "f"; const DISABLE_AUDIO_CACHE_SHORT: &str = "G"; @@ -371,6 +373,8 @@ fn get_setup() -> Setup { #[cfg(not(feature = "alsa-backend"))] const VOLUME_RANGE_DESC: &str = "Range of the volume control (dB) from 0.0 to 100.0. Defaults to 60.0."; + const VOLUME_STEPS_DESC: &str = + "Number of incremental steps when responding to volume control updates. Defaults to 64."; let mut opts = getopts::Options::new(); opts.optflag( @@ -570,6 +574,12 @@ fn get_setup() -> Setup { VOLUME_RANGE_DESC, "RANGE", ) + .optopt( + VOLUME_STEPS_SHORT, + VOLUME_STEPS, + VOLUME_STEPS_DESC, + "STEPS", + ) .optopt( NORMALISATION_METHOD_SHORT, NORMALISATION_METHOD, @@ -1457,7 +1467,8 @@ fn get_setup() -> Setup { } else { cache.as_ref().and_then(Cache::volume) } - }); + }) + .unwrap_or_default(); let device_type = opt_str(DEVICE_TYPE) .as_deref() @@ -1480,23 +1491,34 @@ fn get_setup() -> Setup { }) .unwrap_or_default(); + let volume_steps = opt_str(VOLUME_STEPS) + .map(|steps| match steps.parse::() { + Ok(value) => value, + _ => { + let default_value = &connect_default_config.volume_steps.to_string(); + + invalid_error_msg( + VOLUME_STEPS, + VOLUME_STEPS_SHORT, + &steps, + "a positive whole number <= 65535", + default_value, + ); + + exit(1); + } + }) + .unwrap_or_else(|| connect_default_config.volume_steps); + let is_group = opt_present(DEVICE_IS_GROUP); - if let Some(initial_volume) = initial_volume { - ConnectConfig { - name, - device_type, - is_group, - initial_volume, - ..Default::default() - } - } else { - ConnectConfig { - name, - device_type, - is_group, - ..Default::default() - } + ConnectConfig { + name, + device_type, + is_group, + initial_volume, + volume_steps, + ..connect_default_config } };