Skip to content

Commit 1514256

Browse files
committed
fix: ensure frame-aligned spans in queue to prevent channel misalignment
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
1 parent b65e416 commit 1514256

File tree

4 files changed

+225
-9
lines changed

4 files changed

+225
-9
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3636
- Fixed audio distortion when queueing sources with different sample rates/channel counts or transitioning from empty queue.
3737
- Fixed `SamplesBuffer` to correctly report exhaustion and remaining samples.
3838
- Improved precision in `SkipDuration` to avoid off-by-a-few-samples errors.
39+
- Fixed channel misalignment in queue with non-power-of-2 channel counts (e.g., 6 channels) by ensuring frame-aligned span lengths.
40+
- Fixed channel misalignment when sources end before their promised span length by padding with silence to complete frames.
3941

4042
### Changed
4143
- `output_to_wav` renamed to `wav_to_file` and now takes ownership of the `Source`.

src/queue.rs

Lines changed: 154 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ pub fn queue(keep_alive_if_empty: bool) -> (Arc<SourcesQueueInput>, SourcesQueue
3535
current: Box::new(Empty::new()) as Box<_>,
3636
signal_after_end: None,
3737
input: input.clone(),
38+
samples_consumed_in_span: 0,
39+
padding_samples_remaining: 0,
3840
};
3941

4042
(input, output)
@@ -110,12 +112,29 @@ pub struct SourcesQueueOutput {
110112

111113
// The next sounds.
112114
input: Arc<SourcesQueueInput>,
115+
116+
// Track samples consumed in the current span to detect mid-span endings.
117+
samples_consumed_in_span: usize,
118+
119+
// When a source ends mid-frame, this counts how many silence samples to inject
120+
// to complete the frame before transitioning to the next source.
121+
padding_samples_remaining: usize,
113122
}
114123

115-
const THRESHOLD: usize = 512;
116124
const SILENCE_SAMPLE_RATE: SampleRate = nz!(44100);
117125
const SILENCE_CHANNELS: ChannelCount = nz!(1);
118126

127+
/// Returns a threshold span length that ensures frame alignment.
128+
///
129+
/// Spans must end on frame boundaries (multiples of channel count) to prevent
130+
/// channel misalignment. Returns ~512 samples rounded to the nearest frame.
131+
#[inline]
132+
fn threshold(channels: ChannelCount) -> usize {
133+
const BASE_SAMPLES: usize = 512;
134+
let ch = channels.get() as usize;
135+
BASE_SAMPLES.div_ceil(ch) * ch
136+
}
137+
119138
impl Source for SourcesQueueOutput {
120139
#[inline]
121140
fn current_span_len(&self) -> Option<usize> {
@@ -136,8 +155,8 @@ impl Source for SourcesQueueOutput {
136155
} else if self.input.keep_alive_if_empty.load(Ordering::Acquire)
137156
&& self.input.next_sounds.lock().unwrap().is_empty()
138157
{
139-
// The next source will be a filler silence which will have the length of `THRESHOLD`
140-
return Some(THRESHOLD);
158+
// The next source will be a filler silence which will have a frame-aligned length
159+
return Some(threshold(self.current.channels()));
141160
}
142161

143162
// Try the size hint.
@@ -148,8 +167,8 @@ impl Source for SourcesQueueOutput {
148167
return Some(lower_bound);
149168
}
150169

151-
// Otherwise we use the constant value.
152-
Some(THRESHOLD)
170+
// Otherwise we use a frame-aligned threshold value.
171+
Some(threshold(self.current.channels()))
153172
}
154173

155174
#[inline]
@@ -204,13 +223,33 @@ impl Iterator for SourcesQueueOutput {
204223
#[inline]
205224
fn next(&mut self) -> Option<Self::Item> {
206225
loop {
226+
// If we're padding to complete a frame, return silence.
227+
if self.padding_samples_remaining > 0 {
228+
self.padding_samples_remaining -= 1;
229+
return Some(0.0);
230+
}
231+
207232
// Basic situation that will happen most of the time.
208233
if let Some(sample) = self.current.next() {
234+
self.samples_consumed_in_span += 1;
209235
return Some(sample);
210236
}
211237

212-
// Since `self.current` has finished, we need to pick the next sound.
238+
// Source ended - check if we ended mid-frame and need padding.
239+
let channels = self.current.channels().get() as usize;
240+
let incomplete_frame_samples = self.samples_consumed_in_span % channels;
241+
if incomplete_frame_samples > 0 {
242+
// We're mid-frame - need to pad with silence to complete it.
243+
self.padding_samples_remaining = channels - incomplete_frame_samples;
244+
// Reset counter now since we're transitioning to a new span.
245+
self.samples_consumed_in_span = 0;
246+
// Continue loop - next iteration will inject silence.
247+
continue;
248+
}
249+
250+
// Reset counter and move to next sound.
213251
// In order to avoid inlining this expensive operation, the code is in another function.
252+
self.samples_consumed_in_span = 0;
214253
if self.go_next().is_err() {
215254
return None;
216255
}
@@ -240,7 +279,7 @@ impl SourcesQueueOutput {
240279
let silence = Box::new(Zero::new_samples(
241280
SILENCE_CHANNELS,
242281
SILENCE_SAMPLE_RATE,
243-
THRESHOLD,
282+
threshold(SILENCE_CHANNELS),
244283
)) as Box<_>;
245284
if self.input.keep_alive_if_empty.load(Ordering::Acquire) {
246285
// Play a short silence in order to avoid spinlocking.
@@ -263,8 +302,9 @@ impl SourcesQueueOutput {
263302
mod tests {
264303
use crate::buffer::SamplesBuffer;
265304
use crate::math::nz;
266-
use crate::queue;
267-
use crate::source::Source;
305+
use crate::source::{SeekError, Source};
306+
use crate::{queue, ChannelCount, Sample, SampleRate};
307+
use std::time::Duration;
268308

269309
#[test]
270310
fn basic() {
@@ -340,4 +380,109 @@ mod tests {
340380
assert_eq!(rx.next(), Some(10.0));
341381
assert_eq!(rx.next(), Some(-10.0));
342382
}
383+
384+
#[test]
385+
fn span_ending_mid_frame() {
386+
let mut test_source1 = TestSource::new(&[0.1, 0.2, 0.1, 0.2, 0.1])
387+
.with_channels(nz!(2))
388+
.with_false_span_len(Some(6));
389+
let mut test_source2 = TestSource::new(&[0.3, 0.4, 0.3, 0.4]).with_channels(nz!(2));
390+
391+
let (controls, mut source) = queue::queue(true);
392+
controls.append(test_source1.clone());
393+
controls.append(test_source2.clone());
394+
395+
assert_eq!(source.next(), test_source1.next());
396+
assert_eq!(source.next(), test_source1.next());
397+
assert_eq!(source.next(), test_source1.next());
398+
assert_eq!(source.next(), test_source1.next());
399+
assert_eq!(source.next(), test_source1.next());
400+
assert_eq!(None, test_source1.next());
401+
402+
// Source promised span of 6 but only delivered 5 samples.
403+
// With 2 channels, that's 2.5 frames. Queue should pad with silence.
404+
assert_eq!(
405+
source.next(),
406+
Some(0.0),
407+
"Expected silence to complete frame"
408+
);
409+
410+
assert_eq!(source.next(), test_source2.next());
411+
assert_eq!(source.next(), test_source2.next());
412+
assert_eq!(source.next(), test_source2.next());
413+
assert_eq!(source.next(), test_source2.next());
414+
}
415+
416+
/// Test helper source that allows setting false span length to simulate
417+
/// sources that end before their promised span length.
418+
#[derive(Debug, Clone)]
419+
struct TestSource {
420+
samples: Vec<Sample>,
421+
pos: usize,
422+
channels: ChannelCount,
423+
sample_rate: SampleRate,
424+
total_span_len: Option<usize>,
425+
}
426+
427+
impl TestSource {
428+
fn new(samples: &[Sample]) -> Self {
429+
let samples = samples.to_vec();
430+
Self {
431+
total_span_len: Some(samples.len()),
432+
pos: 0,
433+
channels: nz!(1),
434+
sample_rate: nz!(44100),
435+
samples,
436+
}
437+
}
438+
439+
fn with_channels(mut self, count: ChannelCount) -> Self {
440+
self.channels = count;
441+
self
442+
}
443+
444+
fn with_false_span_len(mut self, total_len: Option<usize>) -> Self {
445+
self.total_span_len = total_len;
446+
self
447+
}
448+
}
449+
450+
impl Iterator for TestSource {
451+
type Item = Sample;
452+
453+
fn next(&mut self) -> Option<Self::Item> {
454+
let res = self.samples.get(self.pos).copied();
455+
self.pos += 1;
456+
res
457+
}
458+
459+
fn size_hint(&self) -> (usize, Option<usize>) {
460+
let remaining = self.samples.len().saturating_sub(self.pos);
461+
(remaining, Some(remaining))
462+
}
463+
}
464+
465+
impl Source for TestSource {
466+
fn current_span_len(&self) -> Option<usize> {
467+
self.total_span_len
468+
}
469+
470+
fn channels(&self) -> ChannelCount {
471+
self.channels
472+
}
473+
474+
fn sample_rate(&self) -> SampleRate {
475+
self.sample_rate
476+
}
477+
478+
fn total_duration(&self) -> Option<Duration> {
479+
None
480+
}
481+
482+
fn try_seek(&mut self, _: Duration) -> Result<(), SeekError> {
483+
Err(SeekError::NotSupported {
484+
underlying_source: std::any::type_name::<Self>(),
485+
})
486+
}
487+
}
343488
}

src/source/mod.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,12 @@ pub trait Source: Iterator<Item = Sample> {
176176
/// After the engine has finished reading the specified number of samples, it will check
177177
/// whether the value of `channels()` and/or `sample_rate()` have changed.
178178
///
179+
/// # Frame Alignment
180+
///
181+
/// Span lengths must be multiples of the channel count to ensure spans end on frame
182+
/// boundaries. A "frame" is one sample for each channel. Returning a span length
183+
/// that is not a multiple of `channels()` will cause channel misalignment issues.
184+
///
179185
/// Note: This returns the total span size, not the remaining samples. Use `Iterator::size_hint`
180186
/// to determine how many samples remain in the iterator.
181187
fn current_span_len(&self) -> Option<usize>;

tests/channel_volume.rs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
use std::fs;
2+
use std::io::BufReader;
3+
4+
use rodio::source::ChannelVolume;
5+
use rodio::{queue, Decoder, Sample, Source};
6+
7+
fn create_6_channel_source() -> ChannelVolume<Decoder<BufReader<fs::File>>> {
8+
let file = fs::File::open("assets/music.mp3").unwrap();
9+
let decoder = Decoder::try_from(file).unwrap();
10+
assert_eq!(decoder.channels().get(), 2);
11+
let channel_volume = ChannelVolume::new(decoder, vec![1.0, 1.0, 0.0, 0.0, 0.0, 0.0]);
12+
assert_eq!(channel_volume.channels().get(), 6);
13+
channel_volume
14+
}
15+
16+
#[test]
17+
fn channel_volume_without_queue() {
18+
let channel_volume = create_6_channel_source();
19+
assert_output_only_on_first_two_channels(channel_volume, 6);
20+
}
21+
22+
#[test]
23+
fn channel_volume_with_queue() {
24+
let channel_volume = create_6_channel_source();
25+
let (controls, queue) = queue::queue(false);
26+
controls.append(channel_volume);
27+
assert_output_only_on_first_two_channels(queue, 6);
28+
}
29+
30+
fn assert_output_only_on_first_two_channels(
31+
mut source: impl Source<Item = Sample>,
32+
channels: usize,
33+
) {
34+
let mut frame_number = 0;
35+
let mut samples_in_frame = Vec::new();
36+
37+
while let Some(sample) = source.next() {
38+
samples_in_frame.push(sample);
39+
40+
if samples_in_frame.len() == channels {
41+
// We have a complete frame - verify channels 2+ are zero
42+
for (ch, &sample) in samples_in_frame[2..].iter().enumerate() {
43+
assert_eq!(
44+
sample,
45+
0.0,
46+
"frame {} channel {} had nonzero value (should be zero)",
47+
frame_number,
48+
ch + 2
49+
);
50+
}
51+
52+
samples_in_frame.clear();
53+
frame_number += 1;
54+
}
55+
}
56+
57+
assert_eq!(
58+
samples_in_frame.len(),
59+
0,
60+
"Source ended with partial frame {} (should end on frame boundary)",
61+
samples_in_frame.len()
62+
);
63+
}

0 commit comments

Comments
 (0)