Merge remote-tracking branch 'upstream/dev' into dithering-and-noise-shaping

Author: roderickvd

CHANGELOG.md

@@ -11,9 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [playback] Add support for noise shaping with `--shape-noise` for lower perceived noise (breaking)
 
 ### Changed
-
 * [audio, playback] Moved `VorbisDecoder`, `VorbisError`, `AudioPacket`, `PassthroughDecoder`, `PassthroughError`, `AudioError`, `AudioDecoder` and the `convert` module from `librespot-audio` to `librespot-playback`. The underlying crates `vorbis`, `librespot-tremor`, `lewton` and `ogg` should be used directly. (breaking)
 
+### Fixed
+* [playback] Incorrect `PlayerConfig::default().normalisation_threshold` caused distortion when using dynamic volume normalisation downstream
+
 ## [0.2.0] - 2021-05-04
 
 ## [0.1.6] - 2021-02-22

core/tests/connect.rs

@@ -1,18 +1,26 @@
+use std::time::Duration;
+
 use librespot_core::authentication::Credentials;
 use librespot_core::config::SessionConfig;
 use librespot_core::session::Session;
 
+use tokio::time::timeout;
+
 #[tokio::test]
 async fn test_connection() {
-    let result = Session::connect(
-        SessionConfig::default(),
-        Credentials::with_password("test", "test"),
-        None,
-    )
-    .await;
+    timeout(Duration::from_secs(30), async {
+        let result = Session::connect(
+            SessionConfig::default(),
+            Credentials::with_password("test", "test"),
+            None,
+        )
+        .await;
 
-    match result {
-        Ok(_) => panic!("Authentication succeeded despite of bad credentials."),
-        Err(e) => assert_eq!(e.to_string(), "Login failed with reason: Bad credentials"),
-    };
+        match result {
+            Ok(_) => panic!("Authentication succeeded despite of bad credentials."),
+            Err(e) => assert_eq!(e.to_string(), "Login failed with reason: Bad credentials"),
+        }
+    })
+    .await
+    .unwrap();
 }

playback/src/config.rs

@@ -1,6 +1,8 @@
+use super::player::NormalisationData;
 use crate::convert::i24;
 pub use crate::dither::{Ditherer, DithererBuilder};
 pub use crate::shape_noise::{NoiseShaper, NoiseShaperBuilder};
+
 use std::convert::TryFrom;
 use std::mem;
 use std::str::FromStr;
@@ -146,7 +148,7 @@ impl Default for PlayerConfig {
             normalisation_type: NormalisationType::default(),
             normalisation_method: NormalisationMethod::default(),
             normalisation_pregain: 0.0,
-            normalisation_threshold: -1.0,
+            normalisation_threshold: NormalisationData::db_to_ratio(-1.0),
             normalisation_attack: 0.005,
             normalisation_release: 0.1,
             normalisation_knee: 1.0,

playback/src/player.rs

@@ -235,6 +235,10 @@ impl NormalisationData {
     }
 
     fn get_factor(config: &PlayerConfig, data: NormalisationData) -> f32 {
+        if !config.normalisation {
+            return 1.0;
+        }
+
         let [gain_db, gain_peak] = match config.normalisation_type {
             NormalisationType::Album => [data.album_gain_db, data.album_peak],
             NormalisationType::Track => [data.track_gain_db, data.track_peak],

src/main.rs

@@ -567,26 +567,24 @@ fn get_setup(args: &[String]) -> Setup {
             .opt_str("normalisation-pregain")
             .map(|pregain| pregain.parse::<f32>().expect("Invalid pregain float value"))
             .unwrap_or(PlayerConfig::default().normalisation_pregain);
-        let normalisation_threshold = NormalisationData::db_to_ratio(
-            matches
-                .opt_str("normalisation-threshold")
-                .map(|threshold| {
+        let normalisation_threshold = matches
+            .opt_str("normalisation-threshold")
+            .map(|threshold| {
+                NormalisationData::db_to_ratio(
                     threshold
                         .parse::<f32>()
-                        .expect("Invalid threshold float value")
-                })
-                .unwrap_or(PlayerConfig::default().normalisation_threshold),
-        );
+                        .expect("Invalid threshold float value"),
+                )
+            })
+            .unwrap_or(PlayerConfig::default().normalisation_threshold);
         let normalisation_attack = matches
             .opt_str("normalisation-attack")
-            .map(|attack| attack.parse::<f32>().expect("Invalid attack float value"))
-            .unwrap_or(PlayerConfig::default().normalisation_attack * MILLIS)
-            / MILLIS;
+            .map(|attack| attack.parse::<f32>().expect("Invalid attack float value") / MILLIS)
+            .unwrap_or(PlayerConfig::default().normalisation_attack);
         let normalisation_release = matches
             .opt_str("normalisation-release")
-            .map(|release| release.parse::<f32>().expect("Invalid release float value"))
-            .unwrap_or(PlayerConfig::default().normalisation_release * MILLIS)
-            / MILLIS;
+            .map(|release| release.parse::<f32>().expect("Invalid release float value") / MILLIS)
+            .unwrap_or(PlayerConfig::default().normalisation_release);
         let normalisation_knee = matches
             .opt_str("normalisation-knee")
             .map(|knee| knee.parse::<f32>().expect("Invalid knee float value"))