diff --git a/Cargo.toml b/Cargo.toml index 28d1cf01..92c53c2d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ hound = { version = "3.3.1", optional = true } lewton = { version = "0.10", optional = true } minimp3_fixed = { version = "0.5.4", optional = true } symphonia = { version = "0.5.4", optional = true, default-features = false } +# symphonia = { git = "https://github.com/roderickvd/Symphonia", branch = "perf/faster-seeking", optional = true } crossbeam-channel = { version = "0.5.15", optional = true } rand = { version = "0.9.0", features = ["small_rng", "os_rng"], optional = true } diff --git a/src/decoder/builder.rs b/src/decoder/builder.rs index 60353127..f4573825 100644 --- a/src/decoder/builder.rs +++ b/src/decoder/builder.rs @@ -74,9 +74,6 @@ pub struct Settings { /// An MIME type hint for the decoder about the format of the stream. /// When known, this can help the decoder to select the correct demuxer. pub(crate) mime_type: Option, - - /// Whether the decoder should report as seekable. - pub(crate) is_seekable: bool, } impl Default for Settings { @@ -87,7 +84,6 @@ impl Default for Settings { gapless: true, hint: None, mime_type: None, - is_seekable: false, } } } @@ -228,37 +224,6 @@ impl DecoderBuilder { self } - /// Configure whether the decoder should report as seekable. - /// - /// 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` - /// for some formats, it is not guaranteed. - /// - /// # Examples - /// ```no_run - /// use std::fs::File; - /// use rodio::Decoder; - /// - /// fn main() -> Result<(), Box> { - /// let file = File::open("audio.mp3")?; - /// let len = file.metadata()?.len(); - /// - /// // Recommended: Set both byte_len and seekable - /// let decoder = Decoder::builder() - /// .with_data(file) - /// .with_byte_len(len) - /// .with_seekable(true) - /// .build()?; - /// - /// // Use the decoder... - /// Ok(()) - /// } - /// ``` - pub fn with_seekable(mut self, is_seekable: bool) -> Self { - self.settings.is_seekable = is_seekable; - self - } - /// Creates the decoder implementation with configured settings. fn build_impl(self) -> Result<(DecoderImpl, Settings), DecoderError> { let data = self.data.ok_or(DecoderError::UnrecognizedFormat)?; diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index ef2486e3..eb05d604 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -25,7 +25,6 @@ //! let decoder = Decoder::builder() //! .with_data(file) //! .with_byte_len(len) // Enable seeking and duration calculation -//! .with_seekable(true) // Enable seeking operations //! .with_hint("mp3") // Optional format hint //! .with_gapless(true) // Enable gapless playback //! .build() @@ -276,7 +275,6 @@ impl TryFrom for Decoder> { Self::builder() .with_data(BufReader::new(file)) .with_byte_len(len) - .with_seekable(true) .build() } } diff --git a/src/decoder/read_seek_source.rs b/src/decoder/read_seek_source.rs index d3a11252..cc650ebc 100644 --- a/src/decoder/read_seek_source.rs +++ b/src/decoder/read_seek_source.rs @@ -14,8 +14,6 @@ pub struct ReadSeekSource { /// Optional length of the media source in bytes. /// When known, this can help with seeking and duration calculations. byte_len: Option, - /// Whether this media source reports as seekable. - is_seekable: bool, } impl ReadSeekSource { @@ -29,16 +27,15 @@ impl ReadSeekSource { ReadSeekSource { inner, byte_len: settings.byte_len, - is_seekable: settings.is_seekable, } } } impl MediaSource for ReadSeekSource { - /// Returns whether this media source reports as seekable. + /// symphonia sources are only seekable if `byte_len.is_some()` #[inline] fn is_seekable(&self) -> bool { - self.is_seekable + self.byte_len.is_some() } /// Returns the total length of the media source in bytes, if known. diff --git a/src/decoder/symphonia.rs b/src/decoder/symphonia.rs index 78e82acd..d9c8d06f 100644 --- a/src/decoder/symphonia.rs +++ b/src/decoder/symphonia.rs @@ -6,7 +6,7 @@ use symphonia::{ codecs::{Decoder, DecoderOptions, CODEC_TYPE_NULL}, errors::Error, formats::{FormatOptions, FormatReader, SeekMode, SeekTo, SeekedTo}, - io::MediaSourceStream, + io::{MediaSource, MediaSourceStream}, meta::MetadataOptions, probe::Hint, units, @@ -28,6 +28,8 @@ pub(crate) struct SymphoniaDecoder { buffer: SampleBuffer, spec: SignalSpec, seek_mode: SeekMode, + /// Symphonia can only seek if bytes_len is availabile. When it is then this is true + can_seek: bool, } impl SymphoniaDecoder { @@ -36,8 +38,8 @@ impl SymphoniaDecoder { Err(e) => match e { Error::IoError(e) => Err(DecoderError::IoError(e.to_string())), Error::DecodeError(e) => Err(DecoderError::DecodeError(e)), - Error::SeekError(_) => { - unreachable!("Seek errors should not occur during initialization") + Error::SeekError(e) => { + unreachable!("Seek errors should not occur during initialization, got: {e:?}") } Error::Unsupported(_) => Err(DecoderError::UnrecognizedFormat), Error::LimitError(e) => Err(DecoderError::LimitError(e)), @@ -74,6 +76,7 @@ impl SymphoniaDecoder { } else { SeekMode::Accurate }; + let can_seek = mss.byte_len().is_some(); let mut probed = get_probe().format(&hint, mss, &format_opts, &metadata_opts)?; let stream = match probed.format.default_track() { @@ -145,6 +148,7 @@ impl SymphoniaDecoder { buffer, spec, seek_mode, + can_seek, })) } @@ -179,6 +183,10 @@ impl Source for SymphoniaDecoder { } fn try_seek(&mut self, pos: Duration) -> Result<(), source::SeekError> { + if !self.can_seek { + return Err(source::SeekError::NeedsBytesLen); + } + if matches!(self.seek_mode, SeekMode::Accurate) && self.decoder.codec_params().time_base.is_none() { diff --git a/src/source/mod.rs b/src/source/mod.rs index 9901be04..f097286c 100644 --- a/src/source/mod.rs +++ b/src/source/mod.rs @@ -614,6 +614,8 @@ pub enum SeekError { /// The source that did not support seek underlying_source: &'static str, }, + /// The decoder needs to know the length of the underlying byte stream + NeedsBytesLen, #[cfg(feature = "symphonia")] /// The symphonia decoder ran into an issue SymphoniaDecoder(crate::decoder::symphonia::SeekError), @@ -640,6 +642,7 @@ impl fmt::Display for SeekError { #[cfg(feature = "wav")] SeekError::HoundDecoder(err) => write!(f, "Error seeking in wav source: {}", err), SeekError::Other(_) => write!(f, "An error occurred"), + SeekError::NeedsBytesLen => write!(f, "The decoder needs to know the length of the file/byte stream to be able to seek. You can set that by using the `DecoderBuilder` or creating a decoder using `Decoder::try_from(some_file)`"), } } } @@ -651,6 +654,7 @@ impl std::error::Error for SeekError { SeekError::SymphoniaDecoder(err) => Some(err), #[cfg(feature = "wav")] SeekError::HoundDecoder(err) => Some(err), + SeekError::NeedsBytesLen => None, SeekError::Other(err) => Some(err.as_ref()), } } @@ -669,6 +673,7 @@ impl SeekError { pub fn source_intact(&self) -> bool { match self { SeekError::NotSupported { .. } => true, + SeekError::NeedsBytesLen => true, #[cfg(feature = "symphonia")] SeekError::SymphoniaDecoder(_) => false, #[cfg(feature = "wav")] diff --git a/tests/seek.rs b/tests/seek.rs index c4a402e0..df430265 100644 --- a/tests/seek.rs +++ b/tests/seek.rs @@ -1,6 +1,8 @@ +use rodio::source::SeekError; use rodio::{ChannelCount, Decoder, Source}; use rstest::rstest; use rstest_reuse::{self, *}; +use std::fs::File; use std::io::{Read, Seek}; use std::path::Path; use std::time::Duration; @@ -255,3 +257,26 @@ fn get_rl(format: &str) -> Decoder { let file = std::fs::File::open(asset).unwrap(); Decoder::try_from(file).unwrap() } + +#[apply(supported_decoders)] +#[trace] +fn decoder_that_need_bytes_len_return_error( + #[case] format: &'static str, + #[case] decoder_name: &'static str, +) { + if decoder_name != "symphonia" { + return; + } + + let asset = Path::new("assets/music").with_extension(format); + let file = File::open(asset).unwrap(); + let mut decoder = Decoder::builder() + .with_data(file) + .build() + .unwrap(); + + let res = decoder.try_seek(Duration::from_secs(2)); + let Err(SeekError::NeedsBytesLen) = res else { + panic!("should not be able to seek without bytes_len.is_some()"); + }; +} diff --git a/tests/total_duration.rs b/tests/total_duration.rs index fecaaa3f..0d41c6d5 100644 --- a/tests/total_duration.rs +++ b/tests/total_duration.rs @@ -55,7 +55,6 @@ fn get_music(format: &str) -> Decoder { rodio::Decoder::builder() .with_data(file) .with_byte_len(len) - .with_seekable(true) .with_gapless(false) .build() .unwrap()