Remove explicit overflow check in sample interpolation

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"]

src/conversions/sample.rs

@@ -74,8 +74,10 @@ where
 pub trait Sample: CpalSample {
     /// Linear interpolation between two samples.
     ///
-    /// The result should be equvivalent to
+    /// The result should be equivalent to
     /// `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;
@@ -93,9 +95,11 @@ pub trait Sample: CpalSample {
 impl Sample for u16 {
     #[inline]
     fn lerp(first: u16, second: u16, numerator: u32, denominator: u32) -> 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")
+        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
     }
 
     #[inline]
@@ -123,9 +127,8 @@ impl Sample for u16 {
 impl Sample for i16 {
     #[inline]
     fn lerp(first: i16, second: i16, numerator: u32, denominator: u32) -> 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")
+        (first as i32 + (second as i32 - first as i32) * numerator as i32 / denominator as i32)
+            as i16
     }
 
     #[inline]
@@ -181,6 +184,7 @@ impl Sample for f32 {
 #[cfg(test)]
 mod test {
     use super::*;
+    use num_rational::Ratio;
     use quickcheck::{quickcheck, TestResult};
 
     #[test]
@@ -200,12 +204,6 @@ mod test {
         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;
@@ -224,29 +222,20 @@ mod test {
         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 {
+        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, denominator) as f64;
-            let diff = x - reference;
-            TestResult::from_bool(diff.abs() < 1.0)
+            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` or `to` are equal to 0.
-    ///
+    /// Panics if `from`, `to` or `num_channels` are 0.
     #[inline]
     pub fn new(
         mut input: I,
@@ -54,23 +62,8 @@ where
         assert!(from >= 1);
         assert!(to >= 1);
 
-        // finding the 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
@@ -84,10 +77,13 @@ where
             (first, next)
         };
 
+        // Reducing nominator to avoid numeric overflows during interpolation.
+        let (to, from) = Ratio::new(to, from).into_raw();
+
         SampleRateConverter {
             input,
-            from: from / gcd,
-            to: to / gcd,
+            from,
+            to,
             channels: num_channels,
             current_frame_pos_in_chunk: 0,
             next_output_frame_pos_in_chunk: 0,
@@ -296,9 +292,10 @@ mod test {
         /// Check that dividing the sample rate by k (integer) is the same as
         ///   dropping a sample from each channel.
         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() {
+            if k == 0 || channels == 0 || channels > 128 || to == 0 || to > 48000 {
                 return TestResult::discard();
             }
+
             let to = SampleRate(to as u32);
             let from = to * k as u32;
 
@@ -320,10 +317,11 @@ mod test {
 
         /// 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: u16, k: u16, input: Vec<u16>, channels: u8) -> TestResult {
-            if k == 0 || channels == 0 || channels > 128 || from == 0 || from.checked_mul(k).is_none() {
+        fn multiply_sample_rate(from: u16, k: u8, input: Vec<u16>, channels: u8) -> TestResult {
+            if k == 0 || channels == 0 || channels > 128 || from == 0 {
                 return TestResult::discard();
             }
+
             let from = SampleRate(from as u32);
             let to = from * k as u32;
 
@@ -388,4 +386,15 @@ mod test {
         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);
+    }
 }