(re)add tests for skip_duration, fixes skip duration

also adds a new mod test support with a special test source that
can be used to test span boundary/parameters_changed handling.

adds two extra tests for `skip_duration` testing
 - parameters_changing during skip
 - ending on frame boundary

Author: dvdsk

src/math.rs

@@ -10,6 +10,30 @@ pub fn lerp(first: &f32, second: &f32, numerator: u32, denominator: u32) -> f32
     first + (second - first) * numerator as f32 / denominator as f32
 }
 
+/// will hopefully get stabilized, this is slightly different to the future
+/// std's version since it does some casting already. When the std's version gets
+/// stable remove this trait.
+pub(crate) trait PrevMultipleOf {
+    fn prev_multiple_of(self, n: u16) -> Self;
+}
+
+macro_rules! impl_prev_multiple_of {
+    ($type:ty) => {
+        impl PrevMultipleOf for $type {
+            fn prev_multiple_of(self, n: u16) -> $type {
+                if self.next_multiple_of(n as $type) > self {
+                    self.next_multiple_of(n as $type) - n as $type
+                } else {
+                    self.next_multiple_of(n as $type)
+                }
+            }
+        }
+    };
+}
+
+impl_prev_multiple_of! {usize}
+impl_prev_multiple_of! {u64}
+
 #[cfg(test)]
 mod test {
     use super::*;

src/source/from_iter.rs

@@ -87,7 +87,7 @@ where
         // If the `size_hint` is `None` as well, we are in the worst case scenario. To handle this
         // situation we force a span to have a maximum number of samples indicate by this
         // constant.
-        
+
         todo!()
         // const THRESHOLD: usize = 10240;
         //

src/source/mod.rs

@@ -33,7 +33,7 @@ pub use self::repeat::Repeat;
 pub use self::sawtooth::SawtoothWave;
 pub use self::signal_generator::{Function, SignalGenerator};
 pub use self::sine::SineWave;
-pub use self::skip::SkipDuration;
+pub use self::skip_duration::SkipDuration;
 pub use self::skippable::Skippable;
 pub use self::spatial::Spatial;
 pub use self::speed::Speed;
@@ -69,7 +69,7 @@ mod repeat;
 mod sawtooth;
 mod signal_generator;
 mod sine;
-mod skip;
+mod skip_duration;
 mod skippable;
 mod spatial;
 mod speed;
@@ -252,7 +252,7 @@ pub trait Source: Iterator<Item = Sample> {
     where
         Self: Sized,
     {
-        skip::skip_duration(self, duration)
+        SkipDuration::new(self, duration)
     }
 
     /// Amplifies the sound by the given value.

src/source/skip.rs renamed to src/source/skip_duration.rs

@@ -2,29 +2,29 @@ use std::time::Duration;
 
 use super::SeekError;
 use crate::common::{ChannelCount, SampleRate};
+use crate::math::PrevMultipleOf;
 use crate::Source;
 
-const US_PER_SECOND: u64 = 1_000_000;
-
-/// Internal function that builds a `SkipDuration` object.
-pub fn skip_duration<I>(mut input: I, duration: Duration) -> SkipDuration<I>
-where
-    I: Source,
-{
-    do_skip_duration(&mut input, duration);
-    SkipDuration {
-        input,
-        skipped_duration: duration,
-    }
-}
-
 /// A source that skips specified duration of the given source from it's current position.
 #[derive(Clone, Debug)]
 pub struct SkipDuration<I> {
     input: I,
     skipped_duration: Duration,
 }
 
+impl<I> SkipDuration<I> {
+    pub(crate) fn new(mut input: I, duration: Duration) -> SkipDuration<I>
+    where
+        I: Source,
+    {
+        do_skip_duration(&mut input, duration);
+        Self {
+            input,
+            skipped_duration: duration,
+        }
+    }
+}
+
 impl<I> SkipDuration<I>
 where
     I: Source,
@@ -99,26 +99,46 @@ where
 }
 
 /// Skips specified `duration` of the given `input` source from it's current position.
-fn do_skip_duration<I>(input: &mut I, mut duration: Duration)
+///
+/// # Panics
+/// If trying to skip more than 584 days ahead. If you need that functionality
+/// please open an issue I would love to know why (and help out).
+fn do_skip_duration<I>(input: &mut I, duration: Duration)
 where
     I: Source,
 {
-    while !duration.is_zero() {
-        let us_per_sample: u64 =
-            US_PER_SECOND / input.sample_rate() as u64 / input.channels() as u64;
-        let mut samples_to_skip = duration.as_micros() as u64 / us_per_sample;
-
-        while samples_to_skip > 0 && !input.parameters_changed() {
-            samples_to_skip -= 1;
+    const NS_PER_SECOND: u64 = 1_000_000_000;
+
+    // `u64::MAX` can store 584 days of nanosecond precision time. To not be off by
+    // a single sample (that would be regression) we first multiply the time by
+    // `samples_per second`. Which for a 96kHz 10 channel audio stream is
+    // 960_000 samples. That would only leave 0.87 hour of potential skip time. Hence
+    // we use an `u128` to calculate samples to skip.
+    let mut duration: u64 = duration
+        .as_nanos()
+        .try_into()
+        .expect("can not skip more then 584 days of audio");
+    let mut ns_per_frame: u64 = 0;
+
+    while duration > ns_per_frame {
+        ns_per_frame = NS_PER_SECOND / input.sample_rate() as u64;
+
+        let samples_per_second = input.sample_rate() as u64 * input.channels() as u64;
+        let samples_to_skip =
+            (duration as u128 * samples_per_second as u128 / NS_PER_SECOND as u128) as u64;
+        let samples_to_skip = samples_to_skip.prev_multiple_of(input.channels());
+
+        let mut skipped = 0;
+        while skipped < samples_to_skip {
             if input.next().is_none() {
                 return;
             }
+            skipped += 1;
+            if input.parameters_changed() {
+                break;
+            }
         }
 
-        if samples_to_skip == 0 {
-            return;
-        } else {
-            duration -= Duration::from_micros(samples_to_skip * us_per_sample);
-        }
+        duration -= skipped * NS_PER_SECOND / samples_per_second;
     }
 }

src/source/take_samples.rs

@@ -2,6 +2,7 @@ use std::time::Duration;
 
 use super::SeekError;
 use crate::common::{ChannelCount, SampleRate};
+use crate::math::PrevMultipleOf;
 use crate::Source;
 
 /// A source that truncates the given source to a certain duration.
@@ -84,13 +85,3 @@ where
         self.input.try_seek(pos)
     }
 }
-
-trait PrevMultipleOf {
-    fn prev_multiple_of(self, n: u16) -> usize;
-}
-
-impl PrevMultipleOf for usize {
-    fn prev_multiple_of(self, n: u16) -> usize {
-        self.next_multiple_of(n as usize).saturating_sub(n as usize)
-    }
-}

tests/skip_duration.rs

@@ -0,0 +1,85 @@
+use std::time::Duration;
+
+use rodio::buffer::SamplesBuffer;
+use rodio::source::Source;
+use rodio::{ChannelCount, SampleRate};
+use rstest::rstest;
+
+mod test_support;
+use test_support::{TestSource, TestSpan};
+
+#[test]
+fn ends_on_frame_boundary() {
+    let source = TestSource::new().with_span(
+        TestSpan::silence(30)
+            .with_channel_count(10)
+            .with_sample_rate(1),
+    );
+    let leftover = source
+        .clone()
+        .skip_duration(Duration::from_secs_f32(1.483))
+        .count();
+    assert!(leftover % source.channels() as usize == 0)
+}
+
+#[rstest]
+fn param_changes_during_skip(#[values(6, 11)] seconds_to_skip: u64) {
+    let source = TestSource::new() // each span is 5 seconds
+        .with_span(
+            TestSpan::silence(5 * 10 * 1)
+                .with_sample_rate(10)
+                .with_channel_count(1),
+        )
+        .with_span(
+            TestSpan::silence(5 * 20 * 2)
+                .with_sample_rate(20)
+                .with_channel_count(2),
+        )
+        .with_span(
+            TestSpan::silence(5 * 5 * 3)
+                .with_sample_rate(5)
+                .with_channel_count(3),
+        );
+
+    let leftover = source
+        .clone()
+        .skip_duration(Duration::from_secs(seconds_to_skip))
+        .count();
+
+    let spans = source.spans;
+    let expected_leftover = match seconds_to_skip {
+        6 => 4 * spans[1].sample_rate as usize * spans[1].channels as usize + spans[2].len(),
+        11 => 4 * spans[2].sample_rate as usize * spans[2].channels as usize,
+        _ => unreachable!(),
+    };
+
+    assert_eq!(leftover, expected_leftover);
+}
+
+#[rstest]
+fn samples_left(
+    #[values(1, 2, 4)] channels: ChannelCount,
+    #[values(100_000)] sample_rate: SampleRate,
+    #[values(0, 5)] seconds: u32,
+    #[values(0, 3, 5)] seconds_to_skip: u32,
+) {
+    println!(
+        "channels: {channels}, sample_rate: {sample_rate}, \
+        seconds: {seconds}, seconds_to_skip: {seconds_to_skip}"
+    );
+    let buf_len = (sample_rate * channels as u32 * seconds) as usize;
+    assert!(buf_len < 10 * 1024 * 1024);
+    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);
+
+    let samples_left_expected = (sample_rate * channels as u32 * seconds_left) as usize;
+    let samples_left = test_buffer
+        .skip_duration(Duration::from_secs(seconds_to_skip as u64))
+        .count();
+
+    assert!(
+        samples_left == samples_left_expected,
+        "expected {samples_left_expected} samples left, counted: {samples_left}"
+    );
+}