Merge pull request #640 from PetrGlad/test-fixes

Test fixes, upgrade quickcheck

.github/workflows/ci.yml

@@ -47,7 +47,11 @@ jobs:
         if: matrix.toolchain == 'stable' && matrix.os == 'ubuntu-latest'
 
       - run: cargo test --all-targets
-      - run: cargo test --features=symphonia-all --all-targets
+      - run: cargo test --lib --bins --tests --benches --features=experimental
+      - run: cargo test --all-targets --features=symphonia-all
+      # `cargo test` does not check benchmarks and `cargo test --all-targets` excludes
+      # documentation tests. Therefore, we need an additional docs test command here.
+      - run: cargo test --doc
   cargo-publish:
     if: github.event_name == 'push' && github.ref == 'refs/heads/master'
     env:

.gitignore

@@ -1,2 +1,2 @@
 target
-Cargo.lock
+

Cargo.toml

@@ -22,6 +22,7 @@ rand = { version = "0.8.5", features = ["small_rng"], optional = true }
 tracing = { version = "0.1.40", optional = true }
 
 atomic_float = { version = "1.1.0", optional = true }
+num-rational = "0.4.2"
 
 [features]
 default = ["flac", "vorbis", "wav", "mp3"]
@@ -47,7 +48,7 @@ symphonia-alac = ["symphonia/isomp4", "symphonia/alac"]
 symphonia-aiff = ["symphonia/aiff", "symphonia/pcm"]
 
 [dev-dependencies]
-quickcheck = "0.9.2"
+quickcheck = "1"
 rstest = "0.18.2"
 rstest_reuse = "0.6.0"
 approx = "0.5.1"

examples/automatic_gain_control.rs

@@ -20,12 +20,11 @@ fn main() -> Result<(), Box<dyn Error>> {
     let agc_source = source.automatic_gain_control(1.0, 4.0, 0.005, 5.0);
 
     // Make it so that the source checks if automatic gain control should be
-    // enabled or disabled every 5 milliseconds. We must clone `agc_enabled`
+    // enabled or disabled every 5 milliseconds. We must clone `agc_enabled`,
     // or we would lose it when we move it into the periodic access.
     let agc_enabled = Arc::new(AtomicBool::new(true));
     let agc_enabled_clone = agc_enabled.clone();
     let controlled = agc_source.periodic_access(Duration::from_millis(5), move |agc_source| {
-        #[cfg(not(feature = "experimental"))]
         agc_source.set_enabled(agc_enabled_clone.load(Ordering::Relaxed));
     });
 

src/buffer.rs

@@ -42,8 +42,8 @@ where
     where
         D: Into<Vec<S>>,
     {
-        assert!(channels != 0);
+        assert!(channels >= 1);
-        assert!(sample_rate != 0);
+        assert!(sample_rate >= 1);
 
         let data = data.into();
         let duration_ns = 1_000_000_000u64.checked_mul(data.len() as u64).unwrap()

src/conversions/sample.rs

@@ -74,13 +74,15 @@ where
 pub trait Sample: CpalSample {
     /// Linear interpolation between two samples.
     ///
-    /// The result should be equal to
+    /// The result should be equivalent to
-    /// `first * numerator / denominator + second * (1 - numerator / denominator)`.
+    /// `first * (1 - numerator / denominator) + second * numerator / denominator`.
+    ///
+    /// To avoid numeric overflows pick smaller numerator.
     fn lerp(first: Self, second: Self, numerator: u32, denominator: u32) -> Self;
     /// Multiplies the value of this sample by the given amount.
     fn amplify(self, value: f32) -> Self;
 
-    /// Converts the sample to an f32 value.
+    /// Converts the sample to a f32 value.
     fn to_f32(self) -> f32;
 
     /// Calls `saturating_add` on the sample.
@@ -178,3 +180,62 @@ impl Sample for f32 {
         0.0
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use num_rational::Ratio;
+    use quickcheck::{quickcheck, TestResult};
+
+    #[test]
+    fn lerp_u16_constraints() {
+        let a = 12u16;
+        let b = 31u16;
+        assert_eq!(Sample::lerp(a, b, 0, 1), a);
+        assert_eq!(Sample::lerp(a, b, 1, 1), b);
+
+        assert_eq!(Sample::lerp(0, u16::MAX, 0, 1), 0);
+        assert_eq!(Sample::lerp(0, u16::MAX, 1, 1), u16::MAX);
+        // Zeroes
+        assert_eq!(Sample::lerp(0u16, 0, 0, 1), 0);
+        assert_eq!(Sample::lerp(0u16, 0, 1, 1), 0);
+        // Downward changes
+        assert_eq!(Sample::lerp(1u16, 0, 0, 1), 1);
+        assert_eq!(Sample::lerp(1u16, 0, 1, 1), 0);
+    }
+
+    #[test]
+    fn lerp_i16_constraints() {
+        let a = 12i16;
+        let b = 31i16;
+        assert_eq!(Sample::lerp(a, b, 0, 1), a);
+        assert_eq!(Sample::lerp(a, b, 1, 1), b);
+
+        assert_eq!(Sample::lerp(0, i16::MAX, 0, 1), 0);
+        assert_eq!(Sample::lerp(0, i16::MAX, 1, 1), i16::MAX);
+        assert_eq!(Sample::lerp(0, i16::MIN, 1, 1), i16::MIN);
+        // Zeroes
+        assert_eq!(Sample::lerp(0u16, 0, 0, 1), 0);
+        assert_eq!(Sample::lerp(0u16, 0, 1, 1), 0);
+        // Downward changes
+        assert_eq!(Sample::lerp(a, i16::MIN, 0, 1), a);
+        assert_eq!(Sample::lerp(a, i16::MIN, 1, 1), i16::MIN);
+    }
+
+    quickcheck! {
+        fn lerp_u16_random(first: u16, second: u16, numerator: u16, denominator: u16) -> TestResult {
+            if denominator == 0 { return TestResult::discard(); }
+
+            let (numerator, denominator) = Ratio::new(numerator, denominator).into_raw();
+            if numerator > 5000 { return TestResult::discard(); }
+
+            let a = first as f64;
+            let b = second as f64;
+            let c = numerator as f64 / denominator as f64;
+            if c < 0.0 || c > 1.0 { return TestResult::discard(); };
+            let reference = a * (1.0 - c) + b * c;
+            let x = Sample::lerp(first, second, numerator as u32, denominator as u32) as f64;
+            TestResult::from_bool((x - reference).abs() < 1.0)
+        }
+    }
+}

src/conversions/sample_rate.rs

@@ -1,5 +1,6 @@
 use crate::conversions::Sample;
 
+use num_rational::Ratio;
 use std::mem;
 
 /// Iterator that converts from a certain sample rate to another.
@@ -34,12 +35,19 @@ where
     I: Iterator,
     I::Item: Sample,
 {
+    /// Create new sample rate converter.
     ///
+    /// The converter uses simple linear interpolation for up-sampling
+    /// and discards samples for down-sampling. This may introduce audible
+    /// distortions in some cases (see [#584](https://github.com/RustAudio/rodio/issues/584)).
+    ///
+    /// # Limitations
+    /// Some rate conversions where target rate is high and rates are mutual primes the sample
+    /// interpolation may cause numeric overflows. Conversion between usual sample rates
+    /// 2400, 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000, ... is expected to work.
     ///
     /// # Panic
-    ///
+    /// Panics if `from`, `to` or `num_channels` are 0.
-    /// Panics if `from` or `to` are equal to 0.
-    ///
     #[inline]
     pub fn new(
         mut input: I,
@@ -50,26 +58,12 @@ where
         let from = from.0;
         let to = to.0;
 
+        assert!(num_channels >= 1);
         assert!(from >= 1);
         assert!(to >= 1);
 
-        // finding greatest common divisor
-        let gcd = {
-            #[inline]
-            fn gcd(a: u32, b: u32) -> u32 {
-                if b == 0 {
-                    a
-                } else {
-                    gcd(b, a % b)
-                }
-            }
-
-            gcd(from, to)
-        };
-
         let (first_samples, next_samples) = if from == to {
             // if `from` == `to` == 1, then we just pass through
-            debug_assert_eq!(from, gcd);
             (Vec::new(), Vec::new())
         } else {
             let first = input
@@ -83,10 +77,13 @@ where
             (first, next)
         };
 
+        // Reducing numerator to avoid numeric overflows during interpolation.
+        let (to, from) = Ratio::new(to, from).into_raw();
+
         SampleRateConverter {
             input,
-            from: from / gcd,
+            from,
-            to: to / gcd,
+            to,
             channels: num_channels,
             current_frame_pos_in_chunk: 0,
             next_output_frame_pos_in_chunk: 0,
@@ -256,99 +253,105 @@ where
 mod test {
     use super::SampleRateConverter;
     use core::time::Duration;
-    use cpal::SampleRate;
+    use cpal::{ChannelCount, SampleRate};
-    use quickcheck::quickcheck;
+    use quickcheck::{quickcheck, TestResult};
-
-    // TODO: Remove once cpal 0.12.2 is released and the dependency is updated
-    //  (cpal#483 implemented ops::Mul on SampleRate)
-    const fn multiply_rate(r: SampleRate, k: u32) -> SampleRate {
-        SampleRate(k * r.0)
-    }
 
     quickcheck! {
         /// Check that resampling an empty input produces no output.
-        fn empty(from: u32, to: u32, n: u16) -> () {
+        fn empty(from: u16, to: u16, channels: u8) -> TestResult {
-            let from = if from == 0 { return; } else { SampleRate(from) };
+            if channels == 0 || channels > 128
-            let to   = if   to == 0 { return; } else { SampleRate(to)   };
+                || from == 0
-            if n == 0 { return; }
+                || to == 0
+            {
+                return TestResult::discard();
+            }
+            let from = SampleRate(from as u32);
+            let to   = SampleRate(to as u32);
 
             let input: Vec<u16> = Vec::new();
             let output =
-                SampleRateConverter::new(input.into_iter(), from, to, n)
+                SampleRateConverter::new(input.into_iter(), from, to, channels as ChannelCount)
                   .collect::<Vec<_>>();
 
             assert_eq!(output, []);
+            TestResult::passed()
         }
 
         /// Check that resampling to the same rate does not change the signal.
-        fn identity(from: u32, n: u16, input: Vec<u16>) -> () {
+        fn identity(from: u16, channels: u8, input: Vec<u16>) -> TestResult {
-            let from = if from == 0 { return; } else { SampleRate(from) };
+            if channels == 0 || channels > 128 || from == 0 { return TestResult::discard(); }
-            if n == 0 { return; }
+            let from = SampleRate(from as u32);
 
             let output =
-                SampleRateConverter::new(input.clone().into_iter(), from, from, n)
+                SampleRateConverter::new(input.clone().into_iter(), from, from, channels as ChannelCount)
                   .collect::<Vec<_>>();
 
-            assert_eq!(input, output);
+            TestResult::from_bool(input == output)
         }
 
         /// Check that dividing the sample rate by k (integer) is the same as
         ///   dropping a sample from each channel.
-        fn divide_sample_rate(to: u32, k: u32, input: Vec<u16>, n: u16) -> () {
+        fn divide_sample_rate(to: u16, k: u16, input: Vec<u16>, channels: u8) -> TestResult {
-            let to = if to == 0 { return; } else { SampleRate(to) };
+            if k == 0 || channels == 0 || channels > 128 || to == 0 || to > 48000 {
-            let from = multiply_rate(to, k);
+                return TestResult::discard();
-            if k == 0 || n == 0 { return; }
+            }
+
+            let to = SampleRate(to as u32);
+            let from = to * k as u32;
 
             // Truncate the input, so it contains an integer number of frames.
             let input = {
-                let ns = n as usize;
+                let ns = channels as usize;
                 let mut i = input;
                 i.truncate(ns * (i.len() / ns));
                 i
             };
 
             let output =
-                SampleRateConverter::new(input.clone().into_iter(), from, to, n)
+                SampleRateConverter::new(input.clone().into_iter(), from, to, channels as ChannelCount)
                   .collect::<Vec<_>>();
 
-            assert_eq!(input.chunks_exact(n.into())
+            TestResult::from_bool(input.chunks_exact(channels.into())
-                         .step_by(k as usize).collect::<Vec<_>>().concat(),
+                         .step_by(k as usize).collect::<Vec<_>>().concat() == output)
-                       output)
         }
 
         /// Check that, after multiplying the sample rate by k, every k-th
         ///  sample in the output matches exactly with the input.
-        fn multiply_sample_rate(from: u32, k: u32, input: Vec<u16>, n: u16) -> () {
+        fn multiply_sample_rate(from: u16, k: u8, input: Vec<u16>, channels: u8) -> TestResult {
-            let from = if from == 0 { return; } else { SampleRate(from) };
+            if k == 0 || channels == 0 || channels > 128 || from == 0 {
-            let to = multiply_rate(from, k);
+                return TestResult::discard();
-            if k == 0 || n == 0 { return; }
+            }
+
+            let from = SampleRate(from as u32);
+            let to = from * k as u32;
 
             // Truncate the input, so it contains an integer number of frames.
             let input = {
-                let ns = n as usize;
+                let ns = channels as usize;
                 let mut i = input;
                 i.truncate(ns * (i.len() / ns));
                 i
             };
 
             let output =
-                SampleRateConverter::new(input.clone().into_iter(), from, to, n)
+                SampleRateConverter::new(input.clone().into_iter(), from, to, channels as ChannelCount)
                   .collect::<Vec<_>>();
 
-            assert_eq!(input,
+            TestResult::from_bool(input ==
-                       output.chunks_exact(n.into())
+                       output.chunks_exact(channels.into())
-                         .step_by(k as usize).collect::<Vec<_>>().concat()
+                         .step_by(k as usize).collect::<Vec<_>>().concat())
-            )
         }
 
         #[ignore]
         /// Check that resampling does not change the audio duration,
         ///  except by a negligible amount (± 1ms).  Reproduces #316.
         /// Ignored, pending a bug fix.
-        fn preserve_durations(d: Duration, freq: f32, to: u32) -> () {
+        fn preserve_durations(d: Duration, freq: f32, to: u32) -> TestResult {
+            if to == 0 { return TestResult::discard(); }
+
             use crate::source::{SineWave, Source};
 
-            let to = if to == 0 { return; } else { SampleRate(to) };
+            let to = SampleRate(to);
             let source = SineWave::new(freq).take_duration(d);
             let from = SampleRate(source.sample_rate());
 
@@ -358,9 +361,7 @@ mod test {
                 Duration::from_secs_f32(resampled.count() as f32 / to.0 as f32);
 
             let delta = if d < duration { duration - d } else { d - duration };
-            assert!(delta < Duration::from_millis(1),
+            TestResult::from_bool(delta < Duration::from_millis(1))
-                    "Resampled duration ({:?}) is not close to original ({:?}); Δ = {:?}",
-                    duration, d, delta);
         }
     }
 
@@ -369,9 +370,31 @@ mod test {
         let input = vec![2u16, 16, 4, 18, 6, 20, 8, 22];
         let output =
             SampleRateConverter::new(input.into_iter(), SampleRate(2000), SampleRate(3000), 2);
-        assert_eq!(output.len(), 12);
+        assert_eq!(output.len(), 12); // Test the source's Iterator::size_hint()
 
         let output = output.collect::<Vec<_>>();
         assert_eq!(output, [2, 16, 3, 17, 4, 18, 6, 20, 7, 21, 8, 22]);
     }
+
+    #[test]
+    fn upsample2() {
+        let input = vec![1u16, 14];
+        let output =
+            SampleRateConverter::new(input.into_iter(), SampleRate(1000), SampleRate(7000), 1);
+        let size_estimation = output.len();
+        let output = output.collect::<Vec<_>>();
+        assert_eq!(output, [1, 2, 4, 6, 8, 10, 12, 14]);
+        assert!((size_estimation as f32 / output.len() as f32).abs() < 2.0);
+    }
+
+    #[test]
+    fn downsample() {
+        let input = Vec::from_iter(0u16..17);
+        let output =
+            SampleRateConverter::new(input.into_iter(), SampleRate(12000), SampleRate(2400), 1);
+        let size_estimation = output.len();
+        let output = output.collect::<Vec<_>>();
+        assert_eq!(output, [0, 5, 10, 15]);
+        assert!((size_estimation as f32 / output.len() as f32).abs() < 2.0);
+    }
 }

src/source/agc.rs

@@ -288,10 +288,13 @@ where
     }
 
     #[cfg(feature = "experimental")]
-    /// Access the AGC on/off control for real-time adjustment.
+    /// Access the AGC on/off control.
-    ///
     /// Use this to dynamically enable or disable AGC processing during runtime.
-    /// Useful for comparing processed and unprocessed audio or for disabling/enabling AGC at runtime.
+    ///
+    /// AGC is on by default. `false` is disabled state, `true` is enabled.
+    /// In disabled state the sound is passed through AGC unchanged.
+    ///
+    /// In particular, this control is useful for comparing processed and unprocessed audio.
     #[inline]
     pub fn get_agc_control(&self) -> Arc<AtomicBool> {
         Arc::clone(&self.is_enabled)

src/source/mod.rs

@@ -170,7 +170,6 @@ where
     fn total_duration(&self) -> Option<Duration>;
 
     /// Stores the source in a buffer in addition to returning it. This iterator can be cloned.
-
     #[inline]
     fn buffered(self) -> Buffered<Self>
     where
@@ -297,7 +296,8 @@ where
     ///   A recommended value for `absolute_max_gain` is `5`, which provides a good balance between
     ///   amplification capability and protection against distortion in most scenarios.
     ///
-    /// Use `get_agc_control` to obtain a handle for real-time enabling/disabling of the AGC.
+    /// `automatic_gain_control` example in this project shows a pattern you can use
+    /// to enable/disable the AGC filter dynamically.
     ///
     /// # Example (Quick start)
     ///

src/source/skip.rs

@@ -179,9 +179,9 @@ mod tests {
         seconds: u32,
         seconds_to_skip: u32,
     ) {
-        let data: Vec<f32> = (1..=(sample_rate * channels as u32 * seconds))
+        let buf_len = (sample_rate * channels as u32 * seconds) as usize;
-            .map(|_| 0f32)
+        assert!(buf_len < 10 * 1024 * 1024);
-            .collect();
+        let data: Vec<f32> = vec![0f32; buf_len];
         let test_buffer = SamplesBuffer::new(channels, sample_rate, data);
         let seconds_left = seconds.saturating_sub(seconds_to_skip);
 

src/source/speed.rs

@@ -4,12 +4,12 @@
 //! encapsulates playback speed controls of the current sink.
 //!
 //! In order to speed up a sink, the speed struct:
-//! - Increases the current sample rate by the given factor
+//! - Increases the current sample rate by the given factor.
-//! - Updates the total duration function to cover for the new factor by dividing by the factor
+//! - Updates the total duration function to cover for the new factor by dividing by the factor.
-//! - Updates the try_seek function by multiplying the audio position by the factor
+//! - Updates the try_seek function by multiplying the audio position by the factor.
 //!
 //! To speed up a source from sink all you need to do is call the   `set_speed(factor: f32)` function
-//! For example, here is how you speed up your sound by using sink or playing raw
+//! For example, here is how you speed up your sound by using sink or playing raw:
 //!
 //! ```no_run
 //!# use std::fs::File;
@@ -28,7 +28,7 @@
 //! stream_handle.mixer().add(source.convert_samples().speed(2.0));
 //! std::thread::sleep(std::time::Duration::from_secs(5));
 //! ```
-//! here is how you would do it using the sink
+//! Here is how you would do it using the sink:
 //!```no_run
 //! use rodio::source::{Source, SineWave};
 //! let source = SineWave::new(440.0)