From 91063d6c323e130d162047554726788805cd7208 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Rouleau?= Date: Sat, 25 Jul 2026 14:00:14 -0400 Subject: [PATCH 1/3] Don't end a resampler chunk at a same-format span boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fill_input_buffer` clamped each read to `current_span_len()`, and `resample_chunk` then flagged any short read as `partial_len` — which rubato treats as a short/final chunk and zero-pads. Decoders report one span per decoded packet, so a single continuous stream arrives as many small spans and a discontinuity was injected at every one of them. Vorbis is the clearest case: it switches to short blocks (commonly 128 or 256 samples) around transients, so real files move in and out of the degraded regime depending on programme material. Measured on a 440 Hz stereo sine resampled 48 kHz -> 44.1 kHz, as the fraction of output energy away from the fundamental: span size before after single 0.054 % 0.054 % 4096 frames 0.054 % 0.054 % 2048 frames 0.054 % 0.054 % 1024 frames 1.02 % 0.054 % 512 frames 23.79 % 0.054 % 256 frames 90.96 % 0.054 % 128 frames 91.00 % 0.054 % Output is bit-identical to the single-span case at every span size once the chunk is no longer cut at same-format boundaries. Output length also stops drifting (44110/44156/44250 -> a consistent 44124). A span boundary only matters to the resampler when the sample rate or channel count actually changes, so keep pulling input across boundaries and stop only on a genuine format change or end of stream. `partial_len` now fires only at real end-of-stream. Adds a regression test asserting the invariant directly: output must stay spectrally pure regardless of span size. It fails on the parent commit (1024-frame spans already breach the threshold) and passes here. Bisected to ca72dac ("add dedicated input datastructure"); the released v0.22.2 predates the rubato resampler and is unaffected. Fixes #907. --- src/conversions/sample_rate/rubato.rs | 40 ++++++++++++++---- src/conversions/sample_rate/tests.rs | 61 +++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 9 deletions(-) diff --git a/src/conversions/sample_rate/rubato.rs b/src/conversions/sample_rate/rubato.rs index df1ee016..b321b640 100644 --- a/src/conversions/sample_rate/rubato.rs +++ b/src/conversions/sample_rate/rubato.rs @@ -193,19 +193,41 @@ impl> RubatoResample { } fn fill_input_buffer(&mut self, needed_by_resampler: InFrameCount) -> InFrameCount { - let current_span_length = self.input.current_span_len().map(InSamples); - let frames_to_take = needed_by_resampler - .samples(self.output.channels) - .min(current_span_length.unwrap_or(InSamples::MAX)); - + let wanted = needed_by_resampler.samples(self.output.channels); self.input_buffer.clear(); - for _ in 0..frames_to_take.raw() { - if let Some(sample) = self.input.next() { - self.input_buffer.push(sample); - } else { + + // Keep pulling across span boundaries while the format is unchanged. + // A span boundary only matters to the resampler when the sample rate + // or channel count actually changes; ending a chunk at every boundary + // would flag it `partial_len`, which rubato zero-pads. + while self.input_buffer.len() < wanted { + let span = self.input.current_span_len().map(InSamples); + let take = (wanted - self.input_buffer.len()).min(span.unwrap_or(InSamples::MAX)); + if take == InSamples::ZERO { + break; + } + + let rate_before = self.input.sample_rate(); + let channels_before = self.input.channels(); + + let mut taken = 0usize; + for _ in 0..take.raw() { + if let Some(sample) = self.input.next() { + self.input_buffer.push(sample); + taken += 1; + } else { + break; + } + } + + if taken == 0 + || self.input.sample_rate() != rate_before + || self.input.channels() != channels_before + { break; } } + self.input_buffer.len().frames(self.output.channels) } diff --git a/src/conversions/sample_rate/tests.rs b/src/conversions/sample_rate/tests.rs index c2ea52cc..af098310 100644 --- a/src/conversions/sample_rate/tests.rs +++ b/src/conversions/sample_rate/tests.rs @@ -338,3 +338,64 @@ fn test_span_boundary_same_format() { output.len() ); } + +/// Resampling must not depend on how the input is chunked into spans. +/// +/// Decoders report one span per decoded packet (Vorbis packets are commonly +/// 128–2048 frames, switching to short blocks around transients), so a single +/// continuous stream arrives as many small spans. If a span boundary ends a +/// resampler chunk, the chunk is flagged `partial_len` and zero-padded, +/// injecting a discontinuity at every boundary. +#[test] +fn small_spans_do_not_degrade_resampled_signal() { + let source_rate = SampleRate::new(48000).unwrap(); + let target_rate = SampleRate::new(44100).unwrap(); + let channels = ChannelCount::new(2).unwrap(); + + // 0.5 s of a 440 Hz stereo sine at 48 kHz. + let signal: Vec = (0..24_000) + .flat_map(|i| { + let t = i as f32 / source_rate.get() as f32; + let s = (std::f32::consts::TAU * 440.0 * t).sin() * 0.5; + [s, s] + }) + .collect(); + + // Fraction of output energy sitting at the 440 Hz fundamental, via + // Goertzel on the left channel. A clean resample keeps this ~1.0. + let tone_energy_fraction = |samples: &[Sample]| -> f64 { + let left: Vec = samples.iter().step_by(2).map(|&s| s as f64).collect(); + let n = left.len(); + let bin = 440.0 * n as f64 / target_rate.get() as f64; + let w = std::f64::consts::TAU * bin / n as f64; + let (mut prev, mut prev2) = (0.0, 0.0); + for &s in &left { + let current = s + 2.0 * w.cos() * prev - prev2; + prev2 = prev; + prev = current; + } + let tone = prev * prev + prev2 * prev2 - 2.0 * w.cos() * prev * prev2; + let total: f64 = left.iter().map(|s| s * s).sum(); + (tone / (n as f64 / 2.0)) / total.max(f64::EPSILON) + }; + + for frames_per_span in [4096usize, 2048, 1024, 512, 256, 128] { + let span = frames_per_span * channels.get() as usize; + let mut chunks = signal.chunks(span); + let mut source = TestSource::new(chunks.next().unwrap().to_vec(), source_rate, channels); + for chunk in chunks { + source = source.chain(chunk.to_vec(), source_rate, channels); + } + + let output: Vec = + SampleRateConverter::new(source, target_rate, ResampleConfig::poly().build()).collect(); + let fraction = tone_energy_fraction(&output); + + assert!( + fraction > 0.99, + "{frames_per_span}-frame spans: only {:.1}% of output energy at the \ + fundamental (expected >99%) — span boundaries are corrupting the signal", + fraction * 100.0 + ); + } +} From b1c0a1b5020b5dc4d129593f7fab47772b0a5ae9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Rouleau?= Date: Sat, 25 Jul 2026 14:09:46 -0400 Subject: [PATCH 2/3] Make the span-chunking test agnostic to the 64bit feature `Sample` is `f32` by default and `f64` under the `64bit` feature, so hardcoding `f32` in the signal generator broke `--all-features` builds. Generate the tone in `Sample` instead. Verified with the CI command under both feature sets. --- src/conversions/sample_rate/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conversions/sample_rate/tests.rs b/src/conversions/sample_rate/tests.rs index af098310..e4e20a6c 100644 --- a/src/conversions/sample_rate/tests.rs +++ b/src/conversions/sample_rate/tests.rs @@ -355,8 +355,8 @@ fn small_spans_do_not_degrade_resampled_signal() { // 0.5 s of a 440 Hz stereo sine at 48 kHz. let signal: Vec = (0..24_000) .flat_map(|i| { - let t = i as f32 / source_rate.get() as f32; - let s = (std::f32::consts::TAU * 440.0 * t).sin() * 0.5; + let t = i as Sample / source_rate.get() as Sample; + let s = (std::f64::consts::TAU as Sample * 440.0 * t).sin() * 0.5; [s, s] }) .collect(); From 2b26a3639b8dc884b9e2d19993c1c41ae04ad1f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Rouleau?= Date: Sat, 25 Jul 2026 14:11:28 -0400 Subject: [PATCH 3/3] Keep clippy clean under both feature sets The f64 accumulation cast is a no-op with `64bit` enabled but required without it, so annotate rather than drop it. --- src/conversions/sample_rate/tests.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/conversions/sample_rate/tests.rs b/src/conversions/sample_rate/tests.rs index e4e20a6c..5412ee9f 100644 --- a/src/conversions/sample_rate/tests.rs +++ b/src/conversions/sample_rate/tests.rs @@ -364,6 +364,9 @@ fn small_spans_do_not_degrade_resampled_signal() { // Fraction of output energy sitting at the 440 Hz fundamental, via // Goertzel on the left channel. A clean resample keeps this ~1.0. let tone_energy_fraction = |samples: &[Sample]| -> f64 { + // Accumulate in f64 regardless of `Sample`: the cast is a no-op + // under the `64bit` feature but needed without it. + #[allow(clippy::unnecessary_cast)] let left: Vec = samples.iter().step_by(2).map(|&s| s as f64).collect(); let n = left.len(); let bin = 440.0 * n as f64 / target_rate.get() as f64;