refactor: address review comments

Author: roderickvd

src/decoder/builder.rs

@@ -62,6 +62,7 @@
 //!     let file = File::open("audio.flac")?;
 //!     let len = file.metadata()?.len();
 //!
+//!     // High-quality decoder with precise seeking
 //!     let decoder = Decoder::builder()
 //!         .with_data(file)
 //!         .with_byte_len(len)
@@ -73,8 +74,7 @@
 //!         .with_gapless(false)
 //!         .build()?;
 //!
-//!     // High-quality decoder with precise seeking
-//!     Ok(())
+//!    Ok(())
 //! }
 //! ```
 //!

src/decoder/flac.rs

@@ -107,11 +107,6 @@ const READER_OPTIONS: FlacReaderOptions = FlacReaderOptions {
 /// to minimize allocations during playback. Buffers are reused across blocks for
 /// optimal performance.
 ///
-/// # Thread Safety
-///
-/// This decoder is not thread-safe. Create separate instances for concurrent access
-/// or use appropriate synchronization primitives.
-///
 /// # Generic Parameters
 ///
 /// * `R` - The underlying data source type, must implement `Read + Seek`
@@ -526,20 +521,6 @@ where
     /// buffer of the current FLAC block. It returns samples one at a time while
     /// automatically decoding new blocks as needed.
     ///
-    /// # Sample Format Conversion
-    ///
-    /// Raw FLAC samples are converted to Rodio's sample format based on bit depth:
-    /// - 8-bit: Direct conversion from `i8`
-    /// - 16-bit: Direct conversion from `i16`
-    /// - 24-bit: Conversion using `I24` type
-    /// - 32-bit: Direct conversion from `i32`
-    /// - Other (12, 20-bit): Bit-shifted to 32-bit then converted
-    ///
-    /// # Performance
-    ///
-    /// - **Hot path**: Returning samples from current block (very fast)
-    /// - **Cold path**: Decoding new blocks when buffer is exhausted (slower)
-    ///
     /// # Returns
     ///
     /// - `Some(sample)` - Next audio sample
@@ -610,12 +591,6 @@ where
     /// This information can be used by consumers for buffer pre-allocation
     /// and progress indication.
     ///
-    /// # Returns
-    ///
-    /// A tuple `(lower_bound, upper_bound)` where:
-    /// - `lower_bound`: Minimum number of samples guaranteed to be available
-    /// - `upper_bound`: Maximum number of samples that might be available (None if unknown)
-    ///
     /// # Accuracy
     ///
     /// - **With metadata**: Exact remaining sample count (lower == upper)
@@ -646,54 +621,13 @@ where
 }
 
 /// Probes input data to detect FLAC format.
-///
-/// This function attempts to parse the FLAC magic bytes and stream info header to determine if the
-/// data contains a valid FLAC stream. The stream position is restored regardless of the result.
-///
-/// # Arguments
-///
-/// * `data` - Mutable reference to the input stream to probe
-///
-/// # Returns
-///
-/// - `true` if the data appears to contain a valid FLAC stream
-/// - `false` if the data is not FLAC or is corrupted
-///
-/// # Implementation
-///
-/// Uses the common `utils::probe_format` helper which:
-/// 1. Saves the current stream position
-/// 2. Attempts FLAC detection using `claxon::FlacReader`
-/// 3. Restores the original stream position
-/// 4. Returns the detection result
-///
-/// # Performance
-///
-/// This function only reads the minimum amount of data needed to identify
-/// the FLAC format (magic bytes and basic header), making it efficient for
-/// format detection in multi-format scenarios.
 fn is_flac<R>(data: &mut R) -> bool
 where
     R: Read + Seek,
 {
     utils::probe_format(data, |reader| FlacReader::new(reader).is_ok())
 }
 
-/// Converts claxon decoder errors to rodio seek errors.
-///
-/// This implementation provides error context preservation when FLAC decoding operations fail
-/// during seeking. The original `claxon` error is wrapped in an `Arc` for thread safety and
-/// converted to the appropriate Rodio error type.
-///
-/// # Error Mapping
-///
-/// All `claxon::Error` variants are mapped to `SeekError::ClaxonDecoder` with the
-/// original error preserved for debugging and error analysis.
-///
-/// # Thread Safety
-///
-/// The error is wrapped in `Arc` to allow sharing across thread boundaries if needed,
-/// following Rodio's error handling patterns.
 impl From<claxon::Error> for SeekError {
     /// Converts a claxon error into a Rodio seek error.
     ///

src/decoder/mod.rs

@@ -111,14 +111,10 @@ pub struct LoopedDecoder<R: Read + Seek> {
     inner: Option<DecoderImpl<R>>,
     /// Configuration settings for the decoder.
     settings: Settings,
-    /// Cached metadata from the first successful decoder creation.
     /// Used to avoid expensive file scanning on subsequent loops.
     cached_duration: Option<Duration>,
 }
 
-// Cannot really reduce the size of the VorbisDecoder. There are not any
-/// Internal enum representing different decoder implementations.
-///
 /// This enum dispatches to the appropriate decoder based on detected format
 /// and available features. Large enum variant size is acceptable here since
 /// these are infrequently created and moved.
@@ -145,7 +141,6 @@ enum DecoderImpl<R: Read + Seek> {
     None(Unreachable, PhantomData<R>),
 }
 
-/// Placeholder type for the None variant that can never be instantiated.
 enum Unreachable {}
 
 impl<R: Read + Seek> DecoderImpl<R> {

src/decoder/mp3.rs

@@ -98,22 +98,15 @@ use crate::{
 /// MP3 frames can theoretically change channel configuration, though this is
 /// rare in practice. The decoder handles such changes dynamically.
 ///
-/// # Thread Safety
-///
-/// This decoder is not thread-safe. Create separate instances for concurrent access
-/// or use appropriate synchronization primitives.
-///
 /// # Generic Parameters
 ///
 /// * `R` - The underlying data source type, must implement `Read + Seek`
 pub struct Mp3Decoder<R>
 where
     R: Read + Seek,
 {
-    /// The underlying minimp3 decoder, wrapped in Option for seeking operations.
-    ///
-    /// Temporarily set to `None` during stream reset operations for seeking.
-    /// Always `Some` during normal operation and iteration.
+    /// The underlying minimp3 decoder, wrapped in Option to allow owning the
+    /// Decoder instance during seeking.
     decoder: Option<Decoder<R>>,
 
     /// Byte position where audio data begins (after headers/metadata).
@@ -142,8 +135,7 @@ where
 
     /// Sample rate in Hz.
     ///
-    /// Fixed for the entire MP3 stream. Common rates include 44.1kHz (CD quality),
-    /// 48kHz (professional), and various rates for different MPEG versions.
+    /// Does not change after decoder initialization.
     sample_rate: SampleRate,
 
     /// Number of samples read so far (for seeking calculations).
@@ -740,12 +732,6 @@ where
     /// Provides size estimates based on MP3 metadata and current playback position.
     /// The accuracy depends on the availability and reliability of duration information.
     ///
-    /// # Returns
-    ///
-    /// A tuple `(lower_bound, upper_bound)` where:
-    /// - `lower_bound`: Minimum number of samples guaranteed to be available
-    /// - `upper_bound`: Maximum number of samples that might be available (None if unknown)
-    ///
     /// # Accuracy Levels
     ///
     /// - **High accuracy**: When total samples calculated from scanned duration
@@ -841,39 +827,6 @@ fn get_mp3_duration<R: Read + Seek>(data: &mut R) -> Option<Duration> {
 }
 
 /// Probes input data to detect MP3 format.
-///
-/// This function attempts to decode the first MP3 frame to determine if the
-/// data contains a valid MP3 stream. The stream position is restored regardless
-/// of the result, making it safe to use for format detection.
-///
-/// # Arguments
-///
-/// * `data` - Mutable reference to the input stream to probe
-///
-/// # Returns
-///
-/// - `true` if the data appears to contain a valid MP3 stream
-/// - `false` if the data is not MP3 or is corrupted
-///
-/// # Implementation
-///
-/// Uses the common `utils::probe_format` helper which:
-/// 1. Saves the current stream position
-/// 2. Attempts MP3 detection using `minimp3::Decoder`
-/// 3. Restores the original stream position
-/// 4. Returns the detection result
-///
-/// # Performance
-///
-/// This function only reads the minimum amount of data needed to identify
-/// and decode the first MP3 frame, making it efficient for format detection
-/// in multi-format scenarios.
-///
-/// # Robustness
-///
-/// The detection uses actual frame decoding rather than just header checking,
-/// providing more reliable format identification at the cost of slightly
-/// higher computational overhead.
 fn is_mp3<R>(data: &mut R) -> bool
 where
     R: Read + Seek,

src/decoder/read_seek_source.rs

@@ -52,12 +52,6 @@ use crate::decoder::builder::Settings;
 /// - **Byte length**: Total stream size for seeking and progress calculations
 /// - **Configuration**: Stream properties from decoder builder settings
 ///
-/// # Thread Safety
-///
-/// This wrapper requires `Send + Sync` bounds on the wrapped type to ensure
-/// thread safety for Symphonia's internal operations. Most standard I/O types
-/// satisfy these requirements.
-///
 /// # Generic Parameters
 ///
 /// * `T` - The wrapped I/O type, must implement `Read + Seek + Send + Sync`

src/decoder/symphonia.rs

@@ -135,11 +135,6 @@ use crate::{decoder::builder::Settings, BitDepth};
 /// - **Reset required**: Recreate decoder and continue with next track
 /// - **I/O errors**: Attempt to continue when possible
 /// - **Terminal errors**: Clean shutdown when recovery is impossible
-///
-/// # Thread Safety
-///
-/// This decoder is not thread-safe. Create separate instances for concurrent access
-/// or use appropriate synchronization primitives.
 pub struct SymphoniaDecoder {
     /// The underlying Symphonia audio decoder.
     ///
@@ -1054,12 +1049,6 @@ impl Iterator for SymphoniaDecoder {
     ///
     /// For multi-track files, estimates represent the currently selected audio
     /// track, not the entire file duration or all tracks combined.
-    ///
-    /// # Returns
-    ///
-    /// A tuple `(lower_bound, upper_bound)` where:
-    /// - `lower_bound`: Minimum number of samples guaranteed to be available
-    /// - `upper_bound`: Maximum number of samples that might be available (None if unknown)
     fn size_hint(&self) -> (usize, Option<usize>) {
         // Samples already decoded and buffered (guaranteed available)
         let buffered_samples = self
@@ -1189,36 +1178,34 @@ fn should_continue_on_decode_error(
     }
 }
 
+/// Converts Rodio's SeekMode to Symphonia's SeekMode.
+///
+/// This conversion maps Rodio's seeking preferences to Symphonia's
+/// internal seeking modes, enabling consistent seeking behavior
+/// across different audio processing layers.
+///
+/// # Mapping
+///
+/// - `SeekMode::Fastest` → `SymphoniaSeekMode::Coarse`
+///   - Prioritizes speed over precision
+///   - Uses keyframe-based seeking when available
+///   - Suitable for user scrubbing and fast navigation
+/// - `SeekMode::Nearest` → `SymphoniaSeekMode::Accurate`
+///   - Prioritizes precision over speed
+///   - Attempts sample-accurate positioning
+///   - Suitable for gapless playback and precise positioning
+///
+/// # Performance Implications
+///
+/// The choice between modes affects performance significantly:
+/// - **Coarse**: Fast seeks but may require fine-tuning
+/// - **Accurate**: Slower seeks but precise positioning
+///
+/// # Format Compatibility
+///
+/// Not all formats support both modes equally. Automatic
+/// fallbacks may occur when preferred mode unavailable.
 impl From<SeekMode> for SymphoniaSeekMode {
-    /// Converts Rodio's SeekMode to Symphonia's SeekMode.
-    ///
-    /// This conversion maps Rodio's seeking preferences to Symphonia's
-    /// internal seeking modes, enabling consistent seeking behavior
-    /// across different audio processing layers.
-    ///
-    /// # Mapping
-    ///
-    /// - `SeekMode::Fastest` → `SymphoniaSeekMode::Coarse`
-    ///   - Prioritizes speed over precision
-    ///   - Uses keyframe-based seeking when available
-    ///   - Suitable for user scrubbing and fast navigation
-    /// - `SeekMode::Nearest` → `SymphoniaSeekMode::Accurate`
-    ///   - Prioritizes precision over speed
-    ///   - Attempts sample-accurate positioning
-    ///   - Suitable for gapless playback and precise positioning
-    ///
-    /// # Performance Implications
-    ///
-    /// The choice between modes affects performance significantly:
-    /// - **Coarse**: Fast seeks but may require fine-tuning
-    /// - **Accurate**: Slower seeks but precise positioning
-    ///
-    /// # Format Compatibility
-    ///
-    /// Not all formats support both modes equally:
-    /// - Some formats only implement one mode effectively
-    /// - Automatic fallbacks may occur when preferred mode unavailable
-    /// - Mode availability may depend on stream characteristics
     fn from(mode: SeekMode) -> Self {
         match mode {
             SeekMode::Fastest => SymphoniaSeekMode::Coarse,

src/decoder/vorbis.rs

@@ -105,11 +105,6 @@ use crate::{
 /// Ogg supports chained streams where multiple Vorbis streams are concatenated.
 /// The decoder adapts to parameter changes (sample rate, channels) between streams.
 ///
-/// # Thread Safety
-///
-/// This decoder is not thread-safe. Create separate instances for concurrent access
-/// or use appropriate synchronization primitives.
-///
 /// # Generic Parameters
 ///
 /// * `R` - The underlying data source type, must implement `Read + Seek`
@@ -725,12 +720,6 @@ where
     /// playback position. The accuracy depends on the availability of duration information
     /// from granule position scanning or explicit duration settings.
     ///
-    /// # Returns
-    ///
-    /// A tuple `(lower_bound, upper_bound)` where:
-    /// - `lower_bound`: Minimum number of samples guaranteed to be available
-    /// - `upper_bound`: Maximum number of samples that might be available (None if unknown)
-    ///
     /// # Accuracy Levels
     ///
     /// - **High accuracy**: When total samples calculated from granule scanning
@@ -995,39 +984,6 @@ fn granules_to_duration(granules: u64, sample_rate: SampleRate) -> Duration {
 }
 
 /// Probes input data to detect Ogg Vorbis format.
-///
-/// This function attempts to initialize a lewton OggStreamReader to determine if the
-/// data contains a valid Ogg Vorbis stream. The stream position is restored regardless
-/// of the result, making it safe to use for format detection.
-///
-/// # Arguments
-///
-/// * `data` - Mutable reference to the input stream to probe
-///
-/// # Returns
-///
-/// - `true` if the data appears to contain a valid Ogg Vorbis stream
-/// - `false` if the data is not Ogg Vorbis or is corrupted
-///
-/// # Implementation
-///
-/// Uses the common `utils::probe_format` helper which:
-/// 1. Saves the current stream position
-/// 2. Attempts Ogg Vorbis detection using `lewton::OggStreamReader`
-/// 3. Restores the original stream position
-/// 4. Returns the detection result
-///
-/// # Performance
-///
-/// This function reads the minimum amount of data needed to identify the Ogg
-/// container format and Vorbis codec headers, making it efficient for format
-/// detection in multi-format scenarios.
-///
-/// # Robustness
-///
-/// The detection uses actual stream reader initialization rather than just header
-/// checking, providing reliable format identification with proper Ogg/Vorbis
-/// validation at the cost of slightly higher computational overhead.
 fn is_vorbis<R>(data: &mut R) -> bool
 where
     R: Read + Seek,