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

yara-blue · 2026-01-27T20:47:15Z

src/source/take.rs

+
+            // Check if duration has expired.
+            if self.remaining_duration < self.duration_per_sample {
+                self.silence_samples_remaining =


this breaks the promise of take duration, should it not instead pass on a modified last span accounting for the target duration?

a similar thing could work for lineargainramp (there we would cut a span into two)

Yes, good point.

Actually, thinking about it more, is the contract for TakeDuration that get you get:
a. precisely the duration you've asked for (zero-padded if necessary)
b. up to the duration you asked for, or less if the source is exhausted before that?

I think it's b and then this is good to go. This implementation even contains a test that we never end mid-frame / always emit a full frame.

I would say b, since you can compose a by applying TakeDuration on the rodio equivalent of source.chain(silence).

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

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)

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.

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?

yara-blue · 2026-03-09T21:56:43Z

src/source/position.rs

@@ -1,20 +1,25 @@
 use std::time::Duration;


done till here. Might continue tomorrow

status update: since this overhauls almost every source I'm taking this one slow. I'm adding the source to rodio-experiments before I handle it, that way I can get read up on the original impl before I see this (makes an in depth review easier)

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

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.

yara-blue · 2026-03-11T23:39:09Z

src/mixer.rs

    }));

    let output = MixerSource {
-        current_sources: Vec::with_capacity(16),


we ideally never allocate here. The 16 sources give us some headway. Its not the best solution, that would probably be tracking capacity and allocating a new Vec in the MixerHandle and then taking that new allocation together with the new Source.

But that is definitely out of the scope of this PR!!

For now maybe leave the 16 sources pre-allocated.

yara-blue · 2026-03-11T23:40:59Z

src/mixer.rs

-        sample_count: 0,
+        current_channel: 0,
+        still_pending: Vec::new(),
        pending_rx: rx,


this changed on main (the still pending list got removed entirely)

yara-blue · 2026-03-11T23:41:59Z

src/mixer.rs

+            }
        }
    }



take care of on main as well

yara-blue · 2026-03-11T23:46:15Z

src/queue.rs

    }
-
-    #[test]
-    fn span_ending_mid_frame() {


why is this test gone? It seems quiet useful

yara-blue · 2026-03-11T23:47:31Z

tests/total_duration.rs


 #[cfg(any(
    feature = "claxon",
-    feature = "minimp3",


why is minimp3 removed here?

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

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

yara-blue reviewed Mar 11, 2026

View reviewed changes

src/mixer.rs

}

}

}

Copy link

Member

yara-blue Mar 11, 2026

Choose a reason for hiding this comment

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

take care of on main as well

yara-blue reviewed Mar 11, 2026

View reviewed changes

src/queue.rs

}

#[test]

fn span_ending_mid_frame() {

Copy link

Member

yara-blue Mar 11, 2026

Choose a reason for hiding this comment

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

why is this test gone? It seems quiet useful

yara-blue reviewed Mar 11, 2026

View reviewed changes

tests/total_duration.rs

#[cfg(any(

feature = "claxon",

feature = "minimp3",

Copy link

Member

yara-blue Mar 11, 2026

Choose a reason for hiding this comment

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

why is minimp3 removed here?

yara-blue reviewed Mar 11, 2026

View reviewed changes


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


		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.

Conversation

roderickvd commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roderickvd commented Jan 14, 2026

Uh oh!

Uh oh!

Uh oh!

roderickvd commented Jan 24, 2026

Uh oh!

roderickvd commented Jan 24, 2026

Uh oh!

roderickvd commented Jan 25, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roderickvd Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yara-blue left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roderickvd commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yara-blue commented Jan 27, 2026

Uh oh!

roderickvd commented Jan 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yara-blue left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

roderickvd commented Jan 14, 2026 •

edited

Loading

roderickvd Feb 9, 2026 •

edited

Loading

yara-blue left a comment •

edited

Loading

roderickvd commented Jan 27, 2026 •

edited

Loading