Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement generic dummy mixer #1162

Open
sqozz opened this issue May 11, 2023 · 2 comments
Open

Implement generic dummy mixer #1162

sqozz opened this issue May 11, 2023 · 2 comments

Comments

@sqozz
Copy link
Contributor

sqozz commented May 11, 2023

Is your feature request related to a problem? Please describe.
I route librespot into pulseaudio to mix it with other sources. The result is written into a socket which is consumed by snapcast. From a users perspective it would be nice if the volume slider in Spotify/the Spotify App could control the general master volume of snapcast.

Describe the solution you'd like
I would like to control the Spotify volume slider without adjusting the actual volume. The volume regulation should happen by an external script which receives the selected volume (I think onevent already receives the selected volume) and e.g. does http requests to something to adjust the volume.

Describe alternatives you've considered
I considered (ab)using pulseaudio for that purpose. Unfortunately it seems like the PA backend is not supported for the --mixer option. I also tried to make use of --mixer softvol --volume-ctrl fixed --initial-volume 100 (Which is described here) but this just disabled volume control via Spotify completely and therefore an onevent-script cannot receive the volume change.

Additional context
Supporting pulseaudio as mixer might be a viable solution but I think a much more generic solution would be a "dummy" volume regulation which receives the selected level but does not adjust the audio stream.

@kingosticks
Copy link
Contributor

When you specify --volume-ctrl fixed we do not advertise the ability to change the volume and the UI will likely be disabled in a Connect controller, as you saw.

We already have librespot::playback::mixer::NoOpVolume but it's not exposed through the --mixer option. Adding it will probably be enough to make this use case work. Give it a try?

@sqozz
Copy link
Contributor Author

sqozz commented May 12, 2023

We already have librespot::playback::mixer::NoOpVolume but it's not exposed through the --mixer option. Adding it will probably be enough to make this use case work. Give it a try?

Yes this sounds like it would actually cover my needs. I tried to come up with some code changes but never programmed in rust and it is not even close to be suitable as a PR so I will just attach it as patch here:

diff --git a/playback/src/mixer/mod.rs b/playback/src/mixer/mod.rs
index 0a8b8d6..b7c35cb 100644
--- a/playback/src/mixer/mod.rs
+++ b/playback/src/mixer/mod.rs
@@ -30,6 +30,8 @@ impl VolumeGetter for NoOpVolume {

 pub mod softmixer;
 use self::softmixer::SoftMixer;
+pub mod noopmixer;
+use self::noopmixer::NoOpMixer;

 #[cfg(feature = "alsa-backend")]
 pub mod alsamixer;
@@ -63,6 +65,7 @@ fn mk_sink<M: Mixer + 'static>(config: MixerConfig) -> Box<dyn Mixer> {

 pub const MIXERS: &[(&str, MixerFn)] = &[
     (SoftMixer::NAME, mk_sink::<SoftMixer>), // default goes first
+    (NoOpMixer::NAME, mk_sink::<NoOpMixer>),
     #[cfg(feature = "alsa-backend")]
     (AlsaMixer::NAME, mk_sink::<AlsaMixer>),
 ];
diff --git a/playback/src/mixer/noopmixer.rs b/playback/src/mixer/noopmixer.rs
new file mode 100644
index 0000000..867c950
--- /dev/null
+++ b/playback/src/mixer/noopmixer.rs
@@ -0,0 +1,49 @@
+use std::sync::atomic::{AtomicU64, Ordering};
+use std::sync::Arc;
+
+use super::VolumeGetter;
+use super::{MappedCtrl, VolumeCtrl};
+use super::{Mixer, MixerConfig};
+
+#[derive(Clone)]
+pub struct NoOpMixer {
+    // There is no AtomicF64, so we store the f64 as bits in a u64 field.
+    // It's much faster than a Mutex<f64>.
+    volume: Arc<AtomicU64>,
+    volume_ctrl: VolumeCtrl,
+}
+
+impl Mixer for NoOpMixer {
+    fn open(config: MixerConfig) -> Self {
+        let volume_ctrl = config.volume_ctrl;
+        info!("Mixing with NoOp mixer");
+
+        Self {
+            volume: Arc::new(AtomicU64::new(f64::to_bits(0.5))),
+            volume_ctrl,
+        }
+    }
+
+    fn volume(&self) -> u16 {
+        let mapped_volume = f64::from_bits(self.volume.load(Ordering::Relaxed));
+        self.volume_ctrl.as_unmapped(mapped_volume)
+    }
+
+    fn set_volume(&self, volume: u16) {
+        let mapped_volume = self.volume_ctrl.to_mapped(volume);
+        self.volume
+            .store(mapped_volume.to_bits(), Ordering::Relaxed)
+    }
+}
+
+impl NoOpMixer {
+    pub const NAME: &'static str = "noop";
+}
+
+struct NoOpVolume(Arc<AtomicU64>);
+
+impl VolumeGetter for NoOpVolume {
+    fn attenuation_factor(&self) -> f64 {
+        f64::from_bits(self.0.load(Ordering::Relaxed))
+    }
+}
diff --git a/src/main.rs b/src/main.rs
index e4727ba..a6deb46 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -290,7 +290,7 @@ fn get_setup() -> Setup {
     // Options that have different descriptions
     // depending on what backends were enabled at build time.
     #[cfg(feature = "alsa-backend")]
-    const MIXER_TYPE_DESC: &str = "Mixer to use {alsa|softvol}. Defaults to softvol.";
+    const MIXER_TYPE_DESC: &str = "Mixer to use {alsa|softvol|noop}. Defaults to softvol.";
     #[cfg(not(feature = "alsa-backend"))]
     const MIXER_TYPE_DESC: &str = "Not supported by the included audio backend(s).";
     #[cfg(any(
@@ -789,10 +789,10 @@ fn get_setup() -> Setup {
         }
     }

-    #[cfg(feature = "alsa-backend")]
+    //#[cfg(feature = "alsa-backend")]
     let mixer_type = opt_str(MIXER_TYPE);
-    #[cfg(not(feature = "alsa-backend"))]
-    let mixer_type: Option<String> = None;
+    //#[cfg(not(feature = "alsa-backend"))]
+    //let mixer_type: Option<String> = None;

     let mixer = mixer::find(mixer_type.as_deref()).unwrap_or_else(|| {
         invalid_error_msg(

It somehow works but the whole "softvol as default" logic for --mixer is now broken. If somebody has hints or wants to pick up my patch I'd be very happy :) My snippet is CC-0 so no mention, credit, anything needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants