From 6c159669bd1a5375820194d8e3832c281d332186 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benoi=CC=82t=20Rouleau?= Date: Wed, 5 Aug 2026 18:03:23 -0400 Subject: [PATCH 1/2] resampler: don't let span chunking change the output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fill_input_buffer` treats `current_span_len()` as the samples left in the span, but it's the span's total length, so it stops reading at the end of every span. Vorbis packets are smaller than a resampler chunk, so every chunk ends up short, gets marked `partial_len`, and rubato pads it with zeros. The same audio comes out different depending on how the decoder split it. Stop only where the rate or channel count actually changes, and check that before reading anything from the next span. When reading stops there, rebuild the resampler. That fixes a second bug: a span that changed the sample rate was still being resampled at the old one. 441 frames at 44.1k then 441 at 22.05k, converted to 48k, gave 962 samples instead of 1440. Drop the counter the boundary detection was using — reading stops at the change itself now, so there's nothing left to count. Only the passthrough still needs it. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 1 + src/conversions/sample_rate/mod.rs | 167 +++++++++++--------------- src/conversions/sample_rate/rubato.rs | 76 +++++++++--- src/conversions/sample_rate/tests.rs | 106 ++++++++++++++++ 4 files changed, 237 insertions(+), 113 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0f149de..8bcc53bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed sources to detect parameter updates after mid-span seeks. - Fixed `Stoppable` and `Skippable` not signaling exhaustion. - Fixed `SpatialAudio` left and write channel swapping +- Fixed resampled output depending on how the input is split into spans. ## Version [0.22.2] (2026-03-05) diff --git a/src/conversions/sample_rate/mod.rs b/src/conversions/sample_rate/mod.rs index 7df49d64..0d932ca8 100644 --- a/src/conversions/sample_rate/mod.rs +++ b/src/conversions/sample_rate/mod.rs @@ -167,6 +167,28 @@ where } } + /// Rebuild for the format the input has moved on to. + fn recreate(&mut self) { + let source = self.inner.take().expect("always set").into_inner(); + self.inner = Some(Self::create_resampler( + source, + self.target_rate, + &self.config, + )); + self.pending_recreate = false; + self.cached_input_span_len = self.resampler().input().current_span_len(); + } + + fn next_resampled(&mut self) -> Option { + match self.resampler_mut() { + ResampleInner::Passthrough { source, .. } => source.next(), + ResampleInner::Poly(resampler) => resampler.next_sample(), + ResampleInner::Sinc(resampler) => resampler.next_sample(), + #[cfg(feature = "rubato-fft")] + ResampleInner::Fft(resampler) => resampler.next_sample(), + } + } + /// Helper method to create a resampler from a source using the stored config and target rate. fn create_resampler( source: I, @@ -330,35 +352,17 @@ where self.pending_recreate = false; let input_span_len = self.resampler().input().current_span_len(); - match self.inner.as_mut().unwrap() { - ResampleInner::Passthrough { - input_span_pos: input_samples_consumed, - .. - } => { - reset_seek_span_tracking( - input_samples_consumed.raw_mut(), - &mut self.cached_input_span_len, - position, - input_span_len, - ); - } - ResampleInner::Poly(r) | ResampleInner::Sinc(r) => { - reset_seek_span_tracking( - r.pos_in_current_span.raw_mut(), - &mut self.cached_input_span_len, - position, - input_span_len, - ); - } - #[cfg(feature = "rubato-fft")] - ResampleInner::Fft(r) => { - reset_seek_span_tracking( - r.pos_in_current_span.raw_mut(), - &mut self.cached_input_span_len, - position, - input_span_len, - ); - } + // Only the passthrough counts samples against the span length; + // `reset` already cleared the resamplers' own tracking. + if let ResampleInner::Passthrough { input_span_pos, .. } = + self.inner.as_mut().expect("always set") + { + reset_seek_span_tracking( + input_span_pos.raw_mut(), + &mut self.cached_input_span_len, + position, + input_span_len, + ); } Ok(()) @@ -376,60 +380,46 @@ where // If a format change was detected at the previous span boundary, wait until the // output buffer is fully drained before recreating the resampler. This guarantees // that fill_input_buffer only ever reads from the current span. - if self.pending_recreate { - let output_empty = match self.resampler() { - ResampleInner::Passthrough { .. } => true, - ResampleInner::Poly(r) | ResampleInner::Sinc(r) => !r.output_has_samples(), - #[cfg(feature = "rubato-fft")] - ResampleInner::Fft(r) => !r.output_has_samples(), - }; - if output_empty { - let source = self.inner.take().unwrap().into_inner(); - self.inner = Some(Self::create_resampler( - source, - self.target_rate, - &self.config, - )); - self.pending_recreate = false; + let sample = loop { + if self.pending_recreate { + let output_empty = match self.resampler() { + ResampleInner::Passthrough { .. } => true, + ResampleInner::Poly(r) | ResampleInner::Sinc(r) => !r.output_has_samples(), + #[cfg(feature = "rubato-fft")] + ResampleInner::Fft(r) => !r.output_has_samples(), + }; + if output_empty { + self.recreate(); + } } - } - let sample = match self.resampler_mut() { - ResampleInner::Passthrough { source, .. } => source.next()?, - ResampleInner::Poly(resampler) => resampler.next_sample()?, - ResampleInner::Sinc(resampler) => resampler.next_sample()?, - #[cfg(feature = "rubato-fft")] - ResampleInner::Fft(resampler) => resampler.next_sample()?, + match self.next_resampled() { + Some(sample) => break sample, + // Built for one format, a resampler stops where its input + // changes rate or channels. Rebuilding matches the format + // it stopped at, so this retries at most once. + None if self.resampler().built_for_input() => return None, + None => self.recreate(), + } }; - // If input reports no span length, parameters are stable by contract - let input_span_len = self.resampler().input().current_span_len(); - if input_span_len.is_none() { + // Only the passthrough tracks spans here; a resampler stops at a + // format change itself while filling, and the loop above rebuilds it. + let ResampleInner::Passthrough { + input_span_pos, + channels, + source_rate, + source, + } = self.resampler_mut() + else { return Some(sample); - } - - let (expected_channels, expected_rate, samples_consumed) = match self.resampler_mut() { - ResampleInner::Passthrough { - input_span_pos: input_samples_consumed, - channels, - source_rate, - .. - } => { - *input_samples_consumed += 1usize; - (*channels, *source_rate, *input_samples_consumed) - } - ResampleInner::Poly(r) | ResampleInner::Sinc(r) => ( - r.output.channels, - r.input.sample_rate(), - r.pos_in_current_span, - ), - #[cfg(feature = "rubato-fft")] - ResampleInner::Fft(r) => ( - r.output.channels, - r.input.sample_rate(), - r.pos_in_current_span, - ), }; + let input_span_len = source.current_span_len(); + // No span length means the format is stable by contract. + input_span_len?; + *input_span_pos += 1usize; + let (expected_channels, expected_rate, samples_consumed) = + (*channels, *source_rate, *input_span_pos); let input = self.resampler().input(); let (at_boundary, parameters_changed) = Self::detect_boundary( @@ -442,30 +432,11 @@ where ); if at_boundary { - // Update cached span length (exits detection mode if we were in it) self.cached_input_span_len = input_span_len; - if parameters_changed { - // Defer recreation until the output buffer is drained (handled above at the - // top of the next next() call) so no cross-span reads occur. self.pending_recreate = true; - } else { - // Just crossed boundary without parameter change, reset counter - match self.resampler_mut() { - ResampleInner::Passthrough { - input_span_pos: input_samples_consumed, - .. - } => { - *input_samples_consumed = InSamples::ZERO; - } - ResampleInner::Poly(r) | ResampleInner::Sinc(r) => { - r.pos_in_current_span = InSamples::ZERO; - } - #[cfg(feature = "rubato-fft")] - ResampleInner::Fft(r) => { - r.pos_in_current_span = InSamples::ZERO; - } - } + } else if let ResampleInner::Passthrough { input_span_pos, .. } = self.resampler_mut() { + *input_span_pos = InSamples::ZERO; } } diff --git a/src/conversions/sample_rate/rubato.rs b/src/conversions/sample_rate/rubato.rs index df1ee016..c2bc3d04 100644 --- a/src/conversions/sample_rate/rubato.rs +++ b/src/conversions/sample_rate/rubato.rs @@ -57,6 +57,26 @@ impl ResampleInner { } } + /// Whether the input still has the format this was built for. + #[inline] + pub fn built_for_input(&self) -> bool { + let (channels, source_rate) = match self { + ResampleInner::Passthrough { + channels, + source_rate, + .. + } => (*channels, *source_rate), + ResampleInner::Poly(resampler) | ResampleInner::Sinc(resampler) => { + (resampler.output.channels, resampler.output.source_rate) + } + #[cfg(feature = "rubato-fft")] + ResampleInner::Fft(resampler) => { + (resampler.output.channels, resampler.output.source_rate) + } + }; + self.input().channels() == channels && self.input().sample_rate() == source_rate + } + /// Extract the inner input source, consuming the resampler #[inline] pub fn into_inner(self) -> I { @@ -83,7 +103,11 @@ pub struct RubatoResample> { pub resample_ratio: Float, pub output_delay_remaining: OutFrameCount, - pub pos_in_current_span: InSamples, + + /// How much of the span being read is left. Tracked because + /// `Source::current_span_len` reports the span's total length, + /// not its remainder. + span_remaining: InSamples, pub frames_being_resampled: OutFrameCount, } @@ -118,7 +142,7 @@ impl> RubatoResample { pub fn reset(&mut self) { self.resampler.reset(); self.output.reset(); - self.pos_in_current_span = InSamples::ZERO; + self.span_remaining = InSamples::ZERO; self.output_delay_remaining = Self::output_delay(&self.resampler); } @@ -147,7 +171,6 @@ impl> RubatoResample { } self.frames_being_resampled += frames_in.resampled_by(self.resample_ratio); - self.pos_in_current_span += frames_in.samples(self.output.channels); let indexing = Some(&rubato::Indexing { input_offset: 0, @@ -192,23 +215,46 @@ impl> RubatoResample { Some(()) } + /// Reads across spans, since the resampler is built for one format + /// and only a span that changes it has to end the chunk. A short + /// read then means end of stream or a format change, which is when + /// rubato should treat the chunk as partial. 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 { + + while self.input_buffer.len() < wanted { + if self.span_remaining == InSamples::ZERO && !self.start_of_span() { break; } + let Some(sample) = self.input.next() else { + break; + }; + self.input_buffer.push(sample); + self.span_remaining -= InSamples(1); } + self.input_buffer.len().frames(self.output.channels) } + /// Whether reading may continue into the next span, recording its + /// length. `false` once the stream ends or the format changes. + fn start_of_span(&mut self) -> bool { + let Some(span_len) = self.input.current_span_len() else { + // Without spans the format never changes. + self.span_remaining = InSamples::MAX; + return true; + }; + if span_len == 0 + || self.input.channels() != self.output.channels + || self.input.sample_rate() != self.output.source_rate + { + return false; + } + self.span_remaining = InSamples(span_len); + true + } + fn resampler_empty(&self) -> bool { self.frames_being_resampled == OutFrameCount::ZERO } @@ -247,7 +293,7 @@ impl RubatoAsyncResample { resampler, input_buffer: Input::new(input_buf_size.samples(channels)), output: Output::new(source_rate, channels, output_buf_size), - pos_in_current_span: InSamples::ZERO, + span_remaining: InSamples::ZERO, output_delay_remaining: initial_output_delay, resample_ratio, frames_being_resampled: OutFrameCount::ZERO, @@ -298,7 +344,7 @@ impl RubatoAsyncResample { resampler, input_buffer: Input::new(input_buf_size.samples(channels)), output: Output::new(source_rate, channels, output_buf_size), - pos_in_current_span: InSamples::ZERO, + span_remaining: InSamples::ZERO, output_delay_remaining: initial_output_delay, resample_ratio, frames_being_resampled: OutFrameCount::ZERO, @@ -348,7 +394,7 @@ impl RubatoFftResample { resampler, input_buffer: Input::new(input_buf_size.samples(channels)), output: Output::new(source_rate, channels, output_buf_size), - pos_in_current_span: InSamples::ZERO, + span_remaining: InSamples::ZERO, output_delay_remaining, resample_ratio, frames_being_resampled: OutFrameCount::ZERO, diff --git a/src/conversions/sample_rate/tests.rs b/src/conversions/sample_rate/tests.rs index c2ea52cc..6ca08cea 100644 --- a/src/conversions/sample_rate/tests.rs +++ b/src/conversions/sample_rate/tests.rs @@ -338,3 +338,109 @@ fn test_span_boundary_same_format() { output.len() ); } + +/// Split `input` into spans of `span_samples`, all with the same format. +fn chunked_into_spans( + input: &[Sample], + span_samples: usize, + rate: SampleRate, + channels: ChannelCount, +) -> TestSource { + let mut spans = input.chunks(span_samples); + let first = spans.next().expect("input is not empty").to_vec(); + spans.fold(TestSource::new(first, rate, channels), |source, span| { + source.chain(span.to_vec(), rate, channels) + }) +} + +/// Same samples, same format, different span lengths must resample to +/// the same output: a span boundary without a format change is not a +/// discontinuity. Reproduces #907, where Vorbis-sized spans made every +/// chunk look like end-of-stream and get zero-padded. +#[test] +fn output_is_independent_of_span_chunking() { + let channels = ChannelCount::new(2).unwrap(); + let rate = SampleRate::new(44100).unwrap(); + let target = SampleRate::new(48000).unwrap(); + let input = create_test_input(InFrameCount(4410), channels); + let config = || ResampleConfig::poly().build(); + + let whole: Vec = SampleRateConverter::new( + TestSource::new(input.clone(), rate, channels), + target, + config(), + ) + .collect(); + + // 128 frames is a typical Vorbis short block. + let source = chunked_into_spans(&input, 128 * channels.get() as usize, rate, channels); + let chunked: Vec = SampleRateConverter::new(source, target, config()).collect(); + + assert_eq!(whole.len(), chunked.len(), "output length changed"); + let worst = whole + .iter() + .zip(&chunked) + .map(|(a, b)| (a - b).abs()) + .fold(0.0, Sample::max); + assert!(worst < 1e-4, "output differs by up to {worst}"); +} + +/// A span that changes the sample rate must be resampled at its own +/// rate. It used to be resampled at the previous span's rate, so a +/// span that halved the rate came out half as long as it should. +#[test] +fn span_boundary_may_change_sample_rate() { + let channels = ChannelCount::new(1).unwrap(); + let target = SampleRate::new(48000).unwrap(); + let (fast, slow) = ( + SampleRate::new(44100).unwrap(), + SampleRate::new(22050).unwrap(), + ); + let frames = InFrameCount(441); + + let source = TestSource::new(create_test_input(frames, channels), fast, channels).chain( + create_test_input(frames, channels), + slow, + channels, + ); + let output: Vec = + SampleRateConverter::new(source, target, ResampleConfig::poly().build()).collect(); + + let expected = resampled_len(frames, fast, target) + resampled_len(frames, slow, target); + assert_close(output.len(), expected, "sample rate change"); +} + +/// The same for a span that changes the channel count. +#[test] +fn span_boundary_may_change_channel_count() { + let (stereo, mono) = (ChannelCount::new(2).unwrap(), ChannelCount::new(1).unwrap()); + let rate = SampleRate::new(44100).unwrap(); + let target = SampleRate::new(48000).unwrap(); + let frames = InFrameCount(441); + + let source = TestSource::new(create_test_input(frames, stereo), rate, stereo).chain( + create_test_input(frames, mono), + rate, + mono, + ); + let output: Vec = + SampleRateConverter::new(source, target, ResampleConfig::poly().build()).collect(); + + let expected = resampled_len(frames, rate, target) * 2 + resampled_len(frames, rate, target); + assert_close(output.len(), expected, "channel count change"); +} + +/// Samples one span of `frames` becomes at `target`, one channel. +fn resampled_len(frames: InFrameCount, from: SampleRate, target: SampleRate) -> usize { + (frames.raw() as f64 * target.get() as f64 / from.get() as f64).round() as usize +} + +/// Resampler delay and rounding move the total by a few samples, so +/// compare with room for that rather than exactly. +fn assert_close(got: usize, expected: usize, what: &str) { + let slack = 16; + assert!( + got.abs_diff(expected) <= slack, + "{what}: got {got} samples, expected {expected} ± {slack}", + ); +} From 2f4c14ea4ce1a7661c3b76cb0334ea50adf42a86 Mon Sep 17 00:00:00 2001 From: Patrick Hayes Date: Thu, 6 Aug 2026 06:36:19 -0700 Subject: [PATCH 2/2] Fixing up confusion between None meaning end-of-stream and None meaning no-spans (stable stream format) --- src/conversions/sample_rate/mod.rs | 4 +++- src/conversions/sample_rate/tests.rs | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/conversions/sample_rate/mod.rs b/src/conversions/sample_rate/mod.rs index 0d932ca8..2f81a11f 100644 --- a/src/conversions/sample_rate/mod.rs +++ b/src/conversions/sample_rate/mod.rs @@ -416,7 +416,9 @@ where }; let input_span_len = source.current_span_len(); // No span length means the format is stable by contract. - input_span_len?; + if input_span_len.is_none() { + return Some(sample); + } *input_span_pos += 1usize; let (expected_channels, expected_rate, samples_consumed) = (*channels, *source_rate, *input_span_pos); diff --git a/src/conversions/sample_rate/tests.rs b/src/conversions/sample_rate/tests.rs index 6ca08cea..4aa47bcc 100644 --- a/src/conversions/sample_rate/tests.rs +++ b/src/conversions/sample_rate/tests.rs @@ -444,3 +444,16 @@ fn assert_close(got: usize, expected: usize, what: &str) { "{what}: got {got} samples, expected {expected} ± {slack}", ); } + +/// Passthrough must keep yielding when the input reports no span length. +#[test] +fn passthrough_none_span_len_yields_samples() { + let rate = crate::DEFAULT_SAMPLE_RATE; + let mut out = + SampleRateConverter::new(SineWave::new(440.0), rate, ResampleConfig::poly().build()); + let n = (&mut out).take(32).count(); + assert_eq!( + n, 32, + "Passthrough with current_span_len() == None stopped early (got {n})" + ); +}