diff --git a/CHANGELOG.md b/CHANGELOG.md index c5757aaf..1d603a93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - [main] Prevent hang when discovery is disabled and there are no credentials or when bad credentials are given. - [main] Don't panic when parsing options. Instead list valid values and exit. +- [main] `--alsa-mixer-device` and `--alsa-mixer-index` now fallback to the card and index specified in `--device`. ### Removed - [playback] `alsamixer`: previously deprecated option `mixer-card` has been removed. diff --git a/src/main.rs b/src/main.rs index 0dc25408..07952e5e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -821,41 +821,136 @@ fn get_setup() -> Setup { exit(1); }); + let is_alsa_mixer = match mixer_type.as_deref() { + #[cfg(feature = "alsa-backend")] + Some(AlsaMixer::NAME) => true, + _ => false, + }; + + #[cfg(feature = "alsa-backend")] + if !is_alsa_mixer { + for a in &[ALSA_MIXER_DEVICE, ALSA_MIXER_INDEX, ALSA_MIXER_CONTROL] { + if opt_present(a) { + warn!("Alsa specific mixer options have no effect if not using the alsa mixer."); + break; + } + } + } + let mixer_config = { let mixer_default_config = MixerConfig::default(); #[cfg(feature = "alsa-backend")] - let device = opt_str(ALSA_MIXER_DEVICE).unwrap_or_else(|| { - if let Some(ref device_name) = device { - device_name.to_string() - } else { - mixer_default_config.device.clone() - } - }); + let index = if !is_alsa_mixer { + mixer_default_config.index + } else { + opt_str(ALSA_MIXER_INDEX) + .map(|index| { + index.parse::().unwrap_or_else(|_| { + invalid_error_msg( + ALSA_MIXER_INDEX, + ALSA_MIXER_INDEX_SHORT, + &index, + "", + &mixer_default_config.index.to_string(), + ); - #[cfg(not(feature = "alsa-backend"))] - let device = mixer_default_config.device; - - #[cfg(feature = "alsa-backend")] - let index = opt_str(ALSA_MIXER_INDEX) - .map(|index| { - index.parse::().unwrap_or_else(|_| { - invalid_error_msg( - ALSA_MIXER_INDEX, - ALSA_MIXER_INDEX_SHORT, - &index, - "", - &mixer_default_config.index.to_string(), - ); - - exit(1); + exit(1); + }) }) - }) - .unwrap_or_else(|| mixer_default_config.index); + .unwrap_or_else(|| match device { + // Look for the dev index portion of --device. + // Specifically when --device is :CARD=,DEV= + // or :,. + + // If --device does not contain a ',' it does not contain a dev index. + // In the case that the dev index is omitted it is assumed to be 0 (mixer_default_config.index). + // Malformed --device values will also fallback to mixer_default_config.index. + Some(ref device_name) if device_name.contains(',') => { + // Turn :CARD=,DEV= or :, + // into DEV= or . + let dev = &device_name[device_name.find(',').unwrap_or_default()..] + .trim_start_matches(','); + + // Turn DEV= into (noop if it's already ) + // and then parse . + // Malformed --device values will fail the parse and fallback to mixer_default_config.index. + dev[dev.find('=').unwrap_or_default()..] + .trim_start_matches('=') + .parse::() + .unwrap_or(mixer_default_config.index) + } + _ => mixer_default_config.index, + }) + }; #[cfg(not(feature = "alsa-backend"))] let index = mixer_default_config.index; + #[cfg(feature = "alsa-backend")] + let device = if !is_alsa_mixer { + mixer_default_config.device + } else { + match opt_str(ALSA_MIXER_DEVICE) { + Some(mixer_device) => { + if mixer_device.is_empty() { + empty_string_error_msg(ALSA_MIXER_DEVICE, ALSA_MIXER_DEVICE_SHORT); + } + + mixer_device + } + None => match device { + Some(ref device_name) => { + // Look for the card name or card index portion of --device. + // Specifically when --device is :CARD=,DEV= + // or card index when --device is :,. + // --device values like `pulse`, `default`, `jack` may be valid but there is no way to + // infer automatically what the mixer should be so they fail auto fallback + // so --alsa-mixer-device must be manually specified in those situations. + let start_index = device_name.find(':').unwrap_or_default(); + + let end_index = match device_name.find(',') { + Some(index) if index > start_index => index, + _ => device_name.len(), + }; + + let card = &device_name[start_index..end_index]; + + if card.starts_with(':') { + // mixers are assumed to be hw:CARD= or hw:. + "hw".to_owned() + card + } else { + error!( + "Could not find an alsa mixer for \"{}\", it must be specified with `--{}` / `-{}`", + &device.unwrap_or_default(), + ALSA_MIXER_DEVICE, + ALSA_MIXER_DEVICE_SHORT + ); + + exit(1); + } + } + None => { + error!( + "`--{}` / `-{}` or `--{}` / `-{}` \ + must be specified when `--{}` / `-{}` is set to \"alsa\"", + DEVICE, + DEVICE_SHORT, + ALSA_MIXER_DEVICE, + ALSA_MIXER_DEVICE_SHORT, + MIXER_TYPE, + MIXER_TYPE_SHORT + ); + + exit(1); + } + }, + } + }; + + #[cfg(not(feature = "alsa-backend"))] + let device = mixer_default_config.device; + #[cfg(feature = "alsa-backend")] let control = opt_str(ALSA_MIXER_CONTROL).unwrap_or(mixer_default_config.control); @@ -897,10 +992,12 @@ fn get_setup() -> Setup { exit(1); } }) - .unwrap_or_else(|| match mixer_type.as_deref() { - #[cfg(feature = "alsa-backend")] - Some(AlsaMixer::NAME) => 0.0, // let alsa query the control - _ => VolumeCtrl::DEFAULT_DB_RANGE, + .unwrap_or_else(|| { + if is_alsa_mixer { + 0.0 + } else { + VolumeCtrl::DEFAULT_DB_RANGE + } }); let volume_ctrl = opt_str(VOLUME_CTRL) @@ -1118,10 +1215,12 @@ fn get_setup() -> Setup { (volume as f32 / 100.0 * VolumeCtrl::MAX_VOLUME as f32) as u16 }) - .or_else(|| match mixer_type.as_deref() { - #[cfg(feature = "alsa-backend")] - Some(AlsaMixer::NAME) => None, - _ => cache.as_ref().and_then(Cache::volume), + .or_else(|| { + if is_alsa_mixer { + None + } else { + cache.as_ref().and_then(Cache::volume) + } }); let device_type = opt_str(DEVICE_TYPE) @@ -1489,6 +1588,9 @@ fn get_setup() -> Setup { #[tokio::main(flavor = "current_thread")] async fn main() { const RUST_BACKTRACE: &str = "RUST_BACKTRACE"; + const RECONNECT_RATE_LIMIT_WINDOW: Duration = Duration::from_secs(600); + const RECONNECT_RATE_LIMIT: usize = 5; + if env::var(RUST_BACKTRACE).is_err() { env::set_var(RUST_BACKTRACE, "full") } @@ -1506,14 +1608,18 @@ async fn main() { if setup.enable_discovery { let device_id = setup.session_config.device_id.clone(); - discovery = Some( - librespot::discovery::Discovery::builder(device_id) - .name(setup.connect_config.name.clone()) - .device_type(setup.connect_config.device_type) - .port(setup.zeroconf_port) - .launch() - .unwrap(), - ); + discovery = match librespot::discovery::Discovery::builder(device_id) + .name(setup.connect_config.name.clone()) + .device_type(setup.connect_config.device_type) + .port(setup.zeroconf_port) + .launch() + { + Ok(d) => Some(d), + Err(e) => { + error!("Discovery Error: {}", e); + exit(1); + } + } } if let Some(credentials) = setup.credentials { @@ -1530,7 +1636,12 @@ async fn main() { loop { tokio::select! { - credentials = async { discovery.as_mut().unwrap().next().await }, if discovery.is_some() => { + credentials = async { + match discovery.as_mut() { + Some(d) => d.next().await, + _ => None + } + }, if discovery.is_some() => { match credentials { Some(credentials) => { last_credentials = Some(credentials.clone()); @@ -1553,8 +1664,8 @@ async fn main() { ).fuse()); }, None => { - warn!("Discovery stopped!"); - discovery = None; + error!("Discovery stopped unexpectedly"); + exit(1); } } }, @@ -1610,20 +1721,22 @@ async fn main() { exit(1); } }, - _ = async { spirc_task.as_mut().unwrap().await }, if spirc_task.is_some() => { + _ = async { + if let Some(task) = spirc_task.as_mut() { + task.await; + } + }, if spirc_task.is_some() => { spirc_task = None; warn!("Spirc shut down unexpectedly"); - while !auto_connect_times.is_empty() - && ((Instant::now() - auto_connect_times[0]).as_secs() > 600) - { - let _ = auto_connect_times.remove(0); - } - if let Some(credentials) = last_credentials.clone() { - if auto_connect_times.len() >= 5 { - warn!("Spirc shut down too often. Not reconnecting automatically."); - } else { + let mut reconnect_exceeds_rate_limit = || { + auto_connect_times.retain(|&t| t.elapsed() < RECONNECT_RATE_LIMIT_WINDOW); + auto_connect_times.len() > RECONNECT_RATE_LIMIT + }; + + match last_credentials.clone() { + Some(credentials) if !reconnect_exceeds_rate_limit() => { auto_connect_times.push(Instant::now()); connecting = Box::pin(Session::connect( @@ -1631,19 +1744,25 @@ async fn main() { credentials, setup.cache.clone(), ).fuse()); - } + }, + _ => { + error!("Spirc shut down too often. Not reconnecting automatically."); + exit(1); + }, } }, - event = async { player_event_channel.as_mut().unwrap().recv().await }, if player_event_channel.is_some() => match event { + event = async { + match player_event_channel.as_mut() { + Some(p) => p.recv().await, + _ => None + } + }, if player_event_channel.is_some() => match event { Some(event) => { if let Some(program) = &setup.player_event_program { if let Some(child) = run_program_on_events(event, program) { - if child.is_ok() { - - let mut child = child.unwrap(); - + if let Ok(mut child) = child { tokio::spawn(async move { - match child.wait().await { + match child.wait().await { Ok(e) if e.success() => (), Ok(e) => { if let Some(code) = e.code() { @@ -1669,7 +1788,8 @@ async fn main() { }, _ = tokio::signal::ctrl_c() => { break; - } + }, + else => break, } } @@ -1684,7 +1804,8 @@ async fn main() { if let Some(mut spirc_task) = spirc_task { tokio::select! { _ = tokio::signal::ctrl_c() => (), - _ = spirc_task.as_mut() => () + _ = spirc_task.as_mut() => (), + else => (), } } }