Skip to content

Conversation

@yara-blue
Copy link
Member

This introduces a small component from rodio-experiments into rodio under the experimental flag.

For full context see that repo, though in short this enables:

  • consumers like other sources and sink to express they need sources that do not change their parameters
  • us to write more optimized versions of queue, channelcount convertor and resample

@yara-blue yara-blue requested a review from roderickvd January 11, 2026 22:24
@roderickvd
Copy link
Member

Sorry for my buddy Claude posting on my behalf earlier. Looks good to both him and me 😄

How much effort do we want to put into salvaging the current situation with current_span_len (#712)? To recount a few pain points:

  • Currently the contract isn't implemented nor even documented consistently. In my view it should represent the total length of the current span - pretty much what it says on the tin (span_length is the wrong abstraction #694). The remaining length would be for size_hint. What filters actually do is all over the place.

  • size_hints contract is that it's a hint, not a guarantee. That's actually true for decoders, because you cannot ensure that you can successfully decode or even read all bytes before you have. Then for signal generators that don't suffer from that, we should implement ExactSizeIterator. Also not done consistently.

  • Just to spell it out, current_span_len returning remaining length would therefore also be a hint and no guarantee.

  • Because we lack something like a parameters_changed function or signal (replace current_span_length in favor of parameters_changed #706), each filter that needs to deal with that pretty much needs to copy the same code. That's poorly maintainable code duplication as well as an ugly runtime performance hit.

  • try_seek and skip_duration are more interesting cases (skip duration breaks span len #688, Seek breaks span #691), because after seeking typically we will be somewhere mid-span, and we can only estimate from size_hint when we hit the next boundary to check for a parameter update. Depending on the fidelity of that size hint, that may not be accurate and lead to glitches. I think it can be quite OK, we just need to fix size_hint here and there.

I've got a couple things brewing to basically ensure consistent usage of current_span_len, size_hint, and ExactSizeIterator. I don't mind putting some time in it and put a PR in, but want to align with you first. Assuming it'll be some time before your experiments get in as mainstream, it'd be nice to have current_span_len and friends working better than they do today.

@yara-blue
Copy link
Member Author

How much effort do we want to put into salvaging the current situation with current_span_len (#712)?

I personally zero :) I hate spans now 😝. Once the experimental stuff gets stable I would like to recommend to users to convert to FixedSource ASAP.

We should at some point benchmark: DynamicSource (the current one) -> Queue -> Resampler (for output) vs DynamicSource -> Resampler -> FixedSource -> Queue -> Resampler. There are quite some scenarios to try (like: constant input sample rate matching/not matching output & changing input sample rate), however I would not be surprised if double resampling to get to FixedSource ends up cheaper as its Queue implementation can be trivial.

Just to spell it out, current_span_len returning remaining length would therefore also be a hint and no guarantee.

That would break resampling I think? Or do you suggest a signal to watch that changes? The latter would wreck performance (which is why I gave up on it).

In my view it should represent the total length of the current span - pretty much what it says on the tin

Good point the current name & contract contradict each-other. Though we could also rename the method to resolve this. In my perfect world the DynamicSource is just there for backwards compatibility, decoders would get a re-sampler build. With a re-sampler build in they can optimally use all the info they have about parameter stability.

@yara-blue
Copy link
Member Author

Sorry for my buddy Claude posting on my behalf earlier. Looks good to both him and me 😄

We can keep the discussion going here 👍 after I merge this.

@yara-blue yara-blue merged commit 1427642 into master Jan 13, 2026
9 checks passed
@roderickvd
Copy link
Member

How much effort do we want to put into salvaging the current situation with current_span_len (#712)?

I personally zero :) I hate spans now 😝. Once the experimental stuff gets stable I would like to recommend to users to convert to FixedSource ASAP.

If you see no reason against it, then I'll put in some effort to fix current_span_len up for now.

We should at some point benchmark: DynamicSource (the current one) -> Queue -> Resampler (for output) vs DynamicSource -> Resampler -> FixedSource -> Queue -> Resampler. There are quite some scenarios to try (like: constant input sample rate matching/not matching output & changing input sample rate), however I would not be surprised if double resampling to get to FixedSource ends up cheaper as its Queue implementation can be trivial.

The resampler at the end of the pipeline would basically be a pass-through, when DynamicSource has already been resampled appropriately. I imagine it's only when users mix multiple sources of different sampling rates, that there could be double resampling. Say source A in 96 kHz, source B in 44.1 kHz and an output to 48 kHz.

Just to spell it out, current_span_len returning remaining length would therefore also be a hint and no guarantee.

That would break resampling I think?

No, the current resampler just takes frames (not spans) without even calling current_span_len, ever.

For the future resampler with Rubato, I think we should use a fixed-out regime. That means that:

  • anything comes after always receives a fixed number of frames
  • we don't care about anything that comes before, and don't need to zero-pad or anything

I'm also toying around with dasp_interpolate. It's easy in that it also just works with frames, iterator-style, but Rubato's performance is just much better both in terms of CPU usage and anti-aliasing attenuation.

Or do you suggest a signal to watch that changes? The latter would wreck performance (which is why I gave up on it).

I imagine it'd be possible to signal over a broadcast channel. But with your architecture, we don't need to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants