From b65e416a137fa82c1d8aedcd6f8f6783ec15d4e9 Mon Sep 17 00:00:00 2001 From: Roderick van Domburg Date: Mon, 22 Dec 2025 21:49:03 +0100 Subject: [PATCH] fix: audio distortion at queue source boundaries Fixes incorrect sample_rate() and channels() metadata returned by SourcesQueueOutput when transitioning between sources with different formats or from an empty queue. This caused audio distortion at boundaries where metadata changes occurred. Changes: - Clarified Source::current_span_len() contract: returns total span size, Some(0) when exhausted - Fixed SamplesBuffer to return Some(total_size) or Some(0) when exhausted (was None) - Fixed SamplesBuffer::size_hint() to return remaining samples - Updated SourcesQueueOutput to peek next source metadata when current is exhausted - Added Source::is_exhausted() helper method for cleaner exhaustion checks throughout codebase - Implemented ExactSizeIterator for SamplesBuffer - Improved SkipDuration precision to avoid rounding errors This supersedes PR #812 with a cleaner implementation following the proper current_span_len() contract. Enabled previously ignored queue::tests::basic test to prevent regressions. Fixes #811 Co-authored-by: 0----0 <9070490+0----0@users.noreply.github.com> --- CHANGELOG.md | 6 ++++++ src/buffer.rs | 11 ++++++++-- src/queue.rs | 45 +++++++++++++++++++++++++++++------------ src/source/from_iter.rs | 6 ++---- src/source/mod.rs | 16 ++++++++++++--- src/source/repeat.rs | 21 ++++++++++--------- src/source/skip.rs | 16 +++++++++------ 7 files changed, 84 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48573720..b7bfc2d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - `Chirp` now implements `Iterator::size_hint` and `ExactSizeIterator`. +- `SamplesBuffer` now implements `ExactSizeIterator`. +- Added `Source::is_exhausted()` helper method to check if a source has no more samples. - Added `Red` noise generator that is more practical than `Brownian` noise. - Added `std_dev()` to `WhiteUniform` and `WhiteTriangular`. - Added a macro `nz!` which facilitates creating NonZero's for `SampleRate` and @@ -31,6 +33,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Chirp::next` now returns `None` when the total duration has been reached, and will work correctly for a number of samples greater than 2^24. - `PeriodicAccess` is slightly more accurate for 44.1 kHz sample rate families. +- Fixed audio distortion when queueing sources with different sample rates/channel counts or transitioning from empty queue. +- Fixed `SamplesBuffer` to correctly report exhaustion and remaining samples. +- Improved precision in `SkipDuration` to avoid off-by-a-few-samples errors. ### Changed - `output_to_wav` renamed to `wav_to_file` and now takes ownership of the `Source`. @@ -38,6 +43,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Gaussian` noise generator has standard deviation of 0.6 for perceptual equivalence. - `Velvet` noise generator takes density in Hz as `usize` instead of `f32`. - Upgrade `cpal` to v0.17. +- Clarified `Source::current_span_len()` contract documentation. ## Version [0.21.1] (2025-07-14) diff --git a/src/buffer.rs b/src/buffer.rs index 305f6b1f..923a8336 100644 --- a/src/buffer.rs +++ b/src/buffer.rs @@ -73,7 +73,11 @@ impl SamplesBuffer { impl Source for SamplesBuffer { #[inline] fn current_span_len(&self) -> Option { - None + if self.pos >= self.data.len() { + Some(0) + } else { + Some(self.data.len()) + } } #[inline] @@ -126,10 +130,13 @@ impl Iterator for SamplesBuffer { #[inline] fn size_hint(&self) -> (usize, Option) { - (self.data.len(), Some(self.data.len())) + let remaining = self.data.len() - self.pos; + (remaining, Some(remaining)) } } +impl ExactSizeIterator for SamplesBuffer {} + #[cfg(test)] mod tests { use crate::buffer::SamplesBuffer; diff --git a/src/queue.rs b/src/queue.rs index 495606b4..82dcdd63 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -113,6 +113,8 @@ pub struct SourcesQueueOutput { } const THRESHOLD: usize = 512; +const SILENCE_SAMPLE_RATE: SampleRate = nz!(44100); +const SILENCE_CHANNELS: ChannelCount = nz!(1); impl Source for SourcesQueueOutput { #[inline] @@ -129,15 +131,13 @@ impl Source for SourcesQueueOutput { // constant. // Try the current `current_span_len`. - if let Some(val) = self.current.current_span_len() { - if val != 0 { - return Some(val); - } else if self.input.keep_alive_if_empty.load(Ordering::Acquire) - && self.input.next_sounds.lock().unwrap().is_empty() - { - // The next source will be a filler silence which will have the length of `THRESHOLD` - return Some(THRESHOLD); - } + if !self.current.is_exhausted() { + return self.current.current_span_len(); + } else if self.input.keep_alive_if_empty.load(Ordering::Acquire) + && self.input.next_sounds.lock().unwrap().is_empty() + { + // The next source will be a filler silence which will have the length of `THRESHOLD` + return Some(THRESHOLD); } // Try the size hint. @@ -154,12 +154,28 @@ impl Source for SourcesQueueOutput { #[inline] fn channels(&self) -> ChannelCount { - self.current.channels() + // When current source is exhausted, peek at the next source's metadata + if !self.current.is_exhausted() { + self.current.channels() + } else if let Some((next, _)) = self.input.next_sounds.lock().unwrap().first() { + next.channels() + } else { + // Queue is empty - return silence metadata + SILENCE_CHANNELS + } } #[inline] fn sample_rate(&self) -> SampleRate { - self.current.sample_rate() + // When current source is exhausted, peek at the next source's metadata + if !self.current.is_exhausted() { + self.current.sample_rate() + } else if let Some((next, _)) = self.input.next_sounds.lock().unwrap().first() { + next.sample_rate() + } else { + // Queue is empty - return silence metadata + SILENCE_SAMPLE_RATE + } } #[inline] @@ -221,7 +237,11 @@ impl SourcesQueueOutput { let mut next = self.input.next_sounds.lock().unwrap(); if next.is_empty() { - let silence = Box::new(Zero::new_samples(nz!(1), nz!(44100), THRESHOLD)) as Box<_>; + let silence = Box::new(Zero::new_samples( + SILENCE_CHANNELS, + SILENCE_SAMPLE_RATE, + THRESHOLD, + )) as Box<_>; if self.input.keep_alive_if_empty.load(Ordering::Acquire) { // Play a short silence in order to avoid spinlocking. (silence, None) @@ -247,7 +267,6 @@ mod tests { use crate::source::Source; #[test] - #[ignore] // FIXME: samples rate and channel not updated immediately after transition fn basic() { let (tx, mut rx) = queue::queue(false); diff --git a/src/source/from_iter.rs b/src/source/from_iter.rs index 59845758..c8c14062 100644 --- a/src/source/from_iter.rs +++ b/src/source/from_iter.rs @@ -92,10 +92,8 @@ where // Try the current `current_span_len`. if let Some(src) = &self.current_source { - if let Some(val) = src.current_span_len() { - if val != 0 { - return Some(val); - } + if !src.is_exhausted() { + return src.current_span_len(); } } diff --git a/src/source/mod.rs b/src/source/mod.rs index c7d469f8..bf222b92 100644 --- a/src/source/mod.rs +++ b/src/source/mod.rs @@ -168,14 +168,24 @@ pub use self::noise::{Pink, WhiteUniform}; /// channels can potentially change. /// pub trait Source: Iterator { - /// Returns the number of samples before the current span ends. `None` means "infinite" or - /// "until the sound ends". - /// Should never return 0 unless there's no more data. + /// Returns the number of samples before the current span ends. + /// + /// `None` means "infinite" or "until the sound ends". Sources that return `Some(x)` should + /// return `Some(0)` if and only if when there's no more data. /// /// After the engine has finished reading the specified number of samples, it will check /// whether the value of `channels()` and/or `sample_rate()` have changed. + /// + /// Note: This returns the total span size, not the remaining samples. Use `Iterator::size_hint` + /// to determine how many samples remain in the iterator. fn current_span_len(&self) -> Option; + /// Returns true if the source is exhausted (has no more samples available). + #[inline] + fn is_exhausted(&self) -> bool { + self.current_span_len() == Some(0) + } + /// Returns the number of channels. Channels are always interleaved. /// Should never be Zero fn channels(&self) -> ChannelCount; diff --git a/src/source/repeat.rs b/src/source/repeat.rs index 12a6b947..ef29968e 100644 --- a/src/source/repeat.rs +++ b/src/source/repeat.rs @@ -56,25 +56,28 @@ where { #[inline] fn current_span_len(&self) -> Option { - match self.inner.current_span_len() { - Some(0) => self.next.current_span_len(), - a => a, + if self.inner.is_exhausted() { + self.next.current_span_len() + } else { + self.inner.current_span_len() } } #[inline] fn channels(&self) -> ChannelCount { - match self.inner.current_span_len() { - Some(0) => self.next.channels(), - _ => self.inner.channels(), + if self.inner.is_exhausted() { + self.next.channels() + } else { + self.inner.channels() } } #[inline] fn sample_rate(&self) -> SampleRate { - match self.inner.current_span_len() { - Some(0) => self.next.sample_rate(), - _ => self.inner.sample_rate(), + if self.inner.is_exhausted() { + self.next.sample_rate() + } else { + self.inner.sample_rate() } } diff --git a/src/source/skip.rs b/src/source/skip.rs index 36f8d210..d3c43857 100644 --- a/src/source/skip.rs +++ b/src/source/skip.rs @@ -39,18 +39,22 @@ where return; } - let ns_per_sample: u128 = - NS_PER_SECOND / input.sample_rate().get() as u128 / input.channels().get() as u128; + let sample_rate = input.sample_rate().get() as u128; + let channels = input.channels().get() as u128; + + let samples_per_channel = duration.as_nanos() * sample_rate / NS_PER_SECOND; + let samples_to_skip: u128 = samples_per_channel * channels; // Check if we need to skip only part of the current span. - if span_len as u128 * ns_per_sample > duration.as_nanos() { - skip_samples(input, (duration.as_nanos() / ns_per_sample) as usize); + if span_len as u128 > samples_to_skip { + skip_samples(input, samples_to_skip as usize); return; } + duration -= Duration::from_nanos( + (NS_PER_SECOND * span_len as u128 / channels / sample_rate) as u64, + ); skip_samples(input, span_len); - - duration -= Duration::from_nanos((span_len * ns_per_sample as usize) as u64); } }