From 055fa87ad8117d786c17762d15960c7b8b6c2d97 Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Tue, 3 Jun 2025 23:08:52 +0200 Subject: [PATCH 1/4] Improve seeking behavior and error handling - Automatically enable is_seekable when byte_len is set - Add specific error for random access seeking without byte_len - Clarify documentation for builder methods with default states - Test forward-only vs random access seeking scenarios Fixes #740 Supersedes #744 --- src/decoder/builder.rs | 10 ++++++--- src/decoder/symphonia.rs | 47 ++++++++++++++++++++++++++-------------- tests/mp4a_test.rs | 3 ++- tests/seek.rs | 31 +++++++++++++++++++++++++- 4 files changed, 70 insertions(+), 21 deletions(-) diff --git a/src/decoder/builder.rs b/src/decoder/builder.rs index 60353127..b6a7dcc8 100644 --- a/src/decoder/builder.rs +++ b/src/decoder/builder.rs @@ -165,6 +165,8 @@ impl DecoderBuilder { /// - Reliable seeking operations /// - Duration calculations in formats that lack timing information (e.g. MP3, Vorbis) /// + /// Note that this also sets `is_seekable` to `true`. + /// /// The byte length should typically be obtained from file metadata: /// ```no_run /// use std::fs::File; @@ -189,10 +191,11 @@ impl DecoderBuilder { /// incorrect duration calculations and seeking errors. pub fn with_byte_len(mut self, byte_len: u64) -> Self { self.settings.byte_len = Some(byte_len); + self.settings.is_seekable = true; self } - /// Enables or disables coarse seeking. + /// Enables or disables coarse seeking. This is disabled by default. /// /// This needs `byte_len` to be set. Coarse seeking is faster but less accurate: /// it may seek to a position slightly before or after the requested one, @@ -202,7 +205,7 @@ impl DecoderBuilder { self } - /// Enables or disables gapless playback. + /// Enables or disables gapless playback. This is enabled by default. /// /// When enabled, removes silence between tracks for formats that support it. pub fn with_gapless(mut self, gapless: bool) -> Self { @@ -228,7 +231,8 @@ impl DecoderBuilder { self } - /// Configure whether the decoder should report as seekable. + /// Configure whether the data supports random access seeking. Without this, + /// only forward seeking may work. /// /// For reliable seeking behavior, `byte_len` should be set either from file metadata /// or by seeking to the end of the stream. While seeking may work without `byte_len` diff --git a/src/decoder/symphonia.rs b/src/decoder/symphonia.rs index 78e82acd..5fe52a54 100644 --- a/src/decoder/symphonia.rs +++ b/src/decoder/symphonia.rs @@ -199,16 +199,20 @@ impl Source for SymphoniaDecoder { // Remember the current channel, so we can restore it after seeking. let active_channel = self.current_span_offset % self.channels() as usize; - let seek_res = self - .format - .seek( - self.seek_mode, - SeekTo::Time { - time: target.into(), - track_id: None, - }, - ) - .map_err(SeekError::BaseSeek)?; + let seek_res = match self.format.seek( + self.seek_mode, + SeekTo::Time { + time: target.into(), + track_id: None, + }, + ) { + Err(Error::SeekError(symphonia::core::errors::SeekErrorKind::ForwardOnly)) => { + return Err(source::SeekError::SymphoniaDecoder( + SeekError::RandomAccessNotSupported, + )); + } + other => other.map_err(SeekError::Demuxer), + }?; // Seeking is a demuxer operation without the decoder knowing about it, // so we need to reset the decoder to make sure it's in sync and prevent @@ -242,18 +246,28 @@ pub enum SeekError { /// This error occurs when the decoder cannot extract time base information from the source. /// You may catch this error to try a coarse seek instead. AccurateSeekNotSupported, - /// Format reader failed to seek - BaseSeek(symphonia::core::errors::Error), + /// The decoder does not support random access seeking + /// + /// This error occurs when the source is not seekable or does not have a known byte length. + RandomAccessNotSupported, + /// Demuxer failed to seek + Demuxer(symphonia::core::errors::Error), } impl fmt::Display for SeekError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { SeekError::AccurateSeekNotSupported => { - write!(f, "Accurate seeking is not supported") + write!( + f, + "Accurate seeking is not supported on this file/byte stream that lacks time base information" + ) + } + SeekError::RandomAccessNotSupported => { + write!(f, "The decoder needs to know the length of the file/byte stream to be able to seek backwards. You can set that by using the `DecoderBuilder` or creating a decoder using `Decoder::try_from(some_file)`.") } - SeekError::BaseSeek(err) => { - write!(f, "Format reader failed to seek: {:?}", err) + SeekError::Demuxer(err) => { + write!(f, "Demuxer failed to seek: {:?}", err) } } } @@ -263,7 +277,8 @@ impl std::error::Error for SeekError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { SeekError::AccurateSeekNotSupported => None, - SeekError::BaseSeek(err) => Some(err), + SeekError::RandomAccessNotSupported => None, + SeekError::Demuxer(err) => Some(err), } } } diff --git a/tests/mp4a_test.rs b/tests/mp4a_test.rs index 62435447..910e7d07 100644 --- a/tests/mp4a_test.rs +++ b/tests/mp4a_test.rs @@ -8,6 +8,7 @@ fn test_mp4a_encodings() { // Licensed under Creative Commons: By Attribution 3.0 // http://creativecommons.org/licenses/by/3.0/ let file = std::fs::File::open("assets/monkeys.mp4a").unwrap(); - let mut decoder = rodio::Decoder::try_from(file).unwrap(); + // Open with `new` instead of `try_from` to ensure it works even without is_seekable + let mut decoder = rodio::Decoder::new(file).unwrap(); assert!(decoder.any(|x| x != 0.0)); // Assert not all zeros } diff --git a/tests/seek.rs b/tests/seek.rs index 64f0963a..697db16e 100644 --- a/tests/seek.rs +++ b/tests/seek.rs @@ -239,7 +239,36 @@ fn seek_does_not_break_channel_order( } } -fn second_channel_beep_range(source: &mut R) -> std::ops::Range { +#[test] +fn random_access_seeks() { + // Decoder::new:: does *not* set byte_len and is_seekable + let mp3_file = std::fs::File::open("assets/music.mp3").unwrap(); + let mut decoder = Decoder::new(mp3_file).unwrap(); + assert!( + decoder.try_seek(Duration::from_secs(2)).is_ok(), + "forward seek should work without byte_len" + ); + assert!( + decoder.try_seek(Duration::from_secs(1)).is_err(), + "backward seek should fail without byte_len" + ); + + // Decoder::try_from:: sets byte_len and is_seekable + let mut decoder = get_music("mp3"); + assert!( + decoder.try_seek(Duration::from_secs(2)).is_ok(), + "forward seek should work with byte_len" + ); + assert!( + decoder.try_seek(Duration::from_secs(1)).is_ok(), + "backward seek should work with byte_len" + ); +} + +fn second_channel_beep_range(source: &mut R) -> std::ops::Range +where + R: Iterator, +{ let channels = source.channels() as usize; let samples: Vec = source.by_ref().collect(); From 87f15d2e4227114d694be7becbeb6e13b228bc27 Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Wed, 4 Jun 2025 21:48:56 +0200 Subject: [PATCH 2/4] Add symphonia-mp3 feature gating and improve seek error check --- tests/seek.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/seek.rs b/tests/seek.rs index 697db16e..9070d5e8 100644 --- a/tests/seek.rs +++ b/tests/seek.rs @@ -1,6 +1,8 @@ #![allow(dead_code)] #![allow(unused_imports)] +#[cfg(feature = "symphonia-mp3")] +use rodio::{decoder::symphonia, source::SeekError}; use rodio::{ChannelCount, Decoder, Source}; use rstest::rstest; use rstest_reuse::{self, *}; @@ -239,6 +241,7 @@ fn seek_does_not_break_channel_order( } } +#[cfg(feature = "symphonia-mp3")] #[test] fn random_access_seeks() { // Decoder::new:: does *not* set byte_len and is_seekable @@ -249,7 +252,12 @@ fn random_access_seeks() { "forward seek should work without byte_len" ); assert!( - decoder.try_seek(Duration::from_secs(1)).is_err(), + matches!( + decoder.try_seek(Duration::from_secs(1)), + Err(SeekError::SymphoniaDecoder( + symphonia::SeekError::RandomAccessNotSupported, + )) + ), "backward seek should fail without byte_len" ); From a32fe7f78ea07c40492d0204151355b905df5c30 Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Wed, 4 Jun 2025 21:49:52 +0200 Subject: [PATCH 3/4] Add missing required features for examples --- Cargo.toml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 6630b82e..35034392 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -108,6 +108,22 @@ required-features = ["playback", "wav"] name = "custom_config" required-features = ["playback", "wav"] +[[example]] +name = "distortion" +required-features = ["playback"] + +[[example]] +name = "distortion_mp3" +required-features = ["playback", "mp3"] + +[[example]] +name = "distortion_wav" +required-features = ["playback", "wav"] + +[[example]] +name = "distortion_wav_alternate" +required-features = ["playback", "wav"] + [[example]] name = "error_callback" required-features = ["playback"] @@ -124,6 +140,10 @@ required-features = ["playback", "wav"] name = "mix_multiple_sources" required-features = ["playback"] +[[example]] +name = "music_flac" +required-features = ["playback", "flac"] + [[example]] name = "music_m4a" required-features = ["playback", "symphonia-isomp4", "symphonia-aac"] From 3fba45f845f3287c496ff0cdfc8b61327e5f3374 Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Wed, 4 Jun 2025 21:55:08 +0200 Subject: [PATCH 4/4] Remove unnecessary second_channel_beep_range trait bound --- tests/seek.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/seek.rs b/tests/seek.rs index 9070d5e8..3b9113b9 100644 --- a/tests/seek.rs +++ b/tests/seek.rs @@ -273,10 +273,7 @@ fn random_access_seeks() { ); } -fn second_channel_beep_range(source: &mut R) -> std::ops::Range -where - R: Iterator, -{ +fn second_channel_beep_range(source: &mut R) -> std::ops::Range { let channels = source.channels() as usize; let samples: Vec = source.by_ref().collect();