Assorted resampler fixes#841
Open
padenot wants to merge 10 commits into
Open
Conversation
The interleaved speex API writes frames*channels samples into the output buffer, but output_frame_count was set to LATENCY_SAMPLES (a sample count), causing a stack overflow when channels > 1 and the upsampling ratio is high. Divide by channels to derive the correct frame-count ceiling.
The assert was checking the pre-clamped input_latency*channels, which can exceed LATENCY_SAMPLES for high channel counts even though the code is safe. Check output_frame_count*channels instead, which is guaranteed ≤ LATENCY_SAMPLES by construction.
The priming pass must process exactly input_latency frames of silence to fully initialize the speex filter delay memory. With large channel counts the per-pass frame budget (latency_frames = LATENCY_SAMPLES/channels) may be smaller than input_latency, so loop until all frames are consumed.
It was never useful in practice, since it's two opposite directions.
auto_array::reserve unconditionally did new[]+memcpy+delete[] even when the capacity was already sufficient. This was hit on every audio callback via cubeb_resampler_speex_one_way::input_buffer, making output and duplex resampling heap-allocate on the real-time thread. Fix reserve to return early when capacity is already >= the requested size (matching std::vector::reserve semantics). Add amortized 2x growth to push/push_silence/push_front_silence so that transient buffer growth (glitches) converges to a stable allocation quickly. Delete copy/move operations on auto_array to prevent accidental double-free from the compiler-generated shallow-copy.
The speex_resample wrappers only captured the return value inside #ifndef NDEBUG, so in release builds the error was silently discarded. Always check and log via the RT-safe async logger on failure.
Verify that reserve at or below current capacity never reallocates (pointer stability), and that push growth at least doubles capacity to amortize allocations.
For each rate pair and block size, verify that feeding exactly input_needed_for_output(N) frames into the resampler produces exactly N output frames. Catches precision regressions in the frame calculation.
The monotonic_state checks were printf-only, so unbounded internal buffer growth would go unnoticed in CI. Use gtest EXPECT/ADD_FAILURE so buffer buildup actually fails the test.
kinetiknz
approved these changes
Apr 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This stems from BMO#2029067 but includes nice things, notably the allocation bits, that could be significant (depending on the input/output rate ratio). Also includes simplifications.