From d9d7606560f21d66545705c63a883f24d06ac96a Mon Sep 17 00:00:00 2001 From: Petr Gladkikh Date: Fri, 29 Nov 2024 20:21:15 +0400 Subject: [PATCH] Remove explicit overflow check in sample interpolation --- Cargo.toml | 1 + src/conversions/sample.rs | 47 +++++++++++------------------ src/conversions/sample_rate.rs | 55 ++++++++++++++++++++-------------- 3 files changed, 51 insertions(+), 52 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a8d03f9..599cb05 100644 --- a/Cargo.toml +++ b/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"] diff --git a/src/conversions/sample.rs b/src/conversions/sample.rs index ec80460..3164f75 100644 --- a/src/conversions/sample.rs +++ b/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) } } } diff --git a/src/conversions/sample_rate.rs b/src/conversions/sample_rate.rs index 9e1e41e..26e0d90 100644 --- a/src/conversions/sample_rate.rs +++ b/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, 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, 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, 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::>(); + assert_eq!(output, [0, 5, 10, 15]); + assert!((size_estimation as f32 / output.len() as f32).abs() < 2.0); + } }