Improve Alsa mixer command-line options

CHANGELOG.md

@@ -29,11 +29,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Deprecated
 - [connect] The `discovery` module was deprecated in favor of the `librespot-discovery` crate
+- [playback] `alsamixer`: renamed `mixer-card` to `alsa-mixer-device`
+- [playback] `alsamixer`: renamed `mixer-name` to `alsa-mixer-control`
+- [playback] `alsamixer`: renamed `mixer-index` to `alsa-mixer-index`
 
 ### Removed
 - [connect] Removed no-op mixer started/stopped logic (breaking)
 - [playback] Removed `with-vorbis` and `with-tremor` features
-- [playback] `alsamixer`: removed `--mixer-linear-volume` option; use `--volume-ctrl linear` instead
+- [playback] `alsamixer`: removed `--mixer-linear-volume` option, now that `--volume-ctrl {linear|log}` work as expected on Alsa
 
 ### Fixed
 - [connect] Fix step size on volume up/down events

playback/src/mixer/alsamixer.rs

@@ -31,14 +31,14 @@ const ZERO_DB: MilliBel = MilliBel(0);
 impl Mixer for AlsaMixer {
     fn open(config: MixerConfig) -> Self {
         info!(
-            "Mixing with alsa and volume control: {:?} for card: {} with mixer control: {},{}",
+            "Mixing with Alsa and volume control: {:?} for device: {} with mixer control: {},{}",
-            config.volume_ctrl, config.card, config.control, config.index,
+            config.volume_ctrl, config.device, config.control, config.index,
         );
 
         let mut config = config; // clone
 
         let mixer =
-            alsa::mixer::Mixer::new(&config.card, false).expect("Could not open Alsa mixer");
+            alsa::mixer::Mixer::new(&config.device, 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");
@@ -56,8 +56,8 @@ impl Mixer for AlsaMixer {
         // 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 =
+            let control = Ctl::new(&config.device, false)
-                Ctl::new(&config.card, false).expect("Could not open Alsa softvol with that card");
+                .expect("Could not open Alsa softvol with that device");
             let mut element_id = ElemId::new(ElemIface::Mixer);
             element_id.set_name(
                 &CString::new(config.control.as_str())
@@ -144,7 +144,7 @@ impl Mixer for AlsaMixer {
 
     fn volume(&self) -> u16 {
         let mixer =
-            alsa::mixer::Mixer::new(&self.config.card, false).expect("Could not open Alsa mixer");
+            alsa::mixer::Mixer::new(&self.config.device, 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");
@@ -184,7 +184,7 @@ impl Mixer for AlsaMixer {
 
     fn set_volume(&self, volume: u16) {
         let mixer =
-            alsa::mixer::Mixer::new(&self.config.card, false).expect("Could not open Alsa mixer");
+            alsa::mixer::Mixer::new(&self.config.device, 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");
@@ -249,7 +249,7 @@ impl AlsaMixer {
         }
 
         let mixer =
-            alsa::mixer::Mixer::new(&self.config.card, false).expect("Could not open Alsa mixer");
+            alsa::mixer::Mixer::new(&self.config.device, 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");

playback/src/mixer/mod.rs

@@ -30,7 +30,7 @@ use self::alsamixer::AlsaMixer;
 
 #[derive(Debug, Clone)]
 pub struct MixerConfig {
-    pub card: String,
+    pub device: String,
     pub control: String,
     pub index: u32,
     pub volume_ctrl: VolumeCtrl,
@@ -39,7 +39,7 @@ pub struct MixerConfig {
 impl Default for MixerConfig {
     fn default() -> MixerConfig {
         MixerConfig {
-            card: String::from("default"),
+            device: String::from("default"),
             control: String::from("PCM"),
             index: 0,
             volume_ctrl: VolumeCtrl::default(),

src/main.rs

@@ -206,9 +206,9 @@ fn get_setup(args: &[String]) -> Setup {
     const HELP: &str = "h";
     const INITIAL_VOLUME: &str = "initial-volume";
     const MIXER_TYPE: &str = "mixer";
-    const MIXER_CARD: &str = "mixer-card";
+    const ALSA_MIXER_DEVICE: &str = "alsa-mixer-device";
-    const MIXER_INDEX: &str = "mixer-index";
+    const ALSA_MIXER_INDEX: &str = "alsa-mixer-index";
-    const MIXER_NAME: &str = "mixer-name";
+    const ALSA_MIXER_CONTROL: &str = "alsa-mixer-control";
     const NAME: &str = "name";
     const NORMALISATION_ATTACK: &str = "normalisation-attack";
     const NORMALISATION_GAIN_TYPE: &str = "normalisation-gain-type";
@@ -296,24 +296,42 @@ fn get_setup(args: &[String]) -> Setup {
         "Specify the dither algorithm to use - [none, gpdf, tpdf, tpdf_hp]. Defaults to 'tpdf' for formats S16, S24, S24_3 and 'none' for other formats.",
         "DITHER",
     )
-    .optopt("", MIXER_TYPE, "Mixer to use {alsa|softvol}.", "MIXER")
+    .optopt("m", MIXER_TYPE, "Mixer to use {alsa|softvol}.", "MIXER")
     .optopt(
-        "m",
+        "",
-        MIXER_NAME,
+        "mixer-name", // deprecated
+        "",
+        "",
+    )
+    .optopt(
+        "",
+        ALSA_MIXER_CONTROL,
         "Alsa mixer control, e.g. 'PCM' or 'Master'. Defaults to 'PCM'.",
         "NAME",
     )
     .optopt(
         "",
-        MIXER_CARD,
+        "mixer-card", // deprecated
-        "Alsa mixer card, e.g 'hw:0' or similar from `aplay -l`. Defaults to DEVICE if specified, 'default' otherwise.",
+        "",
-        "MIXER_CARD",
+        "",
     )
     .optopt(
         "",
-        MIXER_INDEX,
+        ALSA_MIXER_DEVICE,
+        "Alsa mixer device, e.g 'hw:0' or similar from `aplay -l`. Defaults to `--device` if specified, 'default' otherwise.",
+        "DEVICE",
+    )
+    .optopt(
+        "",
+        "mixer-index", // deprecated
+        "",
+        "",
+    )
+    .optopt(
+        "",
+        ALSA_MIXER_INDEX,
         "Alsa index of the cards mixer. Defaults to 0.",
-        "INDEX",
+        "NUMBER",
     )
     .optopt(
         "",
@@ -459,20 +477,50 @@ fn get_setup(args: &[String]) -> Setup {
     let mixer = mixer::find(mixer_type.as_deref()).expect("Invalid mixer");
 
     let mixer_config = {
-        let card = matches.opt_str(MIXER_CARD).unwrap_or_else(|| {
+        let mixer_device = match matches.opt_str("mixer-card") {
-            if let Some(ref device_name) = device {
+            Some(card) => {
-                device_name.to_string()
+                warn!("--mixer-card is deprecated and will be removed in a future release.");
-            } else {
+                warn!("Please use --alsa-mixer-device instead.");
-                MixerConfig::default().card
+                card
             }
-        });
+            None => matches.opt_str(ALSA_MIXER_DEVICE).unwrap_or_else(|| {
-        let index = matches
+                if let Some(ref device_name) = device {
-            .opt_str(MIXER_INDEX)
+                    device_name.to_string()
-            .map(|index| index.parse::<u32>().unwrap())
+                } else {
-            .unwrap_or(0);
+                    MixerConfig::default().device
-        let control = matches
+                }
-            .opt_str(MIXER_NAME)
+            }),
-            .unwrap_or_else(|| MixerConfig::default().control);
+        };
+
+        let index = match matches.opt_str("mixer-index") {
+            Some(index) => {
+                warn!("--mixer-index is deprecated and will be removed in a future release.");
+                warn!("Please use --alsa-mixer-index instead.");
+                index
+                    .parse::<u32>()
+                    .expect("Mixer index is not a valid number")
+            }
+            None => matches
+                .opt_str(ALSA_MIXER_INDEX)
+                .map(|index| {
+                    index
+                        .parse::<u32>()
+                        .expect("Alsa mixer index is not a valid number")
+                })
+                .unwrap_or(0),
+        };
+
+        let control = match matches.opt_str("mixer-name") {
+            Some(name) => {
+                warn!("--mixer-name is deprecated and will be removed in a future release.");
+                warn!("Please use --alsa-mixer-control instead.");
+                name
+            }
+            None => matches
+                .opt_str(ALSA_MIXER_CONTROL)
+                .unwrap_or_else(|| MixerConfig::default().control),
+        };
+
         let mut volume_range = matches
             .opt_str(VOLUME_RANGE)
             .map(|range| range.parse::<f64>().unwrap())
@@ -503,7 +551,7 @@ fn get_setup(args: &[String]) -> Setup {
             });
 
         MixerConfig {
-            card,
+            device: mixer_device,
             control,
             index,
             volume_ctrl,