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

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

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

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.

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.

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

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

yara-blue · 2026-03-09T19:39:34Z

src/source/linear_ramp.rs

+        samples_counted: 0,
+        cached_span_len: None,
+        last_sample_rate: sample_rate,
+        last_channels: channels,


these params (cached_span_len, last_sample_rate and last_channels) seem to be shared by most sources now. Is this something we should extract to a struct?

like:

struct PrevParams { span_len, sample_rate, channel_count, }

maybe even params, then the detect_span_boundary could maybe take, prev and current both of type Params.

Going even further, a private extension trait implemented for everything Source could provide a .params(&self) -> Params method

Sounds all good. I don’t know precisely where you are with per-source merging so again let me know if you’d like me to take something on.

My plan was to get this and the re-sampler merged first and then slowly bring that in. Don't worry there will be a ton to review 😭.

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.

yara-blue · 2026-03-09T19:47:33Z

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.
+/// Returns a tuple indicating whether we're at a span boundary and if parameters changed.


how about we return a struct with fields named: at_span_boundary and parameters_changed or something along does lines.

Yeah that’s be richer info. Hypothetically, sources may need to know one but not the other.

Hypothetically, sources may need to know one but not the other. We trust the compiler to rip the unused part out :)

yara-blue · 2026-03-09T19:50:31Z

src/source/mod.rs

+pub(crate) mod test_utils {
+    use super::*;
+
+    /// Test helper source that can end mid-frame for testing incomplete frame handling.


but we do not support ending mid-frame?
how this is different from SamplesBuffer?

On mobile, but it: does? SamplesBuffer should now enforce it is frame aligned. This source doesn’t.

Okay, but the incomplete frame alignment behavior is not used anywhere with this PR applied right?

src/source/position.rs

yara-blue · 2026-03-11T20:30:50Z

src/source/zero.rs

mhm this behavior was always redundant (inf zero + take dur). Especially now we are size hinting all the other generators as infinite we should consider bringing this in line.

out of scope for this PR though

Also we shouldn’t call it zero. That’s not equilibrium for unsigned integers. Yes it’ll produce silence but at full DC, burning your drivers.

I've called it Silence for the new FixedSource and ConstSource. As part of that overhaul this will be removed.

Since generators never change their parameters they have no span so it does not make senses to have a generator implement Source if it can instead implement the more strict FixedSource and ConstSource.

ConstSource can be turned into a FixedSource easily and for free the same is true for turning a FixedSource into a Source.

yara-blue · 2026-03-11T20:31:50Z

src/source/zero.rs

        sample_rate: SampleRate,
        num_samples: usize,
    ) -> Self {
+        assert_eq!(


Rather have this return a Result instead, it should then just have a single struct as error type.

Agreed. Did this to keep the API stable. But we don’t need to.

Actually this will be removed soon so I don't think it matters, that is if you agree that having generators implement Source is redundant if they already implement FixedSource and ConstSource.

src/mixer.rs

src/queue.rs

tests/total_duration.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!

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

refactor: SourcesQueueOutput silence and span logic

7538971

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

roderickvd added 4 commits January 24, 2026 17:20

fix: detect stream parameter changes at span boundaries

9088afe

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).

refactor: extract span tracking helpers

b16d164

fix: BLT filter stereo and multi-channel audio

354b016

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

fix: handle incomplete frames in ChannelVolume

b5490ac

roderickvd added 4 commits January 25, 2026 15:47

refactor: use shared padding_samples_needed and TestSource helpers

5c667c2

fix: pad partial frames and track span parameters

9565d94

refactor: enforce frame-aligned Source contract

7f9f2bc

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

docs: update changelog

0fbddd8

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 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

yara-blue reviewed Mar 9, 2026

View reviewed changes

src/source/channel_volume.rs Show resolved Hide resolved

yara-blue reviewed Mar 9, 2026

View reviewed changes

src/source/position.rs Show resolved Hide resolved