Test fixes, upgrade quickcheck

.github/workflows/ci.yml

@@ -47,7 +47,9 @@ 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 --all-targets --features=experimental
+      - run: cargo test --all-targets --features=symphonia-all
+      - run: cargo test --doc
   cargo-publish:
     if: github.event_name == 'push' && github.ref == 'refs/heads/master'
     env:

Cargo.toml

@@ -47,7 +47,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

@@ -18,27 +18,42 @@ fn main() {
     // Apply automatic gain control to the source
     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`
-    // 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| {
-        agc_source.set_enabled(agc_enabled_clone.load(Ordering::Relaxed));
-    });
-
-    // Add the source now equipped with automatic gain control and controlled via
-    // periodic_access to the sink for playback
-    sink.append(controlled);
-
-    // after 5 seconds of playback disable automatic gain control using the
+    let agc_enabled: Arc<AtomicBool>;
+
+    #[cfg(not(feature = "experimental"))]
+    {
+        agc_enabled = Arc::new(AtomicBool::new(true));
+        // Make it so that the source checks if automatic gain control should be
+        // 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_clone = agc_enabled.clone();
+        let controlled = agc_source.periodic_access(Duration::from_millis(5), move |agc_source| {
+            agc_source.set_enabled(agc_enabled_clone.load(Ordering::Relaxed));
+        });
+
+        // Add the source now equipped with automatic gain control and controlled via
+        // periodic_access to the sink for playback
+        sink.append(controlled);
+    }
+    #[cfg(feature = "experimental")]
+    {
+        agc_enabled = agc_source.get_agc_control();
+        sink.append(agc_source);
+    }
+
+    // After 5 seconds of playback disable automatic gain control using the
     // shared AtomicBool `agc_enabled`. You could do this from another part
     // of the program since `agc_enabled` is of type Arc<AtomicBool> which
     // is freely clone-able and move-able.
     //
     // Note that disabling the AGC takes up to 5 millis because periodic_access
     // controls the source every 5 millis.
     thread::sleep(Duration::from_secs(5));
+    #[cfg(not(feature = "experimental"))]
+    agc_enabled.store(false, Ordering::Relaxed);
+
+    // AGC on/off control using direct access to the boolean variable.
+    #[cfg(feature = "experimental")]
     agc_enabled.store(false, Ordering::Relaxed);
 
     // Keep the program running until playback is complete

src/buffer.rs

@@ -42,8 +42,8 @@ where
     where
         D: Into<Vec<S>>,
     {
-        assert!(channels != 0);
-        assert!(sample_rate != 0);
+        assert!(channels >= 1);
+        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,13 @@ where
 pub trait Sample: CpalSample {
     /// Linear interpolation between two samples.
     ///
-    /// The result should be equal to
-    /// `first * numerator / denominator + second * (1 - numerator / denominator)`.
+    /// The result should be equvivalent to
+    /// `first * (1 - numerator / denominator) + second * numerator / denominator`.
     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.
@@ -93,11 +93,9 @@ pub trait Sample: CpalSample {
 impl Sample for u16 {
     #[inline]
     fn lerp(first: u16, second: u16, numerator: u32, denominator: u32) -> u16 {
-        let a = first as i32;
-        let b = second as i32;
-        let n = numerator as i32;
-        let d = denominator as i32;
-        (a + (b - a) * n / d) as u16
+        let sample =
+            first as i64 + (second as i64 - first as i64) * numerator as i64 / denominator as i64;
+        u16::try_from(sample).expect("numerator / denominator is within [0, 1] range")
     }
 
     #[inline]
@@ -125,8 +123,9 @@ impl Sample for u16 {
 impl Sample for i16 {
     #[inline]
     fn lerp(first: i16, second: i16, numerator: u32, denominator: u32) -> i16 {
-        (first as i32 + (second as i32 - first as i32) * numerator as i32 / denominator as i32)
-            as i16
+        let sample =
+            first as i64 + (second as i64 - first as i64) * numerator as i64 / denominator as i64;
+        i16::try_from(sample).expect("numerator / denominator is within [0, 1] range")
     }
 
     #[inline]
@@ -178,3 +177,76 @@ impl Sample for f32 {
         0.0
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    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]
+    #[should_panic]
+    fn lerp_u16_overflow() {
+        Sample::lerp(0u16, 1, u16::MAX as u32 + 1, 1);
+    }
+
+    #[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);
+    }
+
+    #[test]
+    #[should_panic]
+    fn lerp_i16_overflow_max() {
+        Sample::lerp(0i16, 1, i16::MAX as u32 + 1, 1);
+    }
+
+    #[test]
+    #[should_panic]
+    fn lerp_i16_overflow_min() {
+        Sample::lerp(0i16, -1, (i16::MIN.abs() + 1) as u32, 1);
+    }
+
+    quickcheck! {
+        fn lerp_u16_random(first: u16, second: u16, numerator: u32, denominator: u32) -> TestResult {
+            if denominator == 0 { 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, denominator) as f64;
+            let diff = x - reference;
+            TestResult::from_bool(diff.abs() < 1.0)
+        }
+    }
+}

src/conversions/sample_rate.rs

@@ -50,10 +50,11 @@ where
         let from = from.0;
         let to = to.0;
 
+        assert!(num_channels >= 1);
         assert!(from >= 1);
         assert!(to >= 1);
 
-        // finding greatest common divisor
+        // finding the greatest common divisor
         let gcd = {
             #[inline]
             fn gcd(a: u32, b: u32) -> u32 {
@@ -256,99 +257,103 @@ where
 mod test {
     use super::SampleRateConverter;
     use core::time::Duration;
-    use cpal::SampleRate;
-    use quickcheck::quickcheck;
-
-    // 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)
-    }
+    use cpal::{ChannelCount, SampleRate};
+    use quickcheck::{quickcheck, TestResult};
 
     quickcheck! {
         /// Check that resampling an empty input produces no output.
-        fn empty(from: u32, to: u32, n: u16) -> () {
-            let from = if from == 0 { return; } else { SampleRate(from) };
-            let to   = if   to == 0 { return; } else { SampleRate(to)   };
-            if n == 0 { return; }
+        fn empty(from: u16, to: u16, channels: u8) -> TestResult {
+            if channels == 0 || channels > 128
+                || from == 0
+                || 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>) -> () {
-            let from = if from == 0 { return; } else { SampleRate(from) };
-            if n == 0 { return; }
+        fn identity(from: u16, channels: u8, input: Vec<u16>) -> TestResult {
+            if channels == 0 || channels > 128 || from == 0 { return TestResult::discard(); }
+            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) -> () {
-            let to = if to == 0 { return; } else { SampleRate(to) };
-            let from = multiply_rate(to, k);
-            if k == 0 || n == 0 { return; }
+        fn divide_sample_rate(to: u16, k: u16, input: Vec<u16>, channels: u8) -> TestResult {
+            if k == 0 || channels == 0 || channels > 128 || to == 0 || to.checked_mul(k).is_none() {
+                return TestResult::discard();
+            }
+            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())
-                         .step_by(k as usize).collect::<Vec<_>>().concat(),
-                       output)
+            TestResult::from_bool(input.chunks_exact(channels.into())
+                         .step_by(k as usize).collect::<Vec<_>>().concat() == 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) -> () {
-            let from = if from == 0 { return; } else { SampleRate(from) };
-            let to = multiply_rate(from, k);
-            if k == 0 || n == 0 { return; }
+        fn multiply_sample_rate(from: u16, k: u16, input: Vec<u16>, channels: u8) -> TestResult {
+            if k == 0 || channels == 0 || channels > 128 || from == 0 || from.checked_mul(k).is_none() {
+                return TestResult::discard();
+            }
+            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,
-                       output.chunks_exact(n.into())
-                         .step_by(k as usize).collect::<Vec<_>>().concat()
-            )
+            TestResult::from_bool(input ==
+                       output.chunks_exact(channels.into())
+                         .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 +363,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),
-                    "Resampled duration ({:?}) is not close to original ({:?}); Δ = {:?}",
-                    duration, d, delta);
+            TestResult::from_bool(delta < Duration::from_millis(1))
         }
     }
 
@@ -369,9 +372,20 @@ 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);
+    }
 }