Merge branch 'dev' into new-api

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.

src/main.rs

@@ -821,23 +821,30 @@ 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(|| {
+        let index = if !is_alsa_mixer {
-            if let Some(ref device_name) = device {
+            mixer_default_config.index
-                device_name.to_string()
         } else {
-                mixer_default_config.device.clone()
+            opt_str(ALSA_MIXER_INDEX)
-            }
-        });
-
-        #[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::<u32>().unwrap_or_else(|_| {
                         invalid_error_msg(
@@ -851,11 +858,99 @@ fn get_setup() -> Setup {
                         exit(1);
                     })
                 })
-            .unwrap_or_else(|| mixer_default_config.index);
+                .unwrap_or_else(|| match device {
+                    // Look for the dev index portion of --device.
+                    // Specifically <dev index> when --device is <something>:CARD=<card name>,DEV=<dev index>
+                    // or <something>:<card index>,<dev index>.
+
+                    // 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 <something>:CARD=<card name>,DEV=<dev index> or <something>:<card index>,<dev index>
+                        // into DEV=<dev index> or <dev index>.
+                        let dev = &device_name[device_name.find(',').unwrap_or_default()..]
+                            .trim_start_matches(',');
+
+                        // Turn DEV=<dev index> into <dev index> (noop if it's already <dev index>)
+                        // and then parse <dev index>.
+                        // Malformed --device values will fail the parse and fallback to mixer_default_config.index.
+                        dev[dev.find('=').unwrap_or_default()..]
+                            .trim_start_matches('=')
+                            .parse::<u32>()
+                            .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 <card name> when --device is <something>:CARD=<card name>,DEV=<dev index>
+                        // or card index when --device is <something>:<card index>,<dev index>.
+                        // --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=<card name> or hw:<card index>.
+                            "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() {
+            .unwrap_or_else(|| {
-                #[cfg(feature = "alsa-backend")]
+                if is_alsa_mixer {
-                Some(AlsaMixer::NAME) => 0.0, // let alsa query the control
+                    0.0
-                _ => VolumeCtrl::DEFAULT_DB_RANGE,
+                } 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() {
+            .or_else(|| {
-                #[cfg(feature = "alsa-backend")]
+                if is_alsa_mixer {
-                Some(AlsaMixer::NAME) => None,
+                    None
-                _ => cache.as_ref().and_then(Cache::volume),
+                } 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(
+        discovery = match librespot::discovery::Discovery::builder(device_id)
-            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(),
+        {
-        );
+            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!");
+                        error!("Discovery stopped unexpectedly");
-                        discovery = None;
+                        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() {
+                let mut reconnect_exceeds_rate_limit = || {
-                    if auto_connect_times.len() >= 5 {
+                    auto_connect_times.retain(|&t| t.elapsed() < RECONNECT_RATE_LIMIT_WINDOW);
-                        warn!("Spirc shut down too often. Not reconnecting automatically.");
+                    auto_connect_times.len() > RECONNECT_RATE_LIMIT
-                    } else {
+                };
+
+                match last_credentials.clone() {
+                    Some(credentials) if !reconnect_exceeds_rate_limit() => {
                         auto_connect_times.push(Instant::now());
 
                         connecting = Box::pin(Session::connect(
@@ -1631,17 +1744,23 @@ 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() {
+                            if let Ok(mut child) = child {
-
-                                let mut child = child.unwrap();
-
                                 tokio::spawn(async move {
                                     match child.wait().await {
                                         Ok(e) if e.success() => (),
@@ -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 => (),
             }
         }
     }