Skip to content

Conversation

@roderickvd
Copy link
Member

Fixes two related issues with frame alignment in the queue:

  1. Frame alignment for non-power-of-2 channel counts: The queue used a fixed threshold of 512 samples which could cause channel misalignment when the channel count doesn't evenly divide 512 (e.g., 6 channels: 512 % 6 = 2 sample offset). Replaced with dynamic threshold function that rounds to nearest frame boundary.

  2. Mid-span ending detection: Sources that end before their promised current_span_len() would cause the queue to immediately transition to the next source, potentially mid-frame. Added sample tracking and silence padding to complete frames before transitioning.

Supersedes #684, carrying over the test suite that was added there.

Branched off of #817 which fixes another span boundary problem.

@yara-blue
Copy link
Member

yara-blue commented Dec 23, 2025

oh this one is fun! Thanks for all the work. I kinda gave up on this :)

@roderickvd
Copy link
Member Author

@yara-blue friendly nudge: do you intend to review this and other PRs, or do you expect me to merge it myself when happy? I understand when you're busy, but would just like to know.

@yara-blue
Copy link
Member

This is super difficult code I tried a while back and gave up on it (could not get it performant and working). I'd love to review it but indeed do not have the time. A quick peek shows nothing that worries me.

If you are happy with it feel free to go ahead and merge!

Little update on what I've been cooking:
I've been working on a hierarchy of Source like traits (from compile time known fixed parameters ConstSource up to the current Source (renamed to DynamicSource). All happily convertible between each-other. Once that's ready ill test drive it a little in my own projects before gradually introducing them into the rodio codebase (under the experimental flag).

Fixes two related issues with frame alignment in the queue:

1. Frame alignment for non-power-of-2 channel counts: The queue used a
   fixed threshold of 512 samples which could cause channel misalignment
   when the channel count doesn't evenly divide 512 (e.g., 6 channels:
   512 % 6 = 2 sample offset). Replaced with dynamic threshold function
   that rounds to nearest frame boundary.

2. Mid-span ending detection: Sources that end before their promised
   current_span_len() would cause the queue to immediately transition
   to the next source, potentially mid-frame. Added sample tracking and
   silence padding to complete frames before transitioning.

Changes:
- Replace THRESHOLD constant with threshold(channels) helper function
  that ensures frame-aligned span lengths
- Add sample tracking to detect mid-span source endings
- Inject silence padding to complete incomplete frames
- Add span_ending_mid_frame test to verify padding behavior

Supersedes #684
@roderickvd roderickvd force-pushed the fix/frame-aligned-spans branch from 1514256 to b11c8b9 Compare December 28, 2025 14:32
@roderickvd
Copy link
Member Author

If you are happy with it feel free to go ahead and merge!

Thanks, will do.

Slightly related question: before I fork have you by any chance changed your stance on #786? Echoing what I said there: not feeling bad about it myself nor want anyone else to feel bad about it, but need that refactoring for my projects so will have to fork it otherwise.

Little update on what I've been cooking: I've been working on a hierarchy of Source like traits (from compile time known fixed parameters ConstSource up to the current Source (renamed to DynamicSource). All happily convertible between each-other. Once that's ready ill test drive it a little in my own projects before gradually introducing them into the rodio codebase (under the experimental flag).

Sounds interesting. Will that also set us up to remove spans entirely?

@roderickvd roderickvd merged commit 639d86f into master Dec 28, 2025
9 checks passed
@roderickvd roderickvd deleted the fix/frame-aligned-spans branch December 28, 2025 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants