fix: correct Source trait semantics and span tracking bugs by roderickvd · Pull Request #831 · RustAudio/rodio

roderickvd · 2026-01-14T23:11:38Z

Clarify Source::current_span_len() returns total span length (not remaining), while size_hint() returns remaining samples of the entire iterator. Fix multiple bugs in span boundary detection, seeking, and iterator implementations.

Opportunistic fixes:

fix division by zero, off-by-one error, and zero case handling
prevent counter overflows
optimize vector allocations

Mid-span seeking: best-effort tracking after try_seek() to detect span boundaries when seeking lands mid-span.

Fixes #691

roderickvd · 2026-01-14T23:15:53Z

I did not fix the decoders because I had already done so with the infamous #786.

src/queue.rs

src/source/position.rs

roderickvd · 2026-01-24T21:29:29Z

Verifying span boundary behavior in the other sources, latest commits I also added:

stereo and multi-channel audio support to BltFilter
mid-frame stream ending in ChannelVolume (akin to what we had fixed in queue)

w.r.t. the first, I wonder how many people use that, given it only ever worked correctly on mono sources.

roderickvd · 2026-01-24T21:53:08Z

I want to take a final look at a couple of other sources, that may need to deal with mid-stream endings as well. In the meantime, feel free to review what we've got already, as I might apply the same patterns that we've already got.

roderickvd · 2026-01-25T22:11:26Z

This thing is getting big but I think I've got it down now. Took me a good week's worth of evenings!

I've left the intermediate commits intact so you can see my journey. I've been going back-and-forth to defend against the fact that when source iterators end mid-frame, multi-channel audio and queuing blows up.

First I added defensive measures to "filter" sources that depend on proper frame alignment. Then, I removed it from the filters in favor of placing it with the "producer" sources: the decoders, mostly, and documenting the frame-alignment requirement.

I think it's better this way.

src/decoder/flac.rs

src/decoder/wav.rs

src/source/agc.rs

src/source/take.rs

yara-blue

Scary how broken this secretly was. Thanks for going through the pain of doing this. It does convince me about some of the ideas in rodio-experiments.

roderickvd · 2026-01-27T21:40:41Z

Indeed! And the sad part is that I expect that 90% of use cases will never use it.

Two things I can think of:

Expedite the introduction of your new source architecture: ensure that sources output to a unified sample rate, always.
Adding a stable_parameters or similar/reciprocal method to the decoder builder, to configure eagerly reporting None as span length even if a decoder could theoretically have its parameters changed.

Point 2 is easily done and I had already put in... well you know.

Thanks for your review.

yara-blue · 2026-01-27T23:05:56Z

Those perf. changes are indeed not insignificant, bummer. I would like to merge this sooner rather then later though (as it's not really isolated). How about we cut a release this week and then merge this after? That gives us until the next release to provide more performance either through 1 or 2.

I feel confident we can land the experimental source architecture within half a year, so before the next release. I'm using it right now in a side project and I'm really happy with how that code looks.

example:
https://github.com/yara-blue/mpdhaj/blob/4f781b71558add4fbe6a503991e58ee86727d475/src/player.rs#L136

roderickvd · 2026-01-29T20:13:40Z

Those perf. changes are indeed not insignificant, bummer. I would like to merge this sooner rather then later though (as it's not really isolated). How about we cut a release this week and then merge this after? That gives us until the next release to provide more performance either through 1 or 2.

OK for me.

yara-blue · 2026-03-09T13:19:40Z

src/source/blt.rs

+
+        let sample_rate = input.sample_rate();
+        let applier = formula.to_applier(sample_rate.get());
+


TODO(yara): Copy the new multichannel logic to the fixed sources.

src/source/channel_volume.rs

src/source/linear_ramp.rs

yara-blue · 2026-03-09T19:46:46Z

src/source/mod.rs


 source_pointer_impl!(<'a, Src> Source for &'a mut Src where Src: Source,);
+
+/// Detects if we're at a span boundary using dual-mode tracking.


this line does not add much for me (I dont know what dual-mode tracking means until I've read the function body)

Yes we should clarify the modes or actually states.

src/source/mod.rs

src/source/position.rs

src/source/zero.rs

src/mixer.rs

yara-blue

yay done reviewing :)

Should be a simple rebase to get main up to date again. I think there are some refactor opportunities and I left some questions here and there.

Let me know when it's ready for a second pass

roderickvd · 2026-03-14T08:47:25Z

Sorry I as processing from top to bottom until I saw your last comment. So you do want me to make the changes here, not as-you-go? Just confirming.

yara-blue · 2026-03-14T12:25:23Z

Sorry I as processing from top to bottom until I saw your last comment. So you do want me to make the changes here, not as-you-go? Just confirming.

Yes confirmed, we will merge this to main after you make the changes here on this PR. Let me know when everything is ready then I'll give it a second pass and merge.

Nothing else will be merged to main until that is done to make sure we get no further conflicts. So take your time do not feel rushed!

Clarify Source::current_span_len() returns total span length (not remaining), while size_hint() returns remaining samples. Fix multiple bugs in span boundary detection, seeking, and iterator implementations. Opportunistic fixes: - fix division by zero, off-by-one error, and zero case handling - prevent counter overflows - optimize vector allocations Mid-span seeking: best-effort tracking after try_seek() to detect span boundaries when seeking lands mid-span. Fixes #691

Parameter updates (channels/sample_rate) occur only at span boundaries or during post-seek detection. Reset counters and enter a detection mode on try_seek (Duration::ZERO is treated as start-of-span).

Use detect_span_boundary and reset_seek_span_tracking to detect span boundaries and parameter changes.

Document that Sources must emit complete frames and pad with silence when ending mid-frame, and ensure that decoders do so.

roderickvd · 2026-03-21T17:55:21Z

@yara-blue rebased and addressed your review comments.

roderickvd · 2026-03-21T18:34:12Z

I wonder if we should replace this ZeroError with something more universally applicable like an AlignmentError or a SourceError::NotAligned.

yara-blue · 2026-03-22T11:17:35Z

I wonder if we should replace this ZeroError with something more universally applicable like an AlignmentError or a SourceError::NotAligned.

We could, though I would propose we resolve by removing it entirely and making the effects strictly mono. Then offer two ways to create multi channel effects:

as mixing multiple mono sources into stereo/multi channel audio
simply repeating the mono sample (channel convertor)

would be a new feature I think? So we can implement that later while 2) will be provided by FixedSource::channel_count_count.

I think this is neat since it makes the overall effects API simpler and moves the channel count decision into the audio pipeline.

yara-blue · 2026-03-22T11:19:39Z

Anyway, this PR looks great. The SpanTracker you introduced nicely abstracts the messy reality.

I know it has taken quiet some effort to get this all sorted. I thought about doing all this myself at some point and gave up. Thank you for persevering through!

roderickvd · 2026-03-22T14:15:29Z

I wonder if we should replace this ZeroError with something more universally applicable like an AlignmentError or a SourceError::NotAligned.

We could, though I would propose we resolve by removing it entirely and making the effects strictly mono.

That’s a good idea. Then we don’t even need Zero::new_samples anymore. Just a combination of Zero (or better: Silence in your experiments) with TakeDuration.

yara-blue · 2026-03-22T23:42:43Z

I wonder if we should replace this ZeroError with something more universally applicable like an AlignmentError or a SourceError::NotAligned.

We could, though I would propose we resolve by removing it entirely and making the effects strictly mono.

That’s a good idea. Then we don’t even need Zero::new_samples anymore. Just a combination of Zero (or better: Silence in your experiments) with TakeDuration.

I could not resist

the macro's that take care of the implementation are straight up evil. The user has to explicitly set the output channel count (the 24 in the example) because we can not yet have const expressions in const argument positions. I did add a nice error as well in case channel counts do not line up:

Note this happens compile time (we panic in a const function). Getting that error formatted was .... a bit of work. Const string formatting isn't in yet so this all happens by copying bytes around in cons functions (and some log10's to get the digits for the channel counts).

roderickvd · 2026-03-23T17:50:17Z

You scare me.

😉

roderickvd requested a review from yara-blue January 14, 2026 23:11

roderickvd commented Jan 21, 2026

View reviewed changes

src/queue.rs Outdated Show resolved Hide resolved

roderickvd force-pushed the fix/span-fixes-clean branch from 16c90ba to 7538971 Compare January 22, 2026 21:33

yara-blue reviewed Jan 23, 2026

View reviewed changes

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

yara-blue reviewed Jan 27, 2026

View reviewed changes

src/decoder/flac.rs Show resolved Hide resolved

yara-blue reviewed Jan 27, 2026

View reviewed changes

src/decoder/wav.rs Show resolved Hide resolved

yara-blue reviewed Jan 27, 2026

View reviewed changes

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

yara-blue reviewed Jan 27, 2026

View reviewed changes

src/source/take.rs Show resolved Hide resolved

yara-blue reviewed Jan 27, 2026

View reviewed changes

yara-blue mentioned this pull request Jan 31, 2026

Release 0.22 #840

Closed

4 tasks

roderickvd mentioned this pull request Feb 9, 2026

feat: high-quality resampler #843

Open

roderickvd mentioned this pull request Feb 26, 2026

Mixer mishandles sources that are added out of phase #854

Closed