From 2b39d27aaf6630decdab12bb48793f2816a11cf2 Mon Sep 17 00:00:00 2001
From: dvdsk <noreply@davidsk.dev>
Date: Tue, 10 Oct 2023 02:25:34 +0200
Subject: [PATCH] adds total_duration() impl to SymphoniaDecoder, makes seek
 saturating at source end if total_duration known

---
 src/decoder/mp3.rs       |  1 +
 src/decoder/symphonia.rs | 20 +++++++++++--
 src/decoder/wav.rs       |  5 +++-
 src/source/mod.rs        |  5 +++-
 tests/seek.rs            | 64 ++++++++++++++++++++++++++--------------
 5 files changed, 69 insertions(+), 26 deletions(-)

diff --git a/src/decoder/mp3.rs b/src/decoder/mp3.rs
index 78f19e8..1ee4c06 100644
--- a/src/decoder/mp3.rs
+++ b/src/decoder/mp3.rs
@@ -74,6 +74,7 @@ where
         // as the seek only takes effect after the current frame is done
         self.decoder.seek_samples(pos)?;
         Ok(())
+    }
 }
 
 impl<R> Iterator for Mp3Decoder<R>
diff --git a/src/decoder/symphonia.rs b/src/decoder/symphonia.rs
index 7d69c31..6eef152 100644
--- a/src/decoder/symphonia.rs
+++ b/src/decoder/symphonia.rs
@@ -26,6 +26,7 @@ pub struct SymphoniaDecoder {
     decoder: Box<dyn Decoder>,
     current_frame_offset: usize,
     format: Box<dyn FormatReader>,
+    total_duration: Option<Time>,
     buffer: SampleBuffer<i16>,
     spec: SignalSpec,
 }
@@ -79,6 +80,11 @@ impl SymphoniaDecoder {
                 ..Default::default()
             },
         )?;
+        let total_duration = stream
+            .codec_params
+            .time_base
+            .zip(stream.codec_params.n_frames)
+            .map(|(base, frames)| base.calc_time(frames));
 
         let mut decode_errors: usize = 0;
         let decoded = loop {
@@ -105,6 +111,7 @@ impl SymphoniaDecoder {
             decoder,
             current_frame_offset: 0,
             format: probed.format,
+            total_duration,
             buffer,
             spec,
         }));
@@ -137,7 +144,8 @@ impl Source for SymphoniaDecoder {
 
     #[inline]
     fn total_duration(&self) -> Option<Duration> {
-        None
+        self.total_duration
+            .map(|Time { seconds, frac }| Duration::new(seconds, (1f64 / frac) as u32))
     }
 
     // TODO: do we return till where we seeked? <dvdsk noreply@davidsk.dev>
@@ -151,13 +159,21 @@ impl Source for SymphoniaDecoder {
             1f64 / pos.subsec_nanos() as f64
         };
 
+        let mut seek_to_time = Time::new(pos.as_secs(), pos_fract);
+        if let Some(total_duration) = self.total_duration() {
+            if total_duration.saturating_sub(pos).as_millis() < 1 {
+                seek_to_time = self.total_duration.unwrap();
+            }
+        }
+
         let res = self.format.seek(
             SeekMode::Accurate,
             SeekTo::Time {
-                time: Time::new(pos.as_secs(), pos_fract),
+                time: seek_to_time,
                 track_id: None,
             },
         );
+        assert!(self.total_duration().is_some());
 
         match res {
             Err(Error::IoError(e)) if e.kind() == ErrorKind::UnexpectedEof => Ok(()),
diff --git a/src/decoder/wav.rs b/src/decoder/wav.rs
index 0468db8..58e8f3d 100644
--- a/src/decoder/wav.rs
+++ b/src/decoder/wav.rs
@@ -133,7 +133,10 @@ where
     #[inline]
     fn try_seek(&mut self, pos: Duration) -> Result<(), SeekError> {
         let samples = pos.as_secs_f32() * self.sample_rate() as f32;
-        self.reader.reader.seek(samples as u32).map_err(SeekError::HoundDecoder)
+        self.reader
+            .reader
+            .seek(samples as u32)
+            .map_err(SeekError::HoundDecoder)
     }
 }
 
diff --git a/src/source/mod.rs b/src/source/mod.rs
index b5e210e..0e0c6dd 100644
--- a/src/source/mod.rs
+++ b/src/source/mod.rs
@@ -357,7 +357,10 @@ where
     ///
     /// Try to seek to a pos, returns [`SeekNotSupported`] if seeking is not
     /// supported by the current source.
-    fn try_seek(&mut self, _: Duration) -> Result<(), SeekError>;
+    ///
+    /// If the duration of the source is known and the seek position lies beyond 
+    /// it the saturates, that is the position is then at the end of the source.
+    fn try_seek(&mut self, pos: Duration) -> Result<(), SeekError>;
 }
 
 // we might add decoders requiring new error types, would non_exhaustive
diff --git a/tests/seek.rs b/tests/seek.rs
index 8daa98a..8f91c02 100644
--- a/tests/seek.rs
+++ b/tests/seek.rs
@@ -30,6 +30,10 @@ fn sink_and_decoder(format: &str) -> (Sink, Decoder<impl Read + Seek>) {
     (sink, decoder)
 }
 
+// run the following to get the other configuration: 
+// cargo test --no-default-features
+// --features symphonia-wav --features symphonia-vorbis
+// --features symphonia-flac --features symphonia-isomp4 --features minimp3
 fn format_decoder_info() -> &'static [(&'static str, bool, &'static str)] {
     &[
         #[cfg(feature = "minimp3")]
@@ -42,14 +46,16 @@ fn format_decoder_info() -> &'static [(&'static str, bool, &'static str)] {
         ("wav", true, "symphonia"),
         #[cfg(feature = "lewton")]
         ("ogg", true, "lewton"),
-        #[cfg(feature = "symphonia-vorbis")]
-        ("ogg", true, "symphonia"),
+        // note: disabled, symphonia returns error unsupported format
+        // #[cfg(feature = "symphonia-vorbis")]
+        // ("ogg", true, "symphonia"),
         #[cfg(feature = "claxon")]
         ("flac", false, "claxon"),
         #[cfg(feature = "symphonia-flac")]
         ("flac", true, "symphonia"),
-        #[cfg(feature = "symphonia-isomp4")]
-        ("m4a", true, "_"),
+        // note: disabled, symphonia returns error unsupported format
+        // #[cfg(feature = "symphonia-isomp4")]
+        // ("m4a", true, "symphonia"),
     ]
 }
 
@@ -64,17 +70,38 @@ fn seek_returns_err_if_unsupported() {
     }
 }
 
-#[test]
-fn seek_beyond_end_does_not_crash() {
+// #[ignore]
+#[test] // in the future use PR #510 (playback position) to speed this up
+fn seek_beyond_end_saturates() {
     for (format, _, decoder_name) in format_decoder_info()
         .iter()
         .cloned()
         .filter(|(_, supported, _)| *supported)
     {
         let (sink, decoder) = sink_and_decoder(format);
-        println!("seeking beyond end in: {format}\t decoded by: {decoder_name}");
         sink.append(decoder);
-        sink.try_seek(Duration::from_secs(999)).unwrap();
+
+        println!("seeking beyond end for: {format}\t decoded by: {decoder_name}");
+        let res = sink.try_seek(Duration::from_secs(999));
+        assert!(res.is_ok());
+
+        let now = Instant::now();
+        sink.sleep_until_end();
+        let elapsed = now.elapsed();
+        assert!(elapsed.as_secs() < 1);
+    }
+}
+
+fn total_duration(format: &'static str) -> Duration {
+    let (sink, decoder) = sink_and_decoder(format);
+    match decoder.total_duration() {
+        Some(d) => d,
+        None => {
+            let now = Instant::now();
+            sink.append(decoder);
+            sink.sleep_until_end();
+            now.elapsed()
+        }
     }
 }
 
@@ -86,23 +113,14 @@ fn seek_results_in_correct_remaining_playtime() {
         .cloned()
         .filter(|(_, supported, _)| *supported)
     {
-        let (sink, decoder) = sink_and_decoder(format);
-        println!("checking seek time in: {format}\t decoded by: {decoder_name}");
-        let total_duration = match decoder.total_duration() {
-            Some(d) => d,
-            None => {
-                let now = Instant::now();
-                sink.append(decoder);
-                sink.sleep_until_end();
-                now.elapsed()
-            }
-        };
+        println!("checking seek duration for: {format}\t decoded by: {decoder_name}");
 
         let (sink, decoder) = sink_and_decoder(format);
         sink.append(decoder);
 
         const SEEK_BEFORE_END: Duration = Duration::from_secs(5);
-        sink.try_seek(total_duration - SEEK_BEFORE_END).unwrap();
+        sink.try_seek(total_duration(format) - SEEK_BEFORE_END)
+            .unwrap();
 
         let now = Instant::now();
         sink.sleep_until_end();
@@ -110,9 +128,11 @@ fn seek_results_in_correct_remaining_playtime() {
         let expected = SEEK_BEFORE_END;
 
         if elapsed.as_millis().abs_diff(expected.as_millis()) > 250 {
-            panic!("Seek did not result in expected leftover playtime
+            panic!(
+                "Seek did not result in expected leftover playtime
     leftover time: {elapsed:?}
-    expected time left in source: {SEEK_BEFORE_END:?}");
+    expected time left in source: {SEEK_BEFORE_END:?}"
+            );
         }
     }
 }