Remove the volume sample iteration (#986)

Move volume calculations out of their own separate samples iteration and into the normalisation iteration

examples/play.rs

@@ -6,6 +6,7 @@ use librespot::core::session::Session;
 use librespot::core::spotify_id::SpotifyId;
 use librespot::playback::audio_backend;
 use librespot::playback::config::{AudioFormat, PlayerConfig};
+use librespot::playback::mixer::NoOpVolume;
 use librespot::playback::player::Player;
 
 #[tokio::main]
@@ -30,7 +31,7 @@ async fn main() {
         .await
         .unwrap();
 
-    let (mut player, _) = Player::new(player_config, session, None, move || {
+    let (mut player, _) = Player::new(player_config, session, Box::new(NoOpVolume), move || {
         backend(None, audio_format)
     });
 

playback/src/mixer/mod.rs

@@ -3,6 +3,8 @@ use crate::config::VolumeCtrl;
 pub mod mappings;
 use self::mappings::MappedCtrl;
 
+pub struct NoOpVolume;
+
 pub trait Mixer: Send {
     fn open(config: MixerConfig) -> Self
     where
@@ -11,13 +13,19 @@ pub trait Mixer: Send {
     fn set_volume(&self, volume: u16);
     fn volume(&self) -> u16;
 
-    fn get_audio_filter(&self) -> Option<Box<dyn AudioFilter + Send>> {
-        None
+    fn get_soft_volume(&self) -> Box<dyn VolumeGetter + Send> {
+        Box::new(NoOpVolume)
     }
 }
 
-pub trait AudioFilter {
-    fn modify_stream(&self, data: &mut [f64]);
+pub trait VolumeGetter {
+    fn attenuation_factor(&self) -> f64;
+}
+
+impl VolumeGetter for NoOpVolume {
+    fn attenuation_factor(&self) -> f64 {
+        1.0
+    }
 }
 
 pub mod softmixer;

playback/src/mixer/softmixer.rs

@@ -1,7 +1,7 @@
 use std::sync::atomic::{AtomicU64, Ordering};
 use std::sync::Arc;
 
-use super::AudioFilter;
+use super::VolumeGetter;
 use super::{MappedCtrl, VolumeCtrl};
 use super::{Mixer, MixerConfig};
 
@@ -35,10 +35,8 @@ impl Mixer for SoftMixer {
             .store(mapped_volume.to_bits(), Ordering::Relaxed)
     }
 
-    fn get_audio_filter(&self) -> Option<Box<dyn AudioFilter + Send>> {
-        Some(Box::new(SoftVolumeApplier {
-            volume: self.volume.clone(),
-        }))
+    fn get_soft_volume(&self) -> Box<dyn VolumeGetter + Send> {
+        Box::new(SoftVolume(self.volume.clone()))
     }
 }
 
@@ -46,17 +44,10 @@ impl SoftMixer {
     pub const NAME: &'static str = "softvol";
 }
 
-struct SoftVolumeApplier {
-    volume: Arc<AtomicU64>,
-}
+struct SoftVolume(Arc<AtomicU64>);
 
-impl AudioFilter for SoftVolumeApplier {
-    fn modify_stream(&self, data: &mut [f64]) {
-        let volume = f64::from_bits(self.volume.load(Ordering::Relaxed));
-        if volume < 1.0 {
-            for x in data.iter_mut() {
-                *x *= volume;
-            }
-        }
+impl VolumeGetter for SoftVolume {
+    fn attenuation_factor(&self) -> f64 {
+        f64::from_bits(self.0.load(Ordering::Relaxed))
     }
 }

playback/src/player.rs

@@ -25,7 +25,7 @@ use crate::core::spotify_id::SpotifyId;
 use crate::core::util::SeqGenerator;
 use crate::decoder::{AudioDecoder, AudioPacket, DecoderError, PassthroughDecoder, VorbisDecoder};
 use crate::metadata::{AudioItem, FileFormat};
-use crate::mixer::AudioFilter;
+use crate::mixer::VolumeGetter;
 
 use crate::{MS_PER_PAGE, NUM_CHANNELS, PAGES_PER_MS, SAMPLES_PER_SECOND};
 
@@ -58,7 +58,7 @@ struct PlayerInternal {
     sink: Box<dyn Sink>,
     sink_status: SinkStatus,
     sink_event_callback: Option<SinkEventCallback>,
-    audio_filter: Option<Box<dyn AudioFilter + Send>>,
+    volume_getter: Box<dyn VolumeGetter + Send>,
     event_senders: Vec<mpsc::UnboundedSender<PlayerEvent>>,
     converter: Converter,
 
@@ -319,7 +319,7 @@ impl Player {
     pub fn new<F>(
         config: PlayerConfig,
         session: Session,
-        audio_filter: Option<Box<dyn AudioFilter + Send>>,
+        volume_getter: Box<dyn VolumeGetter + Send>,
         sink_builder: F,
     ) -> (Player, PlayerEventChannel)
     where
@@ -369,7 +369,7 @@ impl Player {
                 sink: sink_builder(),
                 sink_status: SinkStatus::Closed,
                 sink_event_callback: None,
-                audio_filter,
+                volume_getter,
                 event_senders: [event_sender].to_vec(),
                 converter,
 
@@ -1314,19 +1314,26 @@ impl PlayerInternal {
             Some(mut packet) => {
                 if !packet.is_empty() {
                     if let AudioPacket::Samples(ref mut data) = packet {
+                        // Get the volume for the packet.
+                        // In the case of hardware volume control this will
+                        // always be 1.0 (no change).
+                        let volume = self.volume_getter.attenuation_factor();
+
                         // For the basic normalisation method, a normalisation factor of 1.0 indicates that
                         // there is nothing to normalise (all samples should pass unaltered). For the
                         // dynamic method, there may still be peaks that we want to shave off.
-                        if self.config.normalisation {
-                            if self.config.normalisation_method == NormalisationMethod::Basic
-                                && normalisation_factor < 1.0
+                        // No matter the case we apply volume attenuation last if there is any.
+                        if !self.config.normalisation && volume < 1.0 {
+                            for sample in data.iter_mut() {
+                                *sample *= volume;
+                            }
+                        } else if self.config.normalisation_method == NormalisationMethod::Basic
+                            && (normalisation_factor < 1.0 || volume < 1.0)
                         {
                             for sample in data.iter_mut() {
-                                    *sample *= normalisation_factor;
+                                *sample *= normalisation_factor * volume;
                             }
-                            } else if self.config.normalisation_method
-                                == NormalisationMethod::Dynamic
-                            {
+                        } else if self.config.normalisation_method == NormalisationMethod::Dynamic {
                             // zero-cost shorthands
                             let threshold_db = self.config.normalisation_threshold_dbfs;
                             let knee_db = self.config.normalisation_knee_db;
@@ -1396,8 +1403,7 @@ impl PlayerInternal {
                                     // attack_cf * self.normalisation_peak + (1.0 - attack_cf) * self.normalisation_integrator
                                     // Simplifies to:
                                     // attack_cf * self.normalisation_peak - attack_cf * self.normalisation_integrator + self.normalisation_integrator
-                                        self.normalisation_peak = attack_cf
-                                            * self.normalisation_peak
+                                    self.normalisation_peak = attack_cf * self.normalisation_peak
                                         - attack_cf * self.normalisation_integrator
                                         + self.normalisation_integrator;
 
@@ -1409,14 +1415,9 @@ impl PlayerInternal {
                                     // steps 7-8: conversion into level and multiplication into gain stage
                                     *sample *= db_to_ratio(-self.normalisation_peak);
                                 }
-                                }
-                            }
-                        }
 
-                        // Apply volume attenuation last. TODO: make this so we can chain
-                        // the normaliser and mixer as a processing pipeline.
-                        if let Some(ref editor) = self.audio_filter {
-                            editor.modify_stream(data)
+                                *sample *= volume;
+                            }
                         }
                     }
 

src/main.rs

@@ -1648,12 +1648,12 @@ async fn main() {
                     let player_config = setup.player_config.clone();
                     let connect_config = setup.connect_config.clone();
 
-                    let audio_filter = mixer.get_audio_filter();
+                    let soft_volume = mixer.get_soft_volume();
                     let format = setup.format;
                     let backend = setup.backend;
                     let device = setup.device.clone();
                     let (player, event_channel) =
-                        Player::new(player_config, session.clone(), audio_filter, move || {
+                        Player::new(player_config, session.clone(), soft_volume, move || {
                             (backend)(device, format)
                         });