feat(decoders)!: comprehensive seeking and bit depth detection across all decoders #786

roderickvd · 2025-08-24T00:55:28Z

This PR implements a major enhancement to Rodio's decoder capabilities, adding universal seeking support and bit depth detection while fixing some issues, improving performance, and reducing user-facing seek errors.

Key features

Universal seeking support

All decoders implement try_seek() and new_with_settings()
Configurable seeking modes: SeekMode::Fastest (coarse) vs SeekMode::Nearest (accurate)
Automatic fallback: always perform best attempt at performing seek regardless of SeekMode
Enhanced DecoderBuilder with seeking configuration options

Bit depth detection

New Source::bits_per_sample() method returns bit depth for lossless formats

Bug fixes

Fixed zero span length issue in Symphonia Ogg Vorbis decoder that broke buffered sources and effects like reverb
Improved size_hint() accuracy across all decoders

Performance improvements

VorbisDecoder: improved packet reading
FlacDecoder: better block loading with FlacReaderOptions
Major Symphonia refactoring: enhanced performance and multi-track/chained stream support

API enhancements

New `DecoderBuilder` methods

  DecoderBuilder::new()
      .with_seek_mode(SeekMode::Fastest)     // Configure seeking precision
      .with_scan_duration(true)              // Enable duration scanning  
      .with_total_duration(known_duration)   // Pre-computed duration

Test coverage

Seeking tests across all decoder formats
Duration accuracy validation across all decoder formats
Bit depth detection tests for lossless formats
Hi-res audio support testing (24-bit, 32-bit)

Breaking changes

SeekError::NotSupported renamed to SeekError::SeekingNotSupported
WavDecoder no longer implements ExactSizeIterator (for accuracy)
DecoderBuilder::with_coarse_seek() deprecated (use with_seek_mode())
Source::source_intact() removed (too many unknowns)

Decoder feature overview

Decoder	Seeking	Bit Depth	Duration	Size hint	Span length
claxon	✅	✅	✅	✅	✅
hound	✅	✅	✅	✅	✅
minimp3	✅	❌ Lossy	✅	✅	✅
lewton	✅	❌ Lossy	✅	✅	✅
Symphonia	✅	✅ Format-dependent	✅	✅	✅

Closes:

roderickvd · 2025-08-24T01:30:18Z

I feel a bit guilty about this mega-commit. I got carried away while coding on the camping and this is what happened 😊

Copilot

Pull Request Overview

This PR implements a major enhancement to Rodio's decoder capabilities, adding universal seeking support and bit depth detection across all decoders while improving performance and reducing user-facing seek errors.

Universal seeking support with configurable modes (fastest vs nearest) and automatic fallback behavior
Bit depth detection for lossless audio formats through new Source::bits_per_sample() method
Performance improvements and bug fixes across multiple decoders (zero span length fix in Symphonia Ogg Vorbis, improved packet reading)

Reviewed Changes

Copilot reviewed 58 out of 59 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/total_duration.rs	Completely removed old duration testing implementation
tests/source_traits.rs	New comprehensive test suite for decoder functionality including duration, bit depth, and span length tests
tests/seek.rs	Enhanced seeking tests with universal decoder support and improved error handling
src/static_buffer.rs	Added `bits_per_sample()` implementation and updated seek error type
src/source/*.rs	Added `bits_per_sample()` method to all source types with appropriate implementations
src/source/mod.rs	Enhanced `Source` trait with `bits_per_sample()` method and expanded `SeekError` enum
src/queue.rs	Added `bits_per_sample()` support for queue operations
src/mixer.rs	Added `bits_per_sample()` aggregation across mixed sources
src/math.rs	Minor documentation formatting improvements
src/decoder/wav.rs	Complete rewrite with comprehensive documentation, enhanced seeking, and settings support
src/decoder/vorbis.rs	Complete rewrite with granule-based seeking, duration scanning, and robust error handling
src/decoder/utils.rs	New utility module with common decoder helper functions
src/decoder/symphonia.rs	Major refactoring with improved multi-track support, enhanced seeking, and better error recovery

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/source/zero.rs

src/decoder/vorbis.rs

src/decoder/symphonia.rs

dvdsk · 2025-08-24T09:08:19Z

6k lines.... AAAHHHHGHHHHHH

dvdsk · 2025-08-24T09:08:42Z

This might take more then one weekend to review 😅
Hope you dont mind :)

dvdsk

Review still in progress

src/decoder/builder.rs

src/decoder/flac.rs

dvdsk · 2025-08-24T10:13:22Z

src/decoder/flac.rs

-    /// Attempts to decode the data as FLAC.
-    pub fn new(mut data: R) -> Result<FlacDecoder<R>, R> {
-        if !is_flac(data.by_ref()) {
+    /// Attempts to decode the data as FLAC with default settings.


note to self: got till here (dont trust git to keep my review comments unless I send them now)

roderickvd · 2025-08-24T10:29:29Z

This might take more then one weekend to review 😅 Hope you dont mind :)

Sure take the time! I got that coming 😜

Anyway incorporating your review comments iteratively is a fine approach, because one comment may apply to a lot of other decoders as well without you needing to go through all of them.

roderickvd · 2025-08-24T12:15:09Z

I got some smaller updates to this PR committed locally. Would you rather have me wait pushing them, so your review doesn't get mixed up? As you want.

src/decoder/flac.rs

dvdsk · 2025-08-24T14:53:00Z

src/decoder/flac.rs

note to self, reviewed till here

dvdsk · 2025-08-24T15:39:08Z

I got some smaller updates to this PR committed locally. Would you rather have me wait pushing them, so your review doesn't get mixed up? As you want.

Only see these now, lets keep them locally for now. Feel free to resolve the things I brought up once you've addressed them locally (or let me know why I'm wrong and I'll close em).

I fear it might take me quite some time (few weekends) to review this as I don't have that much time throughout the week...

roderickvd · 2025-08-24T15:43:46Z

I got some smaller updates to this PR committed locally. Would you rather have me wait pushing them, so your review doesn't get mixed up? As you want.

Only see these now, lets keep them locally for now. Feel free to resolve the things I brought up once you've addressed them locally (or let me know why I'm wrong and I'll close em).

Will do.

I fear it might take me quite some time (few weekends) to review this as I don't have that much time throughout the week...

No haste. I'm back to work starting tomorrow, too.

dvdsk · 2025-08-30T17:05:43Z

sorry I spend all my time on #791 in response to #782. I wont be able to look at this again for a while :(

roderickvd · 2025-08-30T17:12:02Z

I'm in no haste. Though as it touches a lot of sources and I want to prevent rebasing as much as possible, I'll wait to have this reviewed before I start other Rodio development.

PetrGlad · 2025-09-02T17:40:27Z

I wanted to play with it, are there any uses of the Builder uses in examples/* ?

src/decoder/builder.rs

roderickvd · 2025-09-02T19:18:08Z

I wanted to play with it, are there any uses of the Builder uses in examples/* ?

No, though the Rustdoc contains a lot of examples.

"Behind the scenes" the examples do use it, because Decoder::TryFrom<File> uses the builder to construct a decoder.

Fixes #141

- Improve clarity and conciseness of doc comments for all decoders - Remove redundant and overly verbose explanations - Add missing details to trait and method docs - Update examples to use consistent file paths and types - Move Settings struct visibility to crate-internal - Refactor Path/PathBuf decoder constructors for optimal hinting - Standardize iterator and trait method documentation across formats - Remove misleading or outdated implementation notes - Fix Zero source bits_per_sample to return None - Update seek test attributes for feature consistency

- Introduce BitDepth as a newtype for non-zero u32 - Update Source trait and all implementations to use Option<BitDepth> - Update decoders, sources, and tests to use BitDepth instead of u32 for bit depth - Replace NonZero usage for sample rate and channel count with SampleRate and ChannelCount types where appropriate - Update documentation and examples to reflect new types fix: change bits_per_sample to return Option<BitDepth>

…d tests

style: cargo fmt

dvdsk · 2025-09-18T11:28:43Z

I have a few hours each weekend for open source. I now realize I can not review a PR of this size in those hours. I can also not spread it between weekends since I will lose context in between.

To get this merged it needs to be split into multiple PR's, each limited to around 1000 lines so it can be independently reviewed.

I realize this is not easy. My recommendation is to add the new decoders in parallel to the existing. The new decoders would live under a feature flag. We can then merge each smaller PR without breaking the main lib (this is also known as CI). Ideally the small PR's each add a testable modular part of the new decoders that can be run independently. Though I realize that is too much to ask. However lets architect new efforts so that this can be done.

Once the whole new decoder suite is in we can remove all the old code and the flag in one PR.

roderickvd · 2025-09-18T11:55:34Z

I am sorry, I don’t see the point in making that effort. The CI is passing and at least one user has given a favorite review. I am not going to put in many hours of free time in something I don’t see the value in.

I’m still up for a beer sometime in the east, or meeting online, if that expedites things.

will3942 · 2025-09-19T10:54:04Z

This is some really good work and would be a shame to see go to waste, if there's value and you want testing from a more complex user I'd be happy to.

Jaxx497 · 2025-09-19T15:42:54Z

@dvdsk

Is there some kind of recourse here?

I can appreciate the desire to implement this in a way, and I can appreciate life outside of OSS, but the rejection of this PR hurts rodio and its users more than it provides any protection to the codebase.

These are real and tangible changes. The premier audio playback crate of Rust should absolutely promote stable audio seeking. I'm not aware what the exact priorities of this project are, but I can't imagine that said functionality isn't of a considerable standing.

I'm also willing to offer my help in resolving this however I can, although admittedly, I'm not a fraction the developer any of you are. Still, if there is no recourse here, I find it seriously unfair not only to the efforts of Roderick (who is a seriously talented and respectable developer), but to the broader community who will surely benefit from the enhanced functionality.

dvdsk · 2025-09-20T09:44:52Z

Is there some kind of recourse here?

I don't have anything against the core feature being added here, nor against the code. I can not review a PR of this size. So it needs to be split up and ideally also slimmed down.

Some context on the size:

Rodio without this PR: 14349L 23% comments/docs
Rodio with this PR: 16868L 40% comments/docs
This PR adds: 2481L of code and 3486L of comments/docs
71% op the PR is comments/docs

In my opinion that means this PR can be slimmed down to 2481 * 1.23 ~ 3kL in total. Split that up in roughly 3 parts and its quite reviewable.

Almost all our users seem to use the symphonia decoders, this also PR supports all the current decoders. The symphonia decoders all have the same interface. Supporting all decoders must add a lot of extra code. This matches what I have seen while trying (and failing) to review this the past few weekends. So lets just drop support for all the other decoders in Rodio and in this PR. Then adding this PR might even slim Rodio down, increasing its maintainability.

Instead of letting all that decoder code slip into the void we would move it to another crate: rodio-extra-decoders. We then point to that from the Readme.

roderickvd · 2025-09-21T20:25:32Z

I am not sure what you want:

Feature-gate and move the decoders in progressively: comment
Extract the alternative decoders into a separate crate: comment

For item 2 I do not see obvious ways to make that work with the DecoderBuilder and LoopedDecoder.

I have the feeling you detest the code comments. I already said we can cut down if you want. Beyond that, I do not understand your apprehension. You have set up Rodio with an extensive CI suite for decoders. For seeking in particular that's really not so easy to pass, yet this PR does. That gives some pretty strong behavioral guarantees.

Sure, a passing CI suite does not ensure code quality. Beyond pulling the creds card hoping to have established some known quality standard, I could equally argue that CI also is releasing early and iterate often.

I already said sorry for the camping-built mega commit and ask that we move forward pragmatically.

roderickvd · 2025-09-28T07:22:23Z

I already said sorry for the camping-built mega commit and ask that we move forward pragmatically.

@dvdsk to raise it to you again.

LDprg · 2025-09-28T22:08:19Z

@roderickvd I want to note something since I have seen this PR. As a user I might ask wheter it makes sense to keep any other backend than Symphonia?

The point I am trying to make is that all other backends seem rather unmaintaned (they all have no real commits the last 2 or more years). Also I have looked into the code of rodio, the different backends increase the complexity alot. Furthermore Symphonia covers all those codes anyways and is actively maintained and used, by alot of people. There is as far as I know no real situation, where you shouldn't use Symphonia (I assume).

So from a development effort point of view I think it might make sense to strip other Codecs to reduce complexity. Since most users will use Symphonia anyways and other could easily migrate. It would also give more time to improvements and new features. A single backend is also just less error prone, since it's less compile time conditionals to care about and more time to spend to fix it.

However I don't think that should effect this PR in particular. Since this work had already been done and it doesn't make sense to just throw it away. So, if the backends get removed, it should be it's own seperate PR.

Just keep in mind that I have little actual knowledge about how rodio actually works, so I might over simply a bit.

will3942 · 2025-10-04T08:35:06Z

@dvdsk bump on this too.

dvdsk · 2025-10-04T13:43:01Z

However I don't think that should effect this PR in particular. Since this work had already been done and it doesn't make sense to just throw it away. So, if the backends get removed, it should be it's own seperate PR.

I disagree there, this is currently blocked on being too big to review. If we consider removing the other backends then this will get significantly smaller and thus easier to cut into ~1000L pieces.

dvdsk · 2025-10-04T13:49:20Z

I am not sure what you want:

Feature-gate and move the decoders in progressively: #786 (comment)
Extract the alternative decoders into a separate crate: #786 (comment)

I'd like to be able to review this PR, for that it needs to shrink to N independent PR's of about ~1k lines. The smaller the total size the faster we will have this merged. I've given some suggestions on how I think that can be achieved. It is up to whomever takes up the work how it is achieved. That does not have to be Roderick who is busy working on Cpal and many other open source projects. As there is clear intrest from the community I'd suggest one of you picks this up.

roderickvd requested review from dvdsk and Copilot August 24, 2025 00:55

roderickvd self-assigned this Aug 24, 2025

roderickvd added bug enhancement breaking Proposed change that would break the public API labels Aug 24, 2025

This was referenced Aug 24, 2025

How to get length of audio file before reading each sample? #294

Closed

MP3 current_ frame_ Len and total_ Duration will never change #458

Open

Multiple tracks within a container #713

Open

This comment was marked as outdated.

Sign in to view

roderickvd requested a review from Copilot August 24, 2025 06:25

Copilot AI reviewed Aug 24, 2025

View reviewed changes

src/source/zero.rs Outdated Show resolved Hide resolved

src/decoder/vorbis.rs Outdated Show resolved Hide resolved

src/decoder/symphonia.rs Show resolved Hide resolved

dvdsk requested changes Aug 24, 2025

View reviewed changes

dvdsk reviewed Aug 24, 2025

View reviewed changes

src/decoder/flac.rs Outdated Show resolved Hide resolved

dvdsk reviewed Aug 24, 2025

View reviewed changes

src/decoder/flac.rs Outdated Show resolved Hide resolved

dvdsk reviewed Aug 24, 2025

View reviewed changes

src/decoder/flac.rs

}

}

}

Copy link

Member

dvdsk Aug 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to self, reviewed till here

roderickvd mentioned this pull request Aug 25, 2025

Playing the same sound several times requires allocating a new buffer each time #141

Open

PetrGlad reviewed Sep 2, 2025

View reviewed changes

src/decoder/builder.rs Outdated Show resolved Hide resolved

PetrGlad reviewed Sep 2, 2025

View reviewed changes

src/decoder/builder.rs Outdated Show resolved Hide resolved

roderickvd added 10 commits September 17, 2025 21:35

feat: add Decoder support for Bytes, Arc, Box, and Vec types

16c3673

Fixes #141

docs: add doc(cfg(...)) attributes for docsrs feature documentation

5e815ad

feat: add bits_per_sample method to Microphone source

81e23f6

refactor: use Decoder::try_from<Path> for file loading in examples an…

4187282

…d tests

refactor: address review comments

4f5dd27

refactor: move LoopedDecoder to its own module and refactor construction

e01f1a2

refactor: move ReadSeekSource into symphonia module

924fa29

fix: import Settings from decoder::builder in decoder modules

712b61a

roderickvd force-pushed the feat/comprehensive-seeking-and-bit-depth branch from 53c2b81 to 65d7a2a Compare September 17, 2025 19:40

roderickvd requested a review from dvdsk September 17, 2025 19:40

style: reorder imports and clean up formatting in decoder modules

46ebd9e

roderickvd force-pushed the feat/comprehensive-seeking-and-bit-depth branch from 65d7a2a to 88ad33b Compare September 17, 2025 20:51

refactor: use SampleRate type throughout

5ecd76b

style: cargo fmt

roderickvd force-pushed the feat/comprehensive-seeking-and-bit-depth branch from 88ad33b to 5ecd76b Compare September 17, 2025 20:53

fix: warnings with no default features

2d60713

dvdsk closed this Sep 18, 2025

roderickvd mentioned this pull request Sep 28, 2025

try_seek just resetting the Source to start #802

Open

feat(decoders)!: comprehensive seeking and bit depth detection across all decoders #786

feat(decoders)!: comprehensive seeking and bit depth detection across all decoders #786

Uh oh!

Conversation

roderickvd commented Aug 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key features

Universal seeking support

Bit depth detection

Bug fixes

Performance improvements

API enhancements

New DecoderBuilder methods

Test coverage

Breaking changes

Decoder feature overview

Uh oh!

roderickvd commented Aug 24, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dvdsk commented Aug 24, 2025

Uh oh!

dvdsk commented Aug 24, 2025

Uh oh!

dvdsk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dvdsk Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

roderickvd commented Aug 24, 2025

Uh oh!

roderickvd commented Aug 24, 2025

Uh oh!

Uh oh!

Uh oh!

dvdsk Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

dvdsk commented Aug 24, 2025

Uh oh!

roderickvd commented Aug 24, 2025

Uh oh!

dvdsk commented Aug 30, 2025

Uh oh!

roderickvd commented Aug 30, 2025

Uh oh!

PetrGlad commented Sep 2, 2025

Uh oh!

Uh oh!

Uh oh!

roderickvd commented Sep 2, 2025

Uh oh!

dvdsk commented Sep 18, 2025

Uh oh!

roderickvd commented Sep 18, 2025

Uh oh!

will3942 commented Sep 19, 2025

Uh oh!

Jaxx497 commented Sep 19, 2025

Uh oh!

dvdsk commented Sep 20, 2025

roderickvd commented Aug 24, 2025 •

edited

Loading

New `DecoderBuilder` methods

dvdsk commented Oct 4, 2025 •

edited

Loading