From eca505c3873e3e0a87b7dd4d56ab25d5d5d67cf2 Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Mon, 24 May 2021 15:53:32 +0200 Subject: [PATCH 1/2] Improve volume controls This is a squashed commit featuring the following: Connect: - Synchronize player volume with mixer volume on playback - Fix step size on volume up/down events - Remove no-op mixer started/stopped logic Playback: - Move from `connect` to `playback` crate - Make cubic volume control available to all mixers with `--volume-ctrl cubic` - Normalize volumes to `[0.0..1.0]` instead of `[0..65535]` for greater precision and performance (breaking) - Add `--volume-range` option to set dB range and control `log` and `cubic` volume control curves - Fix `log` and `cubic` volume controls to be mute at zero volume Alsa mixer: - Complete rewrite (breaking) - Query card dB range for the `log` volume control unless specified otherwise - Query dB range from Alsa softvol (previously only from hardware) - Use `--device` name for `--mixer-card` unless specified otherwise - Fix consistency for `cubic` between cards that report minimum volume as mute, and cards that report some dB value - Fix `--volume-ctrl {linear|log}` to work as expected - Removed `--mixer-linear-volume` option; use `--volume-ctrl linear` instead --- CHANGELOG.md | 23 +- connect/src/spirc.rs | 97 ++----- core/src/config.rs | 30 +-- playback/src/config.rs | 52 +++- playback/src/mixer/alsamixer.rs | 438 ++++++++++++++++++-------------- playback/src/mixer/mappings.rs | 163 ++++++++++++ playback/src/mixer/mod.rs | 31 ++- playback/src/mixer/softmixer.rs | 39 ++- playback/src/player.rs | 24 +- src/main.rs | 230 ++++++++++------- 10 files changed, 689 insertions(+), 438 deletions(-) create mode 100644 playback/src/mixer/mappings.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a775d4c..d549c71f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,29 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html) since v0.2.0. ## [Unreleased] +### Added +- [playback] Add `--volume-range` option to set dB range and control `log` and `cubic` volume control curves +- [playback] `alsamixer`: support for querying dB range from Alsa softvol + +### Changed +* [audio, playback] Moved `VorbisDecoder`, `VorbisError`, `AudioPacket`, `PassthroughDecoder`, `PassthroughError`, `AudioError`, `AudioDecoder` and the `convert` module from `librespot-audio` to `librespot-playback`. The underlying crates `vorbis`, `librespot-tremor`, `lewton` and `ogg` should be used directly. (breaking) +- [connect, playback] Moved volume controls from `connect` to `playback` crate +* [connect] Synchronize player volume with mixer volume on playback +- [playback] Make cubic volume control available to all mixers with `--volume-ctrl cubic` +- [playback] Normalize volumes to `[0.0..1.0]` instead of `[0..65535]` for greater precision and performance (breaking) +- [playback] `alsamixer`: complete rewrite (breaking) +- [playback] `alsamixer`: query card dB range for the `log` volume control unless specified otherwise +- [playback] `alsamixer`: use `--device` name for `--mixer-card` unless specified otherwise + +### Fixed +- [connect] Fix step size on volume up/down events +- [playback] Fix `log` and `cubic` volume controls to be mute at zero volume +- [playback] `alsamixer`: make `cubic` consistent between cards that report minimum volume as mute, and cards that report some dB value +- [playback] `alsamixer`: make `--volume-ctrl {linear|log}` work as expected ### Removed - -* [librespot-audio] Removed `VorbisDecoder`, `VorbisError`, `AudioPacket`, `PassthroughDecoder`, `PassthroughError`, `AudioError`, `AudioDecoder` and the `convert` module from `librespot_audio`. The underlying crates `vorbis`, `librespot-tremor`, `lewton` and `ogg` should be used directly. +- [connect] Removed no-op mixer started/stopped logic +- [playback] `alsamixer`: removed `--mixer-linear-volume` option; use `--volume-ctrl linear` instead ### Fixed diff --git a/connect/src/spirc.rs b/connect/src/spirc.rs index eeb840d2..76cf7054 100644 --- a/connect/src/spirc.rs +++ b/connect/src/spirc.rs @@ -3,7 +3,7 @@ use std::pin::Pin; use std::time::{SystemTime, UNIX_EPOCH}; use crate::context::StationContext; -use crate::core::config::{ConnectConfig, VolumeCtrl}; +use crate::core::config::ConnectConfig; use crate::core::mercury::{MercuryError, MercurySender}; use crate::core::session::Session; use crate::core::spotify_id::{SpotifyAudioType, SpotifyId, SpotifyIdError}; @@ -54,7 +54,6 @@ struct SpircTask { device: DeviceState, state: State, play_request_id: Option, - mixer_started: bool, play_status: SpircPlayStatus, subscription: BoxedStream, @@ -82,13 +81,15 @@ pub enum SpircCommand { } struct SpircTaskConfig { - volume_ctrl: VolumeCtrl, autoplay: bool, } const CONTEXT_TRACKS_HISTORY: usize = 10; const CONTEXT_FETCH_THRESHOLD: u32 = 5; +const VOLUME_STEPS: i64 = 64; +const VOLUME_STEP_SIZE: u16 = 1024; // (std::u16::MAX + 1) / VOLUME_STEPS + pub struct Spirc { commands: mpsc::UnboundedSender, } @@ -163,10 +164,10 @@ fn initial_device_state(config: ConnectConfig) -> DeviceState { msg.set_typ(protocol::spirc::CapabilityType::kVolumeSteps); { let repeated = msg.mut_intValue(); - if let VolumeCtrl::Fixed = config.volume_ctrl { - repeated.push(0) + if config.has_volume_ctrl { + repeated.push(VOLUME_STEPS) } else { - repeated.push(64) + repeated.push(0) } }; msg @@ -214,36 +215,6 @@ fn initial_device_state(config: ConnectConfig) -> DeviceState { } } -fn calc_logarithmic_volume(volume: u16) -> u16 { - // Volume conversion taken from https://www.dr-lex.be/info-stuff/volumecontrols.html#ideal2 - // Convert the given volume [0..0xffff] to a dB gain - // We assume a dB range of 60dB. - // Use the equation: a * exp(b * x) - // in which a = IDEAL_FACTOR, b = 1/1000 - const IDEAL_FACTOR: f64 = 6.908; - let normalized_volume = volume as f64 / std::u16::MAX as f64; // To get a value between 0 and 1 - - let mut val = std::u16::MAX; - // Prevent val > std::u16::MAX due to rounding errors - if normalized_volume < 0.999 { - let new_volume = (normalized_volume * IDEAL_FACTOR).exp() / 1000.0; - val = (new_volume * std::u16::MAX as f64) as u16; - } - - debug!("input volume:{} to mixer: {}", volume, val); - - // return the scale factor (0..0xffff) (equivalent to a voltage multiplier). - val -} - -fn volume_to_mixer(volume: u16, volume_ctrl: &VolumeCtrl) -> u16 { - match volume_ctrl { - VolumeCtrl::Linear => volume, - VolumeCtrl::Log => calc_logarithmic_volume(volume), - VolumeCtrl::Fixed => volume, - } -} - fn url_encode(bytes: impl AsRef<[u8]>) -> String { form_urlencoded::byte_serialize(bytes.as_ref()).collect() } @@ -280,9 +251,8 @@ impl Spirc { let (cmd_tx, cmd_rx) = mpsc::unbounded_channel(); - let volume = config.volume; + let initial_volume = config.initial_volume; let task_config = SpircTaskConfig { - volume_ctrl: config.volume_ctrl.to_owned(), autoplay: config.autoplay, }; @@ -302,7 +272,6 @@ impl Spirc { device, state: initial_state(), play_request_id: None, - mixer_started: false, play_status: SpircPlayStatus::Stopped, subscription, @@ -318,7 +287,12 @@ impl Spirc { context: None, }; - task.set_volume(volume); + if let Some(volume) = initial_volume { + task.set_volume(volume); + } else { + let current_volume = task.mixer.volume(); + task.set_volume(current_volume); + } let spirc = Spirc { commands: cmd_tx }; @@ -437,20 +411,6 @@ impl SpircTask { dur.as_millis() as i64 + 1000 * self.session.time_delta() } - fn ensure_mixer_started(&mut self) { - if !self.mixer_started { - self.mixer.start(); - self.mixer_started = true; - } - } - - fn ensure_mixer_stopped(&mut self) { - if self.mixer_started { - self.mixer.stop(); - self.mixer_started = false; - } - } - fn update_state_position(&mut self, position_ms: u32) { let now = self.now_ms(); self.state.set_position_measured_at(now as u64); @@ -600,7 +560,6 @@ impl SpircTask { _ => { warn!("The player has stopped unexpectedly."); self.state.set_status(PlayStatus::kPlayStatusStop); - self.ensure_mixer_stopped(); self.notify(None, true); self.play_status = SpircPlayStatus::Stopped; } @@ -659,7 +618,6 @@ impl SpircTask { info!("No more tracks left in queue"); self.state.set_status(PlayStatus::kPlayStatusStop); self.player.stop(); - self.mixer.stop(); self.play_status = SpircPlayStatus::Stopped; } @@ -767,7 +725,6 @@ impl SpircTask { self.device.set_is_active(false); self.state.set_status(PlayStatus::kPlayStatusStop); self.player.stop(); - self.ensure_mixer_stopped(); self.play_status = SpircPlayStatus::Stopped; } } @@ -782,7 +739,11 @@ impl SpircTask { position_ms, preloading_of_next_track_triggered, } => { - self.ensure_mixer_started(); + // Synchronize the volume from the mixer. This is useful on + // systems that can switch sources from and back to librespot. + let current_volume = self.mixer.volume(); + self.set_volume(current_volume); + self.player.play(); self.state.set_status(PlayStatus::kPlayStatusPlay); self.update_state_position(position_ms); @@ -792,7 +753,6 @@ impl SpircTask { }; } SpircPlayStatus::LoadingPause { position_ms } => { - self.ensure_mixer_started(); self.player.play(); self.play_status = SpircPlayStatus::LoadingPlay { position_ms }; } @@ -962,7 +922,6 @@ impl SpircTask { self.state.set_playing_track_index(0); self.state.set_status(PlayStatus::kPlayStatusStop); self.player.stop(); - self.ensure_mixer_stopped(); self.play_status = SpircPlayStatus::Stopped; } } @@ -1007,19 +966,13 @@ impl SpircTask { } fn handle_volume_up(&mut self) { - let mut volume: u32 = self.device.get_volume() as u32 + 4096; - if volume > 0xFFFF { - volume = 0xFFFF; - } - self.set_volume(volume as u16); + let volume = (self.device.get_volume() as u16).saturating_add(VOLUME_STEP_SIZE); + self.set_volume(volume); } fn handle_volume_down(&mut self) { - let mut volume: i32 = self.device.get_volume() as i32 - 4096; - if volume < 0 { - volume = 0; - } - self.set_volume(volume as u16); + let volume = (self.device.get_volume() as u16).saturating_sub(VOLUME_STEP_SIZE); + self.set_volume(volume); } fn handle_end_of_track(&mut self) { @@ -1243,7 +1196,6 @@ impl SpircTask { None => { self.state.set_status(PlayStatus::kPlayStatusStop); self.player.stop(); - self.ensure_mixer_stopped(); self.play_status = SpircPlayStatus::Stopped; } } @@ -1273,8 +1225,7 @@ impl SpircTask { fn set_volume(&mut self, volume: u16) { self.device.set_volume(volume as u32); - self.mixer - .set_volume(volume_to_mixer(volume, &self.config.volume_ctrl)); + self.mixer.set_volume(volume); if let Some(cache) = self.session.cache() { cache.save_volume(volume) } diff --git a/core/src/config.rs b/core/src/config.rs index 9c70c25b..db990419 100644 --- a/core/src/config.rs +++ b/core/src/config.rs @@ -108,33 +108,7 @@ impl Default for DeviceType { pub struct ConnectConfig { pub name: String, pub device_type: DeviceType, - pub volume: u16, - pub volume_ctrl: VolumeCtrl, + pub initial_volume: Option, + pub has_volume_ctrl: bool, pub autoplay: bool, } - -#[derive(Clone, Debug)] -pub enum VolumeCtrl { - Linear, - Log, - Fixed, -} - -impl FromStr for VolumeCtrl { - type Err = (); - fn from_str(s: &str) -> Result { - use self::VolumeCtrl::*; - match s.to_lowercase().as_ref() { - "linear" => Ok(Linear), - "log" => Ok(Log), - "fixed" => Ok(Fixed), - _ => Err(()), - } - } -} - -impl Default for VolumeCtrl { - fn default() -> VolumeCtrl { - VolumeCtrl::Log - } -} diff --git a/playback/src/config.rs b/playback/src/config.rs index feb1d61e..9f8d97e1 100644 --- a/playback/src/config.rs +++ b/playback/src/config.rs @@ -1,4 +1,4 @@ -use super::player::NormalisationData; +use super::player::db_to_ratio; use crate::convert::i24; use std::convert::TryFrom; @@ -80,7 +80,7 @@ pub enum NormalisationType { impl FromStr for NormalisationType { type Err = (); fn from_str(s: &str) -> Result { - match s { + match s.to_lowercase().as_ref() { "album" => Ok(Self::Album), "track" => Ok(Self::Track), _ => Err(()), @@ -103,7 +103,7 @@ pub enum NormalisationMethod { impl FromStr for NormalisationMethod { type Err = (); fn from_str(s: &str) -> Result { - match s { + match s.to_lowercase().as_ref() { "basic" => Ok(Self::Basic), "dynamic" => Ok(Self::Dynamic), _ => Err(()), @@ -120,6 +120,7 @@ impl Default for NormalisationMethod { #[derive(Clone, Debug)] pub struct PlayerConfig { pub bitrate: Bitrate, + pub gapless: bool, pub normalisation: bool, pub normalisation_type: NormalisationType, pub normalisation_method: NormalisationMethod, @@ -128,7 +129,6 @@ pub struct PlayerConfig { pub normalisation_attack: f32, pub normalisation_release: f32, pub normalisation_knee: f32, - pub gapless: bool, pub passthrough: bool, } @@ -136,16 +136,56 @@ impl Default for PlayerConfig { fn default() -> PlayerConfig { PlayerConfig { bitrate: Bitrate::default(), + gapless: true, normalisation: false, normalisation_type: NormalisationType::default(), normalisation_method: NormalisationMethod::default(), normalisation_pregain: 0.0, - normalisation_threshold: NormalisationData::db_to_ratio(-1.0), + normalisation_threshold: db_to_ratio(-1.0), normalisation_attack: 0.005, normalisation_release: 0.1, normalisation_knee: 1.0, - gapless: true, passthrough: false, } } } + +// fields are intended for volume control range in dB +#[derive(Clone, Copy, Debug)] +pub enum VolumeCtrl { + Cubic(f32), + Fixed, + Linear, + Log(f32), +} + +impl FromStr for VolumeCtrl { + type Err = (); + fn from_str(s: &str) -> Result { + Self::from_str_with_range(s, Self::DEFAULT_DB_RANGE) + } +} + +impl Default for VolumeCtrl { + fn default() -> VolumeCtrl { + VolumeCtrl::Log(Self::DEFAULT_DB_RANGE) + } +} + +impl VolumeCtrl { + pub const MAX_VOLUME: u16 = std::u16::MAX; + + // Taken from: https://www.dr-lex.be/info-stuff/volumecontrols.html + pub const DEFAULT_DB_RANGE: f32 = 60.0; + + pub fn from_str_with_range(s: &str, db_range: f32) -> Result::Err> { + use self::VolumeCtrl::*; + match s.to_lowercase().as_ref() { + "cubic" => Ok(Cubic(db_range)), + "fixed" => Ok(Fixed), + "linear" => Ok(Linear), + "log" => Ok(Log(db_range)), + _ => Err(()), + } + } +} diff --git a/playback/src/mixer/alsamixer.rs b/playback/src/mixer/alsamixer.rs index 5e0a963f..62009184 100644 --- a/playback/src/mixer/alsamixer.rs +++ b/playback/src/mixer/alsamixer.rs @@ -1,218 +1,264 @@ -use super::AudioFilter; -use super::{Mixer, MixerConfig}; -use std::error::Error; +use crate::player::{db_to_ratio, ratio_to_db}; -const SND_CTL_TLV_DB_GAIN_MUTE: i64 = -9999999; +use super::mappings::{LogMapping, MappedCtrl, VolumeMapping}; +use super::{Mixer, MixerConfig, VolumeCtrl}; -#[derive(Clone)] -struct AlsaMixerVolumeParams { - min: i64, - max: i64, - range: f64, - min_db: alsa::mixer::MilliBel, - max_db: alsa::mixer::MilliBel, - has_switch: bool, -} +use alsa::ctl::{ElemId, ElemIface}; +use alsa::mixer::{MilliBel, SelemChannelId, SelemId}; +use alsa::{Ctl, Round}; + +use std::ffi::CString; #[derive(Clone)] pub struct AlsaMixer { config: MixerConfig, - params: AlsaMixerVolumeParams, + min: i64, + max: i64, + range: i64, + min_db: f32, + max_db: f32, + db_range: f32, + has_switch: bool, + is_softvol: bool, + use_linear_in_db: bool, } -impl AlsaMixer { - fn pvol(&self, vol: T, min: T, max: T) -> f64 - where - T: std::ops::Sub + Copy, - f64: std::convert::From<::Output>, - { - f64::from(vol - min) / f64::from(max - min) - } - - fn init_mixer(mut config: MixerConfig) -> Result> { - let mixer = alsa::mixer::Mixer::new(&config.card, false)?; - let sid = alsa::mixer::SelemId::new(&config.mixer, config.index); - - let selem = mixer.find_selem(&sid).unwrap_or_else(|| { - panic!( - "Couldn't find simple mixer control for {},{}", - &config.mixer, &config.index, - ) - }); - let (min, max) = selem.get_playback_volume_range(); - let (min_db, max_db) = selem.get_playback_db_range(); - let hw_mix = selem - .get_playback_vol_db(alsa::mixer::SelemChannelId::mono()) - .is_ok(); - let has_switch = selem.has_playback_switch(); - if min_db != alsa::mixer::MilliBel(SND_CTL_TLV_DB_GAIN_MUTE) { - warn!("Alsa min-db is not SND_CTL_TLV_DB_GAIN_MUTE!!"); - } - info!( - "Alsa Mixer info min: {} ({:?}[dB]) -- max: {} ({:?}[dB]) HW: {:?}", - min, min_db, max, max_db, hw_mix - ); - - if config.mapped_volume && (max_db - min_db <= alsa::mixer::MilliBel(24)) { - warn!( - "Switching to linear volume mapping, control range: {:?}", - max_db - min_db - ); - config.mapped_volume = false; - } else if !config.mapped_volume { - info!("Using Alsa linear volume"); - } - - if min_db != alsa::mixer::MilliBel(SND_CTL_TLV_DB_GAIN_MUTE) { - debug!("Alsa min-db is not SND_CTL_TLV_DB_GAIN_MUTE!!"); - } - - Ok(AlsaMixer { - config, - params: AlsaMixerVolumeParams { - min, - max, - range: (max - min) as f64, - min_db, - max_db, - has_switch, - }, - }) - } - - fn map_volume(&self, set_volume: Option) -> Result> { - let mixer = alsa::mixer::Mixer::new(&self.config.card, false)?; - let sid = alsa::mixer::SelemId::new(&*self.config.mixer, self.config.index); - - let selem = mixer.find_selem(&sid).unwrap(); - let cur_vol = selem - .get_playback_volume(alsa::mixer::SelemChannelId::mono()) - .expect("Couldn't get current volume"); - let cur_vol_db = selem - .get_playback_vol_db(alsa::mixer::SelemChannelId::mono()) - .unwrap_or(alsa::mixer::MilliBel(-SND_CTL_TLV_DB_GAIN_MUTE)); - - let mut new_vol: u16 = 0; - trace!("Current alsa volume: {}{:?}", cur_vol, cur_vol_db); - - match set_volume { - Some(vol) => { - if self.params.has_switch { - let is_muted = selem - .get_playback_switch(alsa::mixer::SelemChannelId::mono()) - .map(|b| b == 0) - .unwrap_or(false); - if vol == 0 { - debug!("Toggling mute::True"); - selem.set_playback_switch_all(0).expect("Can't switch mute"); - - return Ok(vol); - } else if is_muted { - debug!("Toggling mute::False"); - selem.set_playback_switch_all(1).expect("Can't reset mute"); - } - } - - if self.config.mapped_volume { - // Cubic mapping ala alsamixer - // https://linux.die.net/man/1/alsamixer - // In alsamixer, the volume is mapped to a value that is more natural for a - // human ear. The mapping is designed so that the position in the interval is - // proportional to the volume as a human ear would perceive it, i.e. the - // position is the cubic root of the linear sample multiplication factor. For - // controls with a small range (24 dB or less), the mapping is linear in the dB - // values so that each step has the same size visually. TODO - // TODO: Check if min is not mute! - let vol_db = (self.pvol(vol, 0x0000, 0xFFFF).log10() * 6000.0).floor() as i64 - + self.params.max_db.0; - selem - .set_playback_db_all(alsa::mixer::MilliBel(vol_db), alsa::Round::Floor) - .expect("Couldn't set alsa dB volume"); - debug!( - "Mapping volume [{:.3}%] {:?} [u16] ->> Alsa [{:.3}%] {:?} [dB] - {} [i64]", - self.pvol(vol, 0x0000, 0xFFFF) * 100.0, - vol, - self.pvol( - vol_db as f64, - self.params.min as f64, - self.params.max as f64 - ) * 100.0, - vol_db as f64 / 100.0, - vol_db - ); - } else { - // Linear mapping - let alsa_volume = - ((vol as f64 / 0xFFFF as f64) * self.params.range) as i64 + self.params.min; - selem - .set_playback_volume_all(alsa_volume) - .expect("Couldn't set alsa raw volume"); - debug!( - "Mapping volume [{:.3}%] {:?} [u16] ->> Alsa [{:.3}%] {:?} [i64]", - self.pvol(vol, 0x0000, 0xFFFF) * 100.0, - vol, - self.pvol( - alsa_volume as f64, - self.params.min as f64, - self.params.max as f64 - ) * 100.0, - alsa_volume - ); - }; - } - None => { - new_vol = (((cur_vol - self.params.min) as f64 / self.params.range) * 0xFFFF as f64) - as u16; - debug!( - "Mapping volume [{:.3}%] {:?} [u16] <<- Alsa [{:.3}%] {:?} [i64]", - self.pvol(new_vol, 0x0000, 0xFFFF), - new_vol, - self.pvol( - cur_vol as f64, - self.params.min as f64, - self.params.max as f64 - ), - cur_vol - ); - } - } - - Ok(new_vol) - } -} +// min_db cannot be depended on to be mute. Also note that contrary to +// its name copied verbatim from Alsa, this is in millibel scale. +const SND_CTL_TLV_DB_GAIN_MUTE: MilliBel = MilliBel(-9999999); +const ZERO_DB: MilliBel = MilliBel(0); impl Mixer for AlsaMixer { - fn open(config: Option) -> AlsaMixer { - let config = config.unwrap_or_default(); + fn open(config: MixerConfig) -> Self { info!( - "Setting up new mixer: card:{} mixer:{} index:{}", - config.card, config.mixer, config.index + "Mixing with alsa and volume control: {:?} for card: {} with mixer control: {},{}", + config.volume_ctrl, config.card, config.control, config.index, ); - AlsaMixer::init_mixer(config).expect("Error setting up mixer!") + + let mut config = config; // clone + + let mixer = + alsa::mixer::Mixer::new(&config.card, false).expect("Could not open Alsa mixer"); + let simple_element = mixer + .find_selem(&SelemId::new(&config.control, config.index)) + .expect("Could not find Alsa mixer control"); + + // Query capabilities + let has_switch = simple_element.has_playback_switch(); + let is_softvol = simple_element + .get_playback_vol_db(SelemChannelId::mono()) + .is_err(); + + // Query raw volume range + let (min, max) = simple_element.get_playback_volume_range(); + let range = i64::abs(max - min); + + // Query dB volume range -- note that Alsa exposes a different + // API for hardware and software mixers + let (min_millibel, max_millibel) = if is_softvol { + let control = + Ctl::new(&config.card, false).expect("Could not open Alsa softvol with that card"); + let mut element_id = ElemId::new(ElemIface::Mixer); + element_id.set_name( + &CString::new(config.control.as_str()) + .expect("Could not open Alsa softvol with that name"), + ); + element_id.set_index(config.index); + let (min_millibel, mut max_millibel) = control + .get_db_range(&element_id) + .expect("Could not get Alsa softvol dB range"); + + // Alsa can report incorrect maximum volumes due to rounding + // errors. e.g. Alsa rounds [-60.0..0.0] in range [0..255] to + // step size 0.23. Then multiplying 0.23 by 255 incorrectly + // returns a dB range of 58.65 instead of 60 dB, from + // [-60.00..-1.35]. This workaround checks the default case + // where the maximum dB volume is expected to be 0, and cannot + // cover all cases. + if max_millibel != ZERO_DB { + warn!("Alsa mixer reported maximum dB != 0, which is suspect"); + let reported_step_size = (max_millibel - min_millibel).0 / range; + let assumed_step_size = (ZERO_DB - min_millibel).0 / range; + if reported_step_size == assumed_step_size { + warn!("Alsa rounding error detected, setting maximum dB to {:.2} instead of {:.2}", ZERO_DB.to_db(), max_millibel.to_db()); + max_millibel = ZERO_DB; + } else { + warn!("Please manually set with `--volume-ctrl` if this is incorrect"); + } + } + (min_millibel, max_millibel) + } else { + let (mut min_millibel, max_millibel) = simple_element.get_playback_db_range(); + + // Some controls report that their minimum volume is mute, instead + // of their actual lowest dB setting before that. + if min_millibel == SND_CTL_TLV_DB_GAIN_MUTE && min < max { + debug!("Alsa mixer reported minimum dB as mute, trying workaround"); + min_millibel = simple_element + .ask_playback_vol_db(min + 1) + .expect("Could not convert Alsa raw volume to dB volume"); + } + (min_millibel, max_millibel) + }; + + let min_db = min_millibel.to_db(); + let max_db = max_millibel.to_db(); + let db_range = f32::abs(max_db - min_db); + + // Synchronize the volume control dB range with the mixer control, + // unless it was already set with a command line option. + if !config.volume_ctrl.range_ok() { + config.volume_ctrl.set_db_range(db_range); + } + + // For hardware controls with a small range (24 dB or less), + // force using the dB API with a linear mapping. + let mut use_linear_in_db = false; + if !is_softvol && db_range <= 24.0 { + use_linear_in_db = true; + config.volume_ctrl = VolumeCtrl::Linear; + } + + debug!("Alsa mixer control is softvol: {}", is_softvol); + debug!("Alsa support for playback (mute) switch: {}", has_switch); + debug!("Alsa raw volume range: [{}..{}] ({})", min, max, range); + debug!( + "Alsa dB volume range: [{:.2}..{:.2}] ({:.2})", + min_db, max_db, db_range + ); + debug!("Alsa forcing linear dB mapping: {}", use_linear_in_db); + + Self { + config, + min, + max, + range, + min_db, + max_db, + db_range, + has_switch, + is_softvol, + use_linear_in_db, + } } - fn start(&self) {} - - fn stop(&self) {} - fn volume(&self) -> u16 { - match self.map_volume(None) { - Ok(vol) => vol, - Err(e) => { - error!("Error getting volume for <{}>, {:?}", self.config.card, e); - 0 - } + let mixer = + alsa::mixer::Mixer::new(&self.config.card, false).expect("Could not open Alsa mixer"); + let simple_element = mixer + .find_selem(&SelemId::new(&self.config.control, self.config.index)) + .expect("Could not find Alsa mixer control"); + + if self.switched_off() { + return 0; } + + let mut mapped_volume = if self.is_softvol { + let raw_volume = simple_element + .get_playback_volume(SelemChannelId::mono()) + .expect("Could not get raw Alsa volume"); + + raw_volume as f32 / self.range as f32 - self.min as f32 + } else { + let db_volume = simple_element + .get_playback_vol_db(SelemChannelId::mono()) + .expect("Could not get Alsa dB volume") + .to_db(); + + if self.use_linear_in_db { + (db_volume - self.min_db) / self.db_range + } else if f32::abs(db_volume - SND_CTL_TLV_DB_GAIN_MUTE.to_db()) <= f32::EPSILON { + 0.0 + } else { + db_to_ratio(db_volume - self.max_db) + } + }; + + // see comment in `set_volume` why we are handling an antilog volume + if mapped_volume > 0.0 && self.is_some_linear() { + mapped_volume = LogMapping::linear_to_mapped(mapped_volume, self.db_range); + } + + self.config.volume_ctrl.from_mapped(mapped_volume) } fn set_volume(&self, volume: u16) { - match self.map_volume(Some(volume)) { - Ok(_) => (), - Err(e) => error!("Error setting volume for <{}>, {:?}", self.config.card, e), - } - } + let mixer = + alsa::mixer::Mixer::new(&self.config.card, false).expect("Could not open Alsa mixer"); + let simple_element = mixer + .find_selem(&SelemId::new(&self.config.control, self.config.index)) + .expect("Could not find Alsa mixer control"); - fn get_audio_filter(&self) -> Option> { - None + if self.has_switch { + if volume == 0 { + debug!("Disabling playback (setting mute) on Alsa"); + simple_element + .set_playback_switch_all(0) + .expect("Could not disable playback (set mute) on Alsa"); + } else if self.switched_off() { + debug!("Enabling playback (unsetting mute) on Alsa"); + simple_element + .set_playback_switch_all(1) + .expect("Could not enable playback (unset mute) on Alsa"); + } + } + + let mut mapped_volume = self.config.volume_ctrl.to_mapped(volume); + + // Alsa's linear algorithms map everything onto log. Alsa softvol does + // this internally. In the case of `use_linear_in_db` this happens + // automatically by virtue of the dB scale. This means that linear + // controls become log, log becomes log-on-log, and so on. To make + // the controls work as expected, perform an antilog calculation to + // counteract what Alsa will be doing to the set volume. + if mapped_volume > 0.0 && self.is_some_linear() { + mapped_volume = LogMapping::mapped_to_linear(mapped_volume, self.db_range); + } + + if self.is_softvol { + let scaled_volume = (self.min as f32 + mapped_volume * self.range as f32) as i64; + debug!("Setting Alsa raw volume to {}", scaled_volume); + simple_element + .set_playback_volume_all(scaled_volume) + .expect("Could not set Alsa raw volume"); + return; + } + + let db_volume = if self.use_linear_in_db { + self.min_db + mapped_volume * self.db_range + } else if volume == 0 { + // prevent ratio_to_db(0.0) from returning -inf + SND_CTL_TLV_DB_GAIN_MUTE.to_db() + } else { + ratio_to_db(mapped_volume) + self.max_db + }; + + debug!("Setting Alsa volume to {:.2} dB", db_volume); + simple_element + .set_playback_db_all(MilliBel::from_db(db_volume), Round::Floor) + .expect("Could not set Alsa dB volume"); + } +} + +impl AlsaMixer { + fn switched_off(&self) -> bool { + if !self.has_switch { + return false; + } + + let mixer = + alsa::mixer::Mixer::new(&self.config.card, false).expect("Could not open Alsa mixer"); + let simple_element = mixer + .find_selem(&SelemId::new(&self.config.control, self.config.index)) + .expect("Could not find Alsa mixer control"); + + simple_element + .get_playback_switch(SelemChannelId::mono()) + .map(|playback| playback == 0) + .unwrap_or(false) + } + + fn is_some_linear(&self) -> bool { + self.is_softvol || self.use_linear_in_db } } diff --git a/playback/src/mixer/mappings.rs b/playback/src/mixer/mappings.rs new file mode 100644 index 00000000..6a274442 --- /dev/null +++ b/playback/src/mixer/mappings.rs @@ -0,0 +1,163 @@ +use super::VolumeCtrl; +use crate::player::db_to_ratio; + +pub trait MappedCtrl { + fn to_mapped(&self, volume: u16) -> f32; + fn from_mapped(&self, mapped_volume: f32) -> u16; + + fn db_range(&self) -> f32; + fn set_db_range(&mut self, new_db_range: f32); + fn range_ok(&self) -> bool; +} + +impl MappedCtrl for VolumeCtrl { + fn to_mapped(&self, volume: u16) -> f32 { + // More than just an optimization, this ensures that zero volume is + // really mute (both the log and cubic equations would otherwise not + // reach zero). + if volume == 0 { + return 0.0; + } else if volume == 1 { + // And limit in case of rounding errors (as is the case for log). + return 1.0; + } + + let normalized_volume = volume as f32 / Self::MAX_VOLUME as f32; + let mapped_volume = if self.range_ok() { + match *self { + Self::Cubic(db_range) => { + CubicMapping::linear_to_mapped(normalized_volume, db_range) + } + Self::Log(db_range) => LogMapping::linear_to_mapped(normalized_volume, db_range), + _ => normalized_volume, + } + } else { + // Ensure not to return -inf or NaN due to division by zero. + error!( + "{:?} does not work with 0 dB range, using linear mapping instead", + self + ); + normalized_volume + }; + + debug!( + "Input volume {} mapped to: {:.2}%", + volume, + mapped_volume * 100.0 + ); + + mapped_volume + } + + fn from_mapped(&self, mapped_volume: f32) -> u16 { + // More than just an optimization, this ensures that zero mapped volume + // is unmapped to non-negative real numbers (otherwise the log and cubic + // equations would respectively return -inf and -1/9.) + if f32::abs(mapped_volume - 0.0) <= f32::EPSILON { + return 0; + } else if f32::abs(mapped_volume - 1.0) <= f32::EPSILON { + return Self::MAX_VOLUME; + } + + let unmapped_volume = if self.range_ok() { + match *self { + Self::Cubic(db_range) => CubicMapping::mapped_to_linear(mapped_volume, db_range), + Self::Log(db_range) => LogMapping::mapped_to_linear(mapped_volume, db_range), + _ => mapped_volume, + } + } else { + // Ensure not to return -inf or NaN due to division by zero. + error!( + "{:?} does not work with 0 dB range, using linear mapping instead", + self + ); + mapped_volume + }; + + (unmapped_volume * Self::MAX_VOLUME as f32) as u16 + } + + fn db_range(&self) -> f32 { + match *self { + Self::Fixed => 0.0, + Self::Linear => Self::DEFAULT_DB_RANGE, // arbitrary, could be anything > 0 + Self::Log(db_range) | Self::Cubic(db_range) => db_range, + } + } + + fn set_db_range(&mut self, new_db_range: f32) { + match self { + Self::Cubic(ref mut db_range) | Self::Log(ref mut db_range) => *db_range = new_db_range, + _ => error!("Invalid to set dB range for volume control type {:?}", self), + } + + debug!("Volume control is now {:?}", self) + } + + fn range_ok(&self) -> bool { + self.db_range() > 0.0 || matches!(self, Self::Fixed | Self::Linear) + } +} + +pub trait VolumeMapping { + fn linear_to_mapped(unmapped_volume: f32, db_range: f32) -> f32; + fn mapped_to_linear(mapped_volume: f32, db_range: f32) -> f32; +} + +// Volume conversion taken from: https://www.dr-lex.be/info-stuff/volumecontrols.html#ideal2 +// +// As the human auditory system has a logarithmic sensitivity curve, this +// mapping results in a near linear loudness experience with the listener. +pub struct LogMapping {} +impl VolumeMapping for LogMapping { + fn linear_to_mapped(normalized_volume: f32, db_range: f32) -> f32 { + let (db_ratio, ideal_factor) = Self::coefficients(db_range); + f32::exp(ideal_factor * normalized_volume) / db_ratio + } + + fn mapped_to_linear(mapped_volume: f32, db_range: f32) -> f32 { + let (db_ratio, ideal_factor) = Self::coefficients(db_range); + f32::ln(db_ratio * mapped_volume) / ideal_factor + } +} + +impl LogMapping { + fn coefficients(db_range: f32) -> (f32, f32) { + let db_ratio = db_to_ratio(db_range); + let ideal_factor = f32::ln(db_ratio); + (db_ratio, ideal_factor) + } +} + +// Ported from: https://github.com/alsa-project/alsa-utils/blob/master/alsamixer/volume_mapping.c +// which in turn was inspired by: https://www.robotplanet.dk/audio/audio_gui_design/ +// +// Though this mapping is computationally less expensive than the logarithmic +// mapping, it really does not matter as librespot memoizes the mapped value. +// Use this mapping if you have some reason to mimic Alsa's native mixer or +// prefer a more granular control in the upper volume range. +// +// Note: https://www.dr-lex.be/info-stuff/volumecontrols.html#ideal3 shows +// better approximations to the logarithmic curve but because we only intend +// to mimic Alsa here, we do not implement them. If your desire is to use a +// logarithmic mapping, then use that volume control. +pub struct CubicMapping {} +impl VolumeMapping for CubicMapping { + fn linear_to_mapped(normalized_volume: f32, db_range: f32) -> f32 { + let min_norm = Self::min_norm(db_range); + f32::powi(normalized_volume * (1.0 - min_norm) + min_norm, 3) + } + + fn mapped_to_linear(mapped_volume: f32, db_range: f32) -> f32 { + let min_norm = Self::min_norm(db_range); + (mapped_volume.powf(1.0 / 3.0) - min_norm) / (1.0 - min_norm) + } +} + +impl CubicMapping { + fn min_norm(db_range: f32) -> f32 { + // Note that this 60.0 is unrelated to DEFAULT_DB_RANGE. + // Instead, it's the cubic voltage to dB ratio. + f32::powf(10.0, -1.0 * db_range / 60.0) + } +} diff --git a/playback/src/mixer/mod.rs b/playback/src/mixer/mod.rs index af41c6f4..3c3bed2e 100644 --- a/playback/src/mixer/mod.rs +++ b/playback/src/mixer/mod.rs @@ -1,11 +1,16 @@ +use crate::config::VolumeCtrl; + +pub mod mappings; +use self::mappings::MappedCtrl; + pub trait Mixer: Send { - fn open(_: Option) -> Self + fn open(config: MixerConfig) -> Self where Self: Sized; - fn start(&self); - fn stop(&self); + fn set_volume(&self, volume: u16); fn volume(&self) -> u16; + fn get_audio_filter(&self) -> Option> { None } @@ -15,6 +20,9 @@ pub trait AudioFilter { fn modify_stream(&self, data: &mut [f32]); } +pub mod softmixer; +use self::softmixer::SoftMixer; + #[cfg(feature = "alsa-backend")] pub mod alsamixer; #[cfg(feature = "alsa-backend")] @@ -23,29 +31,26 @@ use self::alsamixer::AlsaMixer; #[derive(Debug, Clone)] pub struct MixerConfig { pub card: String, - pub mixer: String, + pub control: String, pub index: u32, - pub mapped_volume: bool, + pub volume_ctrl: VolumeCtrl, } impl Default for MixerConfig { fn default() -> MixerConfig { MixerConfig { card: String::from("default"), - mixer: String::from("PCM"), + control: String::from("PCM"), index: 0, - mapped_volume: true, + volume_ctrl: VolumeCtrl::default(), } } } -pub mod softmixer; -use self::softmixer::SoftMixer; +pub type MixerFn = fn(MixerConfig) -> Box; -type MixerFn = fn(Option) -> Box; - -fn mk_sink(device: Option) -> Box { - Box::new(M::open(device)) +fn mk_sink(config: MixerConfig) -> Box { + Box::new(M::open(config)) } pub fn find>(name: Option) -> Option { diff --git a/playback/src/mixer/softmixer.rs b/playback/src/mixer/softmixer.rs index ec8ed6b2..0be161ad 100644 --- a/playback/src/mixer/softmixer.rs +++ b/playback/src/mixer/softmixer.rs @@ -1,28 +1,40 @@ -use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::atomic::{AtomicU32, Ordering}; use std::sync::Arc; use super::AudioFilter; +use super::{MappedCtrl, VolumeCtrl}; use super::{Mixer, MixerConfig}; #[derive(Clone)] pub struct SoftMixer { - volume: Arc, + // There is no AtomicF32, so we store the f32 as bits in a u32 field. + // It's much faster than a Mutex. + volume: Arc, + volume_ctrl: VolumeCtrl, } impl Mixer for SoftMixer { - fn open(_: Option) -> SoftMixer { - SoftMixer { - volume: Arc::new(AtomicUsize::new(0xFFFF)), + fn open(config: MixerConfig) -> Self { + let volume_ctrl = config.volume_ctrl; + info!("Mixing with softvol and volume control: {:?}", volume_ctrl); + + Self { + volume: Arc::new(AtomicU32::new(f32::to_bits(0.5))), + volume_ctrl, } } - fn start(&self) {} - fn stop(&self) {} + fn volume(&self) -> u16 { - self.volume.load(Ordering::Relaxed) as u16 + let mapped_volume = f32::from_bits(self.volume.load(Ordering::Relaxed)); + self.volume_ctrl.from_mapped(mapped_volume) } + fn set_volume(&self, volume: u16) { - self.volume.store(volume as usize, Ordering::Relaxed); + let mapped_volume = self.volume_ctrl.to_mapped(volume); + self.volume + .store(mapped_volume.to_bits(), Ordering::Relaxed) } + fn get_audio_filter(&self) -> Option> { Some(Box::new(SoftVolumeApplier { volume: self.volume.clone(), @@ -31,16 +43,15 @@ impl Mixer for SoftMixer { } struct SoftVolumeApplier { - volume: Arc, + volume: Arc, } impl AudioFilter for SoftVolumeApplier { fn modify_stream(&self, data: &mut [f32]) { - let volume = self.volume.load(Ordering::Relaxed) as u16; - if volume != 0xFFFF { - let volume_factor = volume as f64 / 0xFFFF as f64; + let volume = f32::from_bits(self.volume.load(Ordering::Relaxed)); + if volume < 1.0 { for x in data.iter_mut() { - *x = (*x as f64 * volume_factor) as f32; + *x = (*x as f64 * volume as f64) as f32; } } } diff --git a/playback/src/player.rs b/playback/src/player.rs index 8cbb4372..659804f8 100644 --- a/playback/src/player.rs +++ b/playback/src/player.rs @@ -30,7 +30,7 @@ pub const NUM_CHANNELS: u8 = 2; pub const SAMPLES_PER_SECOND: u32 = SAMPLE_RATE as u32 * NUM_CHANNELS as u32; const PRELOAD_NEXT_TRACK_BEFORE_END_DURATION_MS: u32 = 30000; -const DB_VOLTAGE_RATIO: f32 = 20.0; +pub const DB_VOLTAGE_RATIO: f32 = 20.0; pub struct Player { commands: Option>, @@ -196,6 +196,14 @@ impl PlayerEvent { pub type PlayerEventChannel = mpsc::UnboundedReceiver; +pub fn db_to_ratio(db: f32) -> f32 { + f32::powf(10.0, db / DB_VOLTAGE_RATIO) +} + +pub fn ratio_to_db(ratio: f32) -> f32 { + ratio.log10() * DB_VOLTAGE_RATIO +} + #[derive(Clone, Copy, Debug)] pub struct NormalisationData { track_gain_db: f32, @@ -205,14 +213,6 @@ pub struct NormalisationData { } impl NormalisationData { - pub fn db_to_ratio(db: f32) -> f32 { - f32::powf(10.0, db / DB_VOLTAGE_RATIO) - } - - pub fn ratio_to_db(ratio: f32) -> f32 { - ratio.log10() * DB_VOLTAGE_RATIO - } - fn parse_from_file(mut file: T) -> io::Result { const SPOTIFY_NORMALIZATION_HEADER_START_OFFSET: u64 = 144; file.seek(SeekFrom::Start(SPOTIFY_NORMALIZATION_HEADER_START_OFFSET))?; @@ -243,11 +243,11 @@ impl NormalisationData { }; let normalisation_power = gain_db + config.normalisation_pregain; - let mut normalisation_factor = Self::db_to_ratio(normalisation_power); + let mut normalisation_factor = db_to_ratio(normalisation_power); if normalisation_factor * gain_peak > config.normalisation_threshold { let limited_normalisation_factor = config.normalisation_threshold / gain_peak; - let limited_normalisation_power = Self::ratio_to_db(limited_normalisation_factor); + let limited_normalisation_power = ratio_to_db(limited_normalisation_factor); if config.normalisation_method == NormalisationMethod::Basic { warn!("Limiting gain to {:.2} dB for the duration of this track to stay under normalisation threshold.", limited_normalisation_power); @@ -266,7 +266,7 @@ impl NormalisationData { debug!("Normalisation Type: {:?}", config.normalisation_type); debug!( "Normalisation Threshold: {:.1}", - Self::ratio_to_db(config.normalisation_threshold) + ratio_to_db(config.normalisation_threshold) ); debug!("Normalisation Method: {:?}", config.normalisation_method); debug!("Normalisation Factor: {}", normalisation_factor); diff --git a/src/main.rs b/src/main.rs index a5106af2..6f41db9f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9,15 +9,16 @@ use url::Url; use librespot::connect::spirc::Spirc; use librespot::core::authentication::Credentials; use librespot::core::cache::Cache; -use librespot::core::config::{ConnectConfig, DeviceType, SessionConfig, VolumeCtrl}; +use librespot::core::config::{ConnectConfig, DeviceType, SessionConfig}; use librespot::core::session::Session; use librespot::core::version; -use librespot::playback::audio_backend::{self, Sink, BACKENDS}; +use librespot::playback::audio_backend::{self, SinkBuilder, BACKENDS}; use librespot::playback::config::{ - AudioFormat, Bitrate, NormalisationMethod, NormalisationType, PlayerConfig, + AudioFormat, Bitrate, NormalisationMethod, NormalisationType, PlayerConfig, VolumeCtrl, }; -use librespot::playback::mixer::{self, Mixer, MixerConfig}; -use librespot::playback::player::{NormalisationData, Player}; +use librespot::playback::mixer::mappings::MappedCtrl; +use librespot::playback::mixer::{self, MixerConfig, MixerFn}; +use librespot::playback::player::{db_to_ratio, Player}; mod player_event_handler; use player_event_handler::{emit_sink_event, run_program_on_events}; @@ -66,7 +67,7 @@ fn setup_logging(verbose: bool) { } fn list_backends() { - println!("Available Backends : "); + println!("Available backends : "); for (&(name, _), idx) in BACKENDS.iter().zip(0..) { if idx == 0 { println!("- {} (default)", name); @@ -172,11 +173,9 @@ fn print_version() { #[derive(Clone)] struct Setup { format: AudioFormat, - backend: fn(Option, AudioFormat) -> Box, + backend: SinkBuilder, device: Option, - - mixer: fn(Option) -> Box, - + mixer: MixerFn, cache: Option, player_config: PlayerConfig, session_config: SessionConfig, @@ -266,11 +265,6 @@ fn get_setup(args: &[String]) -> Setup { "Alsa mixer index, Index of the cards mixer. Defaults to 0", "MIXER_INDEX", ) - .optflag( - "", - "mixer-linear-volume", - "Disable alsa's mapped volume scale (cubic). Default false", - ) .optopt( "", "initial-volume", @@ -333,8 +327,14 @@ fn get_setup(args: &[String]) -> Setup { .optopt( "", "volume-ctrl", - "Volume control type - [linear, log, fixed]. Default is logarithmic", + "Volume control type - [cubic, fixed, linear, log]. Default is log.", "VOLUME_CTRL" + ) + .optopt( + "", + "volume-range", + "Range of the volume control (dB). Defaults to 60 for softvol and for alsa what the mixer supports.", + "RANGE", ) .optflag( "", @@ -399,18 +399,55 @@ fn get_setup(args: &[String]) -> Setup { let mixer_name = matches.opt_str("mixer"); let mixer = mixer::find(mixer_name.as_ref()).expect("Invalid mixer"); - let mixer_config = MixerConfig { - card: matches - .opt_str("mixer-card") - .unwrap_or_else(|| String::from("default")), - mixer: matches - .opt_str("mixer-name") - .unwrap_or_else(|| String::from("PCM")), - index: matches + let mixer_config = { + let card = matches.opt_str("mixer-card").unwrap_or_else(|| { + if let Some(ref device_name) = device { + device_name.to_string() + } else { + String::from("default") + } + }); + let index = matches .opt_str("mixer-index") .map(|index| index.parse::().unwrap()) - .unwrap_or(0), - mapped_volume: !matches.opt_present("mixer-linear-volume"), + .unwrap_or(0); + let control = matches + .opt_str("mixer-name") + .unwrap_or_else(|| String::from("PCM")); + let mut volume_range = matches + .opt_str("volume-range") + .map(|range| range.parse::().unwrap()) + .unwrap_or_else(|| match mixer_name.as_ref().map(AsRef::as_ref) { + Some("alsa") => 0.0, // let Alsa query the control + _ => VolumeCtrl::DEFAULT_DB_RANGE, + }); + if volume_range < 0.0 { + // User might have specified range as minimum dB volume. + volume_range *= -1.0; + warn!( + "Please enter positive volume ranges only, assuming {:.2} dB", + volume_range + ); + } + let volume_ctrl = matches + .opt_str("volume-ctrl") + .as_ref() + .map(|volume_ctrl| { + VolumeCtrl::from_str_with_range(volume_ctrl, volume_range) + .expect("Invalid volume control type") + }) + .unwrap_or_else(|| { + let mut volume_ctrl = VolumeCtrl::default(); + volume_ctrl.set_db_range(volume_range); + volume_ctrl + }); + + MixerConfig { + card, + control, + index, + volume_ctrl, + } }; let cache = { @@ -459,15 +496,18 @@ fn get_setup(args: &[String]) -> Setup { let initial_volume = matches .opt_str("initial-volume") - .map(|volume| { - let volume = volume.parse::().unwrap(); + .map(|initial_volume| { + let volume = initial_volume.parse::().unwrap(); if volume > 100 { - panic!("Initial volume must be in the range 0-100"); + error!("Initial volume must be in the range 0-100."); + // the cast will saturate, not necessary to take further action } - (volume as i32 * 0xFFFF / 100) as u16 + (volume as f32 / 100.0 * VolumeCtrl::MAX_VOLUME as f32) as u16 }) - .or_else(|| cache.as_ref().and_then(Cache::volume)) - .unwrap_or(0x8000); + .or_else(|| match mixer_name.as_ref().map(AsRef::as_ref) { + Some("alsa") => None, + _ => cache.as_ref().and_then(Cache::volume), + }); let zeroconf_port = matches .opt_str("zeroconf-port") @@ -506,15 +546,15 @@ fn get_setup(args: &[String]) -> Setup { match Url::parse(&s) { Ok(url) => { if url.host().is_none() || url.port_or_known_default().is_none() { - panic!("Invalid proxy url, only urls on the format \"http://host:port\" are allowed"); + panic!("Invalid proxy url, only URLs on the format \"http://host:port\" are allowed"); } if url.scheme() != "http" { - panic!("Only unsecure http:// proxies are supported"); + panic!("Only insecure http:// proxies are supported"); } url }, - Err(err) => panic!("Invalid proxy url: {}, only urls on the format \"http://host:port\" are allowed", err) + Err(err) => panic!("Invalid proxy URL: {}, only URLs in the format \"http://host:port\" are allowed", err) } }, ), @@ -524,21 +564,14 @@ fn get_setup(args: &[String]) -> Setup { } }; - let passthrough = matches.opt_present("passthrough"); - let player_config = { let bitrate = matches .opt_str("b") .as_ref() .map(|bitrate| Bitrate::from_str(bitrate).expect("Invalid bitrate")) .unwrap_or_default(); - let gain_type = matches - .opt_str("normalisation-gain-type") - .as_ref() - .map(|gain_type| { - NormalisationType::from_str(gain_type).expect("Invalid normalisation type") - }) - .unwrap_or_default(); + let gapless = !matches.opt_present("disable-gapless"); + let normalisation = matches.opt_present("enable-volume-normalisation"); let normalisation_method = matches .opt_str("normalisation-method") .as_ref() @@ -546,41 +579,52 @@ fn get_setup(args: &[String]) -> Setup { NormalisationMethod::from_str(gain_type).expect("Invalid normalisation method") }) .unwrap_or_default(); + let normalisation_type = matches + .opt_str("normalisation-gain-type") + .as_ref() + .map(|gain_type| { + NormalisationType::from_str(gain_type).expect("Invalid normalisation type") + }) + .unwrap_or_default(); + let normalisation_pregain = matches + .opt_str("normalisation-pregain") + .map(|pregain| pregain.parse::().expect("Invalid pregain float value")) + .unwrap_or(PlayerConfig::default().normalisation_pregain); + let normalisation_threshold = matches + .opt_str("normalisation-threshold") + .map(|threshold| { + db_to_ratio( + threshold + .parse::() + .expect("Invalid threshold float value"), + ) + }) + .unwrap_or(PlayerConfig::default().normalisation_threshold); + let normalisation_attack = matches + .opt_str("normalisation-attack") + .map(|attack| attack.parse::().expect("Invalid attack float value") / MILLIS) + .unwrap_or(PlayerConfig::default().normalisation_attack); + let normalisation_release = matches + .opt_str("normalisation-release") + .map(|release| release.parse::().expect("Invalid release float value") / MILLIS) + .unwrap_or(PlayerConfig::default().normalisation_release); + let normalisation_knee = matches + .opt_str("normalisation-knee") + .map(|knee| knee.parse::().expect("Invalid knee float value")) + .unwrap_or(PlayerConfig::default().normalisation_knee); + let passthrough = matches.opt_present("passthrough"); PlayerConfig { bitrate, - gapless: !matches.opt_present("disable-gapless"), - normalisation: matches.opt_present("enable-volume-normalisation"), + gapless, + normalisation, + normalisation_type, normalisation_method, - normalisation_type: gain_type, - normalisation_pregain: matches - .opt_str("normalisation-pregain") - .map(|pregain| pregain.parse::().expect("Invalid pregain float value")) - .unwrap_or(PlayerConfig::default().normalisation_pregain), - normalisation_threshold: matches - .opt_str("normalisation-threshold") - .map(|threshold| { - NormalisationData::db_to_ratio( - threshold - .parse::() - .expect("Invalid threshold float value"), - ) - }) - .unwrap_or(PlayerConfig::default().normalisation_threshold), - normalisation_attack: matches - .opt_str("normalisation-attack") - .map(|attack| attack.parse::().expect("Invalid attack float value") / MILLIS) - .unwrap_or(PlayerConfig::default().normalisation_attack), - normalisation_release: matches - .opt_str("normalisation-release") - .map(|release| { - release.parse::().expect("Invalid release float value") / MILLIS - }) - .unwrap_or(PlayerConfig::default().normalisation_release), - normalisation_knee: matches - .opt_str("normalisation-knee") - .map(|knee| knee.parse::().expect("Invalid knee float value")) - .unwrap_or(PlayerConfig::default().normalisation_knee), + normalisation_pregain, + normalisation_threshold, + normalisation_attack, + normalisation_release, + normalisation_knee, passthrough, } }; @@ -591,39 +635,37 @@ fn get_setup(args: &[String]) -> Setup { .as_ref() .map(|device_type| DeviceType::from_str(device_type).expect("Invalid device type")) .unwrap_or_default(); - - let volume_ctrl = matches - .opt_str("volume-ctrl") - .as_ref() - .map(|volume_ctrl| VolumeCtrl::from_str(volume_ctrl).expect("Invalid volume ctrl type")) - .unwrap_or_default(); + let has_volume_ctrl = !matches!(mixer_config.volume_ctrl, VolumeCtrl::Fixed); + let autoplay = matches.opt_present("autoplay"); ConnectConfig { name, device_type, - volume: initial_volume, - volume_ctrl, - autoplay: matches.opt_present("autoplay"), + initial_volume, + has_volume_ctrl, + autoplay, } }; let enable_discovery = !matches.opt_present("disable-discovery"); + let player_event_program = matches.opt_str("onevent"); + let emit_sink_events = matches.opt_present("emit-sink-events"); Setup { format, backend, - cache, - session_config, - player_config, - connect_config, - credentials, device, + mixer, + cache, + player_config, + session_config, + connect_config, + mixer_config, + credentials, enable_discovery, zeroconf_port, - mixer, - mixer_config, - player_event_program: matches.opt_str("onevent"), - emit_sink_events: matches.opt_present("emit-sink-events"), + player_event_program, + emit_sink_events, } } @@ -697,7 +739,7 @@ async fn main() { session = &mut connecting, if !connecting.is_terminated() => match session { Ok(session) => { let mixer_config = setup.mixer_config.clone(); - let mixer = (setup.mixer)(Some(mixer_config)); + let mixer = (setup.mixer)(mixer_config); let player_config = setup.player_config.clone(); let connect_config = setup.connect_config.clone(); From 9efd886e9132c18667372ed0eaae80eff1656c17 Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Tue, 25 May 2021 20:17:28 +0200 Subject: [PATCH 2/2] Describe new `mixer-card` getopts behavior Also remove some other getopts and string changes to a separate PR. --- src/main.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/main.rs b/src/main.rs index 6f41db9f..5b8d1d1a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -250,13 +250,13 @@ fn get_setup(args: &[String]) -> Setup { .optopt( "m", "mixer-name", - "Alsa mixer name, e.g \"PCM\" or \"Master\". Defaults to 'PCM'", + "Alsa mixer control, e.g. 'PCM' or 'Master'. Defaults to 'PCM'.", "MIXER_NAME", ) .optopt( "", "mixer-card", - "Alsa mixer card, e.g \"hw:0\" or similar from `aplay -l`. Defaults to 'default' ", + "Alsa mixer card, e.g 'hw:0' or similar from `aplay -l`. Defaults to DEVICE if specified, 'default' otherwise.", "MIXER_CARD", ) .optopt( @@ -268,7 +268,7 @@ fn get_setup(args: &[String]) -> Setup { .optopt( "", "initial-volume", - "Initial volume in %, once connected (must be from 0 to 100)", + "Initial volume (%) once connected {0..100}. Defaults to 50 for softvol and for Alsa mixer the current volume.", "VOLUME", ) .optopt( @@ -327,14 +327,14 @@ fn get_setup(args: &[String]) -> Setup { .optopt( "", "volume-ctrl", - "Volume control type - [cubic, fixed, linear, log]. Default is log.", + "Volume control type {cubic|fixed|linear|log}. Defaults to log.", "VOLUME_CTRL" ) - .optopt( - "", - "volume-range", - "Range of the volume control (dB). Defaults to 60 for softvol and for alsa what the mixer supports.", - "RANGE", + .optopt( + "", + "volume-range", + "Range of the volume control (dB). Defaults to 60 for softvol and for Alsa mixer what the mixer supports.", + "RANGE", ) .optflag( "", @@ -423,7 +423,7 @@ fn get_setup(args: &[String]) -> Setup { }); if volume_range < 0.0 { // User might have specified range as minimum dB volume. - volume_range *= -1.0; + volume_range = -volume_range; warn!( "Please enter positive volume ranges only, assuming {:.2} dB", volume_range @@ -546,15 +546,15 @@ fn get_setup(args: &[String]) -> Setup { match Url::parse(&s) { Ok(url) => { if url.host().is_none() || url.port_or_known_default().is_none() { - panic!("Invalid proxy url, only URLs on the format \"http://host:port\" are allowed"); + panic!("Invalid proxy url, only urls on the format \"http://host:port\" are allowed"); } if url.scheme() != "http" { - panic!("Only insecure http:// proxies are supported"); + panic!("Only unsecure http:// proxies are supported"); } url }, - Err(err) => panic!("Invalid proxy URL: {}, only URLs in the format \"http://host:port\" are allowed", err) + Err(err) => panic!("Invalid proxy url: {}, only urls on the format \"http://host:port\" are allowed", err) } }, ), @@ -575,8 +575,8 @@ fn get_setup(args: &[String]) -> Setup { let normalisation_method = matches .opt_str("normalisation-method") .as_ref() - .map(|gain_type| { - NormalisationMethod::from_str(gain_type).expect("Invalid normalisation method") + .map(|method| { + NormalisationMethod::from_str(method).expect("Invalid normalisation method") }) .unwrap_or_default(); let normalisation_type = matches