From 1936be60857b1fa7a8b3061c9ed67e07461c6305 Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Thu, 16 Apr 2026 12:00:21 +0000 Subject: [PATCH 01/10] Fix stack buffer overflow in cubeb_resampler_speex_one_way constructor 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. --- src/cubeb_resampler_internal.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cubeb_resampler_internal.h b/src/cubeb_resampler_internal.h index 08e019c6..b35ec4f7 100644 --- a/src/cubeb_resampler_internal.h +++ b/src/cubeb_resampler_internal.h @@ -211,8 +211,10 @@ template class cubeb_resampler_speex_one_way : public processor { const size_t LATENCY_SAMPLES = 8192; T input_buffer[LATENCY_SAMPLES] = {}; T output_buffer[LATENCY_SAMPLES] = {}; - uint32_t input_frame_count = input_latency; - uint32_t output_frame_count = LATENCY_SAMPLES; + const uint32_t latency_frames = + LATENCY_SAMPLES / std::max(channels, 1); + uint32_t input_frame_count = std::min(input_latency, latency_frames); + uint32_t output_frame_count = latency_frames; assert(input_latency * channels <= LATENCY_SAMPLES); speex_resample(input_buffer, &input_frame_count, output_buffer, &output_frame_count); From 47b80ec0e29bc8cb08c118f5ac9cfe66a72b2f2c Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Thu, 16 Apr 2026 12:00:35 +0000 Subject: [PATCH 02/10] Fix stale assert in resampler priming pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/cubeb_resampler_internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cubeb_resampler_internal.h b/src/cubeb_resampler_internal.h index b35ec4f7..dfc93f15 100644 --- a/src/cubeb_resampler_internal.h +++ b/src/cubeb_resampler_internal.h @@ -215,7 +215,7 @@ template class cubeb_resampler_speex_one_way : public processor { LATENCY_SAMPLES / std::max(channels, 1); uint32_t input_frame_count = std::min(input_latency, latency_frames); uint32_t output_frame_count = latency_frames; - assert(input_latency * channels <= LATENCY_SAMPLES); + assert(output_frame_count * channels <= LATENCY_SAMPLES); speex_resample(input_buffer, &input_frame_count, output_buffer, &output_frame_count); } From a5a41736945932c7e16806389af6ac6e5573fc1c Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Thu, 16 Apr 2026 12:00:52 +0000 Subject: [PATCH 03/10] Complete resampler priming for all input_latency frames 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. --- src/cubeb_resampler_internal.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/cubeb_resampler_internal.h b/src/cubeb_resampler_internal.h index dfc93f15..47e8ded3 100644 --- a/src/cubeb_resampler_internal.h +++ b/src/cubeb_resampler_internal.h @@ -213,11 +213,15 @@ template class cubeb_resampler_speex_one_way : public processor { T output_buffer[LATENCY_SAMPLES] = {}; const uint32_t latency_frames = LATENCY_SAMPLES / std::max(channels, 1); - uint32_t input_frame_count = std::min(input_latency, latency_frames); - uint32_t output_frame_count = latency_frames; - assert(output_frame_count * channels <= LATENCY_SAMPLES); - speex_resample(input_buffer, &input_frame_count, output_buffer, - &output_frame_count); + assert(latency_frames * channels <= LATENCY_SAMPLES); + uint32_t remaining = input_latency; + while (remaining > 0) { + uint32_t input_frame_count = std::min(remaining, latency_frames); + uint32_t output_frame_count = latency_frames; + speex_resample(input_buffer, &input_frame_count, output_buffer, + &output_frame_count); + remaining -= input_frame_count; + } } /** Destructor, deallocate the resampler */ From e9ed33d21107f7196464ca8c61bc6a893e4145d5 Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Thu, 16 Apr 2026 14:26:28 +0200 Subject: [PATCH 04/10] Remove the delay line It was never useful in practice, since it's two opposite directions. --- src/cubeb_resampler.cpp | 75 +++++++----- src/cubeb_resampler_internal.h | 209 +++++---------------------------- test/test_resampler.cpp | 64 +--------- 3 files changed, 83 insertions(+), 265 deletions(-) diff --git a/src/cubeb_resampler.cpp b/src/cubeb_resampler.cpp index c09ea4b9..3301ca99 100644 --- a/src/cubeb_resampler.cpp +++ b/src/cubeb_resampler.cpp @@ -131,16 +131,21 @@ template cubeb_resampler_speex:: cubeb_resampler_speex(InputProcessor * input_processor, OutputProcessor * output_processor, cubeb_stream * s, - cubeb_data_callback cb, void * ptr) + cubeb_data_callback cb, void * ptr, + cubeb_resampler_direction direction) : input_processor(input_processor), output_processor(output_processor), stream(s), data_callback(cb), user_ptr(ptr) { - if (input_processor && output_processor) { + switch (direction) { + case cubeb_resampler_direction::DUPLEX: fill_internal = &cubeb_resampler_speex::fill_internal_duplex; - } else if (input_processor) { + break; + case cubeb_resampler_direction::INPUT: fill_internal = &cubeb_resampler_speex::fill_internal_input; - } else if (output_processor) { + break; + case cubeb_resampler_direction::OUTPUT: fill_internal = &cubeb_resampler_speex::fill_internal_output; + break; } } @@ -250,7 +255,10 @@ cubeb_resampler_speex::fill_internal_duplex( { if (draining) { // discard input and drain any signal remaining in the resampler. - return output_processor->output(out_buffer, output_frames_needed); + if (output_processor) { + return output_processor->output(out_buffer, output_frames_needed); + } + return 0; } /* The input data, after eventual resampling. This is passed to the callback. @@ -273,23 +281,31 @@ cubeb_resampler_speex::fill_internal_duplex( * get the output data, and resample it to the number of frames needed by the * caller. */ - output_frames_before_processing = - output_processor->input_needed_for_output(output_frames_needed); - /* fill directly the input buffer of the output processor to save a copy */ - out_unprocessed = - output_processor->input_buffer(output_frames_before_processing); + if (output_processor) { + output_frames_before_processing = + output_processor->input_needed_for_output(output_frames_needed); + /* fill directly the input buffer of the output processor to save a copy */ + out_unprocessed = + output_processor->input_buffer(output_frames_before_processing); + } else { + output_frames_before_processing = output_frames_needed; + out_unprocessed = out_buffer; + } if (in_buffer) { - /* process the input, and present exactly `output_frames_needed` in the - * callback. */ - input_processor->input(in_buffer, *input_frames_count); - - size_t frames_resampled = 0; - resampled_input = input_processor->output(output_frames_before_processing, - &frames_resampled); - *input_frames_count = frames_resampled; - } else { - resampled_input = nullptr; + if (input_processor) { + /* process the input, and present exactly `output_frames_needed` in the + * callback. */ + input_processor->input(in_buffer, *input_frames_count); + size_t frames_resampled = 0; + resampled_input = input_processor->output(output_frames_before_processing, + &frames_resampled); + *input_frames_count = frames_resampled; + } else { + output_frames_before_processing = + std::min(output_frames_before_processing, *input_frames_count); + resampled_input = in_buffer; + } } got = data_callback(stream, user_ptr, resampled_input, out_unprocessed, @@ -303,15 +319,20 @@ cubeb_resampler_speex::fill_internal_duplex( } } - output_processor->written(got); - - input_processor->drop_audio_if_needed(); + if (output_processor) { + output_processor->written(got); + } - /* Process the output. If not enough frames have been returned from the - * callback, drain the processors. */ - got = output_processor->output(out_buffer, output_frames_needed); + if (input_processor) { + input_processor->drop_audio_if_needed(); + } - output_processor->drop_audio_if_needed(); + if (output_processor) { + /* Process the output. If not enough frames have been returned from the + * callback, drain the processors. */ + got = output_processor->output(out_buffer, output_frames_needed); + output_processor->drop_audio_if_needed(); + } return got; } diff --git a/src/cubeb_resampler_internal.h b/src/cubeb_resampler_internal.h index 47e8ded3..78f9e067 100644 --- a/src/cubeb_resampler_internal.h +++ b/src/cubeb_resampler_internal.h @@ -41,6 +41,8 @@ MOZ_END_STD_NAMESPACE /* This header file contains the internal C++ API of the resamplers, for * testing. */ +enum class cubeb_resampler_direction { INPUT, OUTPUT, DUPLEX }; + // When dropping audio input frames to prevent building // an input delay, this function returns the number of frames // to keep in the buffer. @@ -118,15 +120,15 @@ class passthrough_resampler : public cubeb_resampler, public processor { uint32_t sample_rate; }; -/** Bidirectional resampler, can resample an input and an output stream, or just - * an input stream or output stream. In this case a delay is inserted in the - * opposite direction to keep the streams synchronized. */ +/** Bidirectional resampler, can resample an input and output stream, or just + * one direction. */ template class cubeb_resampler_speex : public cubeb_resampler { public: cubeb_resampler_speex(InputProcessing * input_processor, OutputProcessing * output_processor, cubeb_stream * s, - cubeb_data_callback cb, void * ptr); + cubeb_data_callback cb, void * ptr, + cubeb_resampler_direction direction); virtual ~cubeb_resampler_speex(); @@ -147,18 +149,18 @@ class cubeb_resampler_speex : public cubeb_resampler { return stats; } - virtual long latency() + long input_latency() const { - if (input_processor && output_processor) { - assert(input_processor->latency() == output_processor->latency()); - return input_processor->latency(); - } else if (input_processor) { - return input_processor->latency(); - } else { - return output_processor->latency(); - } + return input_processor ? input_processor->latency() : 0; + } + + long output_latency() const + { + return output_processor ? output_processor->latency() : 0; } + virtual long latency() { return output_latency(); } + private: typedef long (cubeb_resampler_speex::*processing_callback)( T * input_buffer, long * input_frames_count, T * output_buffer, @@ -200,7 +202,7 @@ template class cubeb_resampler_speex_one_way : public processor { uint32_t target_rate, int quality) : processor(channels), resampling_ratio(static_cast(source_rate) / target_rate), - source_rate(source_rate), additional_latency(0), leftover_samples(0) + source_rate(source_rate), leftover_samples(0) { int r; speex_resampler = @@ -300,14 +302,7 @@ template class cubeb_resampler_speex_one_way : public processor { { /* The documentation of the resampler talks about "samples" here, but it * only consider a single channel here so it's the same number of frames. */ - int latency = 0; - - latency = speex_resampler_get_output_latency(speex_resampler) + - additional_latency; - - assert(latency >= 0); - - return latency; + return speex_resampler_get_output_latency(speex_resampler); } /** Returns the number of frames to pass in the input of the resampler to have @@ -403,132 +398,11 @@ template class cubeb_resampler_speex_one_way : public processor { auto_array resampling_in_buffer; /* Storage for the resampled frames, to be passed back to the caller. */ auto_array resampling_out_buffer; - /** Additional latency inserted into the pipeline for synchronisation. */ - uint32_t additional_latency; /** When `input_buffer` is called, this allows tracking the number of samples that were in the buffer. */ uint32_t leftover_samples; }; -/** This class allows delaying an audio stream by `frames` frames. */ -template class delay_line : public processor { -public: - /** Constructor - * @parameter frames the number of frames of delay. - * @parameter channels the number of channels of this delay line. - * @parameter sample_rate sample-rate of the audio going through this delay - * line */ - delay_line(uint32_t frames, uint32_t channels, uint32_t sample_rate) - : processor(channels), length(frames), leftover_samples(0), - sample_rate(sample_rate) - { - /* Fill the delay line with some silent frames to add latency. */ - delay_input_buffer.push_silence(frames * channels); - } - /** Push some frames into the delay line. - * @parameter buffer the frames to push. - * @parameter frame_count the number of frames in #buffer. */ - void input(T * buffer, uint32_t frame_count) - { - delay_input_buffer.push(buffer, frames_to_samples(frame_count)); - } - /** Pop some frames from the internal buffer, into a internal output buffer. - * @parameter frames_needed the number of frames to be returned. - * @return a buffer containing the delayed frames. The consumer should not - * hold onto the pointer. */ - T * output(uint32_t frames_needed, size_t * input_frames_used) - { - if (delay_output_buffer.capacity() < frames_to_samples(frames_needed)) { - delay_output_buffer.reserve(frames_to_samples(frames_needed)); - } - - delay_output_buffer.clear(); - delay_output_buffer.push(delay_input_buffer.data(), - frames_to_samples(frames_needed)); - delay_input_buffer.pop(nullptr, frames_to_samples(frames_needed)); - *input_frames_used = frames_needed; - - return delay_output_buffer.data(); - } - /** Get a pointer to the first writable location in the input buffer> - * @parameter frames_needed the number of frames the user needs to write - * into the buffer. - * @returns a pointer to a location in the input buffer where #frames_needed - * can be writen. */ - T * input_buffer(uint32_t frames_needed) - { - leftover_samples = delay_input_buffer.length(); - delay_input_buffer.reserve(leftover_samples + - frames_to_samples(frames_needed)); - return delay_input_buffer.data() + leftover_samples; - } - /** This method works with `input_buffer`, and allows to inform the - processor how much frames have been written in the provided buffer. */ - void written(size_t frames_written) - { - delay_input_buffer.set_length(leftover_samples + - frames_to_samples(frames_written)); - } - /** Drains the delay line, emptying the buffer. - * @parameter output_buffer the buffer in which the frames are written. - * @parameter frames_needed the maximum number of frames to write. - * @return the actual number of frames written. */ - size_t output(T * output_buffer, uint32_t frames_needed) - { - uint32_t in_len = samples_to_frames(delay_input_buffer.length()); - uint32_t out_len = frames_needed; - - uint32_t to_pop = std::min(in_len, out_len); - - delay_input_buffer.pop(output_buffer, frames_to_samples(to_pop)); - - return to_pop; - } - /** Returns the number of frames one needs to input into the delay line to - * get #frames_needed frames back. - * @parameter frames_needed the number of frames one want to write into the - * delay_line - * @returns the number of frames one will get. */ - uint32_t input_needed_for_output(int32_t frames_needed) const - { - assert(frames_needed >= 0); // Check overflow - return frames_needed; - } - /** Returns the number of frames produces for `input_frames` frames in input - */ - size_t output_for_input(uint32_t input_frames) { return input_frames; } - /** The number of frames this delay line delays the stream by. - * @returns The number of frames of delay. */ - size_t latency() { return length; } - - void drop_audio_if_needed() - { - uint32_t available = samples_to_frames(delay_input_buffer.length()); - uint32_t to_keep = min_buffered_audio_frame(sample_rate); - if (available > to_keep) { - ALOGV("Dropping %u frames", available - to_keep); - - delay_input_buffer.pop(nullptr, frames_to_samples(available - to_keep)); - } - } - - size_t input_buffer_size() const { return delay_input_buffer.length(); } - size_t output_buffer_size() const { return delay_output_buffer.length(); } - -private: - /** The length, in frames, of this delay line */ - uint32_t length; - /** When `input_buffer` is called, this allows tracking the number of - samples that where in the buffer. */ - uint32_t leftover_samples; - /** The input buffer, where the delay is applied. */ - auto_array delay_input_buffer; - /** The output buffer. This is only ever used if using the ::output with a - * single argument. */ - auto_array delay_output_buffer; - uint32_t sample_rate; -}; - /** This sits behind the C API and is more typed. */ template cubeb_resampler * @@ -542,8 +416,6 @@ cubeb_resampler_create_internal(cubeb_stream * stream, { std::unique_ptr> input_resampler = nullptr; std::unique_ptr> output_resampler = nullptr; - std::unique_ptr> input_delay = nullptr; - std::unique_ptr> output_delay = nullptr; assert((input_params || output_params) && "need at least one valid parameter pointer."); @@ -562,8 +434,6 @@ cubeb_resampler_create_internal(cubeb_stream * stream, target_rate); } - /* Determine if we need to resampler one or both directions, and create the - resamplers. */ if (output_params && (output_params->rate != target_rate)) { output_resampler.reset(new cubeb_resampler_speex_one_way( output_params->channels, target_rate, output_params->rate, @@ -582,25 +452,10 @@ cubeb_resampler_create_internal(cubeb_stream * stream, } } - /* If we resample only one direction but we have a duplex stream, insert a - * delay line with a length equal to the resampler latency of the - * other direction so that the streams are synchronized. */ - if (input_resampler && !output_resampler && input_params && output_params) { - output_delay.reset(new delay_line(input_resampler->latency(), - output_params->channels, - output_params->rate)); - if (!output_delay) { - return NULL; - } - } else if (output_resampler && !input_resampler && input_params && - output_params) { - input_delay.reset(new delay_line(output_resampler->latency(), - input_params->channels, - output_params->rate)); - if (!input_delay) { - return NULL; - } - } + auto direction = (input_params && output_params) + ? cubeb_resampler_direction::DUPLEX + : (input_params ? cubeb_resampler_direction::INPUT + : cubeb_resampler_direction::OUTPUT); if (input_resampler && output_resampler) { LOG("Resampling input (%d) and output (%d) to target rate of %dHz", @@ -608,21 +463,21 @@ cubeb_resampler_create_internal(cubeb_stream * stream, return new cubeb_resampler_speex, cubeb_resampler_speex_one_way>( input_resampler.release(), output_resampler.release(), stream, callback, - user_ptr); + user_ptr, cubeb_resampler_direction::DUPLEX); } else if (input_resampler) { - LOG("Resampling input (%d) to target and output rate of %dHz", - input_params->rate, target_rate); + LOG("Resampling input (%d) to target rate of %dHz", input_params->rate, + target_rate); return new cubeb_resampler_speex, - delay_line>(input_resampler.release(), - output_delay.release(), - stream, callback, user_ptr); + cubeb_resampler_speex_one_way>( + input_resampler.release(), nullptr, stream, callback, user_ptr, + direction); } else { - LOG("Resampling output (%dHz) to target and input rate of %dHz", - output_params->rate, target_rate); - return new cubeb_resampler_speex, + LOG("Resampling output (%dHz) to target rate of %dHz", output_params->rate, + target_rate); + return new cubeb_resampler_speex, cubeb_resampler_speex_one_way>( - input_delay.release(), output_resampler.release(), stream, callback, - user_ptr); + nullptr, output_resampler.release(), stream, callback, user_ptr, + direction); } } diff --git a/test/test_resampler.cpp b/test/test_resampler.cpp index dd60455f..0229279f 100644 --- a/test/test_resampler.cpp +++ b/test/test_resampler.cpp @@ -131,46 +131,6 @@ epsilon(float ratio) return static_cast(10 * epsilon_tweak_ratio(ratio)); } -void -test_delay_lines(uint32_t delay_frames, uint32_t channels, uint32_t chunk_ms) -{ - const size_t length_s = 2; - const size_t rate = 44100; - const size_t length_frames = rate * length_s; - delay_line delay(delay_frames, channels, rate); - auto_array input; - auto_array output; - uint32_t chunk_length = channels * chunk_ms * rate / 1000; - uint32_t output_offset = 0; - uint32_t channel = 0; - - /** Generate diracs every 100 frames, and check they are delayed. */ - input.push_silence(length_frames * channels); - for (uint32_t i = 0; i < input.length() - 1; i += 100) { - input.data()[i + channel] = 0.5; - channel = (channel + 1) % channels; - } - dump("input.raw", input.data(), input.length()); - while (input.length()) { - uint32_t to_pop = - std::min(input.length(), chunk_length * channels); - float * in = delay.input_buffer(to_pop / channels); - input.pop(in, to_pop); - delay.written(to_pop / channels); - output.push_silence(to_pop); - delay.output(output.data() + output_offset, to_pop / channels); - output_offset += to_pop; - } - - // Check the diracs have been shifted by `delay_frames` frames. - for (uint32_t i = 0; i < output.length() - delay_frames * channels + 1; - i += 100) { - ASSERT_EQ(output.data()[i + channel + delay_frames * channels], 0.5); - channel = (channel + 1) % channels; - } - - dump("output.raw", output.data(), output.length()); -} /** * This takes sine waves with a certain `channels` count, `source_rate`, and * resample them, by chunk of `chunk_duration` milliseconds, to `target_rate`. @@ -497,20 +457,6 @@ TEST(cubeb, DISABLED_resampler_duplex) } } -TEST(cubeb, resampler_delay_line) -{ - for (uint32_t channel = 1; channel <= 2; channel++) { - for (uint32_t delay_frames = 4; delay_frames <= 40; - delay_frames += chunk_increment) { - for (uint32_t chunk_size = 10; chunk_size <= 30; chunk_size++) { - fprintf(stderr, "channel: %d, delay_frames: %d, chunk_size: %d\n", - channel, delay_frames, chunk_size); - test_delay_lines(delay_frames, channel, chunk_size); - } - } - } -} - long test_output_only_noop_data_cb(cubeb_stream * /*stm*/, void * /*user_ptr*/, const void * input_buffer, void * output_buffer, @@ -1154,19 +1100,15 @@ TEST(cubeb, individual_methods) const uint32_t sample_rate = 44100; const uint32_t frames = 256; - delay_line dl(10, channels, sample_rate); - uint32_t frames_needed1 = dl.input_needed_for_output(0); - ASSERT_EQ(frames_needed1, 0u); - cubeb_resampler_speex_one_way one_way( channels, sample_rate, sample_rate, CUBEB_RESAMPLER_QUALITY_DEFAULT); float buffer[channels * frames] = {0.0}; // Add all frames in the resampler's internal buffer. one_way.input(buffer, frames); - // Ask for less than the existing frames, this would create a uint overlflow + // Ask for less than the existing frames, this would create a uint overflow // without the fix. - uint32_t frames_needed2 = one_way.input_needed_for_output(0); - ASSERT_EQ(frames_needed2, 0u); + uint32_t frames_needed = one_way.input_needed_for_output(0); + ASSERT_EQ(frames_needed, 0u); } struct sine_wave_state { From 8c49362c2fa5e8eaf55fb61c69811df678eec7d0 Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Mon, 20 Apr 2026 14:17:40 +0000 Subject: [PATCH 05/10] auto_array: delete copy/move, fix reserve, add amortized growth 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. --- src/cubeb_utils.h | 24 +++++++++++++++++------- test/test_utils.cpp | 2 +- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/cubeb_utils.h b/src/cubeb_utils.h index 851d24de..9424f66b 100644 --- a/src/cubeb_utils.h +++ b/src/cubeb_utils.h @@ -8,10 +8,15 @@ #if !defined(CUBEB_UTILS) #define CUBEB_UTILS +#ifndef NOMINMAX +#define NOMINMAX +#endif + #include "cubeb/cubeb.h" #ifdef __cplusplus +#include #include #include #include @@ -124,6 +129,11 @@ template class auto_array { ~auto_array() { delete[] data_; } + auto_array(const auto_array &) = delete; + auto_array & operator=(const auto_array &) = delete; + auto_array(auto_array &&) = delete; + auto_array & operator=(auto_array &&) = delete; + /** Get a constant pointer to the underlying data. */ T * data() const { return data_; } @@ -150,16 +160,16 @@ template class auto_array { /** Keeps the storage, but removes all the elements from the array. */ void clear() { length_ = 0; } - /** Change the storage of this auto array, copying the elements to the new - * storage. + /** Ensure the storage can hold at least `new_capacity` elements, reallocating + * if needed. Never shrinks. * @returns true in case of success * @returns false if the new capacity is not big enough to accomodate for the * elements in the array. */ bool reserve(size_t new_capacity) { - if (new_capacity < length_) { - return false; + if (new_capacity <= capacity_) { + return true; } T * new_data = new T[new_capacity]; if (data_ && length_) { @@ -180,7 +190,7 @@ template class auto_array { void push(const T * elements, size_t length) { if (length_ + length > capacity_) { - reserve(length_ + length); + reserve(std::max(length_ + length, capacity_ * 2)); } if (data_) { PodCopy(data_ + length_, elements, length); @@ -195,7 +205,7 @@ template class auto_array { void push_silence(size_t length) { if (length_ + length > capacity_) { - reserve(length + length_); + reserve(std::max(length_ + length, capacity_ * 2)); } if (data_) { PodZero(data_ + length_, length); @@ -210,7 +220,7 @@ template class auto_array { void push_front_silence(size_t length) { if (length_ + length > capacity_) { - reserve(length + length_); + reserve(std::max(length_ + length, capacity_ * 2)); } if (data_) { PodMove(data_ + length, data_, length_); diff --git a/test/test_utils.cpp b/test/test_utils.cpp index a6227d4d..0b8a6f5d 100644 --- a/test/test_utils.cpp +++ b/test/test_utils.cpp @@ -19,7 +19,7 @@ TEST(cubeb, auto_array) array.push(a, 10); - ASSERT_TRUE(!array.reserve(9)); + ASSERT_TRUE(array.reserve(9)); for (uint32_t i = 0; i < 10; i++) { ASSERT_EQ(array.data()[i], i); From 816ab1548ee103129cba4e2163d9ef0c934124bf Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Mon, 20 Apr 2026 14:18:23 +0000 Subject: [PATCH 06/10] Remove dead null checks after new in cubeb_resampler_create_internal --- src/cubeb_resampler_internal.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/cubeb_resampler_internal.h b/src/cubeb_resampler_internal.h index 78f9e067..968e5e20 100644 --- a/src/cubeb_resampler_internal.h +++ b/src/cubeb_resampler_internal.h @@ -438,18 +438,12 @@ cubeb_resampler_create_internal(cubeb_stream * stream, output_resampler.reset(new cubeb_resampler_speex_one_way( output_params->channels, target_rate, output_params->rate, to_speex_quality(quality))); - if (!output_resampler) { - return NULL; - } } if (input_params && (input_params->rate != target_rate)) { input_resampler.reset(new cubeb_resampler_speex_one_way( input_params->channels, input_params->rate, target_rate, to_speex_quality(quality))); - if (!input_resampler) { - return NULL; - } } auto direction = (input_params && output_params) From f3c23e40eb13763b9c7bab1dc21618f74e6d938b Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Thu, 16 Apr 2026 15:47:23 +0000 Subject: [PATCH 07/10] Check speex resampler return value in release builds 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. --- src/cubeb_resampler_internal.h | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/cubeb_resampler_internal.h b/src/cubeb_resampler_internal.h index 968e5e20..c191d9e6 100644 --- a/src/cubeb_resampler_internal.h +++ b/src/cubeb_resampler_internal.h @@ -365,27 +365,25 @@ template class cubeb_resampler_speex_one_way : public processor { void speex_resample(float * input_buffer, uint32_t * input_frame_count, float * output_buffer, uint32_t * output_frame_count) { -#ifndef NDEBUG - int rv; - rv = -#endif - speex_resampler_process_interleaved_float( - speex_resampler, input_buffer, input_frame_count, output_buffer, - output_frame_count); + int rv = speex_resampler_process_interleaved_float( + speex_resampler, input_buffer, input_frame_count, output_buffer, + output_frame_count); assert(rv == RESAMPLER_ERR_SUCCESS); + if (rv != RESAMPLER_ERR_SUCCESS) { + ALOG("speex_resampler_process_interleaved_float error: %d", rv); + } } void speex_resample(short * input_buffer, uint32_t * input_frame_count, short * output_buffer, uint32_t * output_frame_count) { -#ifndef NDEBUG - int rv; - rv = -#endif - speex_resampler_process_interleaved_int( - speex_resampler, input_buffer, input_frame_count, output_buffer, - output_frame_count); + int rv = speex_resampler_process_interleaved_int( + speex_resampler, input_buffer, input_frame_count, output_buffer, + output_frame_count); assert(rv == RESAMPLER_ERR_SUCCESS); + if (rv != RESAMPLER_ERR_SUCCESS) { + ALOG("speex_resampler_process_interleaved_int error: %d", rv); + } } /** The state for the speex resampler used internaly. */ From 79feeed3d41b45a39c274f854d94130f887648c6 Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Thu, 16 Apr 2026 15:47:54 +0000 Subject: [PATCH 08/10] Test auto_array reserve stability and amortized growth Verify that reserve at or below current capacity never reallocates (pointer stability), and that push growth at least doubles capacity to amortize allocations. --- test/test_utils.cpp | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/test_utils.cpp b/test/test_utils.cpp index 0b8a6f5d..1a2c8e67 100644 --- a/test/test_utils.cpp +++ b/test/test_utils.cpp @@ -68,3 +68,44 @@ TEST(cubeb, auto_array) ASSERT_EQ(array.length(), 15u); ASSERT_EQ(array.capacity(), 20u); } + +TEST(cubeb, auto_array_reserve_no_realloc) +{ + auto_array array(128); + float * ptr = array.data(); + + // reserve at or below current capacity must not reallocate + for (size_t i = 0; i <= 128; i++) { + ASSERT_TRUE(array.reserve(i)); + ASSERT_EQ(array.data(), ptr); + ASSERT_EQ(array.capacity(), 128u); + } + + // push within capacity must not reallocate + float buf[64] = {}; + array.push(buf, 64); + ASSERT_EQ(array.data(), ptr); + + // reserve still within capacity after push + array.reserve(100); + ASSERT_EQ(array.data(), ptr); + ASSERT_EQ(array.capacity(), 128u); +} + +TEST(cubeb, auto_array_growth_amortized) +{ + auto_array array; + float val = 0.0f; + + array.push(&val, 1); + size_t cap_after_first = array.capacity(); + ASSERT_GE(cap_after_first, 1u); + + // Force growth: push more than current capacity + float buf[128] = {}; + size_t old_cap = array.capacity(); + array.push(buf, old_cap + 1); + + // Capacity should have at least doubled + ASSERT_GE(array.capacity(), old_cap * 2); +} From 92c4c8c08512a6e62651a854da0b3c14b16256da Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Thu, 16 Apr 2026 15:48:44 +0000 Subject: [PATCH 09/10] Test that input_needed_for_output always provides enough input 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. --- test/test_resampler.cpp | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/test_resampler.cpp b/test/test_resampler.cpp index 0229279f..42454ce0 100644 --- a/test/test_resampler.cpp +++ b/test/test_resampler.cpp @@ -1111,6 +1111,42 @@ TEST(cubeb, individual_methods) ASSERT_EQ(frames_needed, 0u); } +TEST(cubeb, input_needed_for_output_sufficiency) +{ + // For various rate pairs, verify that feeding exactly + // input_needed_for_output(N) frames always produces at least N output frames. + const uint32_t test_rates[] = {8000, 16000, 32000, 44100, 48000, 96000}; + const uint32_t test_blocks[] = {128, 256, 512, 1024}; + const int iterations = 200; + + for (uint32_t source_rate : test_rates) { + for (uint32_t target_rate : test_rates) { + if (source_rate == target_rate) { + continue; + } + for (uint32_t block_size : test_blocks) { + cubeb_resampler_speex_one_way resampler( + 1, source_rate, target_rate, + to_speex_quality(CUBEB_RESAMPLER_QUALITY_DEFAULT)); + + std::vector input_buf(block_size * 16); + std::vector output_buf(block_size); + + for (int i = 0; i < iterations; i++) { + uint32_t needed = resampler.input_needed_for_output(block_size); + ASSERT_LE(needed, input_buf.size()); + resampler.input(input_buf.data(), needed); + size_t got = resampler.output(output_buf.data(), block_size); + ASSERT_EQ(got, block_size) + << "source=" << source_rate << " target=" << target_rate + << " block=" << block_size << " iter=" << i + << " needed=" << needed; + } + } + } + } +} + struct sine_wave_state { float frequency; int sample_rate; From d66522c3af487ff9726f36b87f8b8fca2625f202 Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Thu, 16 Apr 2026 15:49:57 +0000 Subject: [PATCH 10/10] Fail the test on unbounded buffer growth in resampler_typical_uses 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. --- test/test_resampler.cpp | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/test/test_resampler.cpp b/test/test_resampler.cpp index 42454ce0..1163617b 100644 --- a/test/test_resampler.cpp +++ b/test/test_resampler.cpp @@ -1268,26 +1268,19 @@ struct monotonic_state { } ~monotonic_state() { - float ratio = - static_cast(source_rate) / static_cast(target_rate); - // Only report if there has been a meaningful increase in buffering. Do + // Only flag if there has been a meaningful increase in buffering. Do // not warn if the buffering was constant and small. if (monotonic && max_value && max_value != max_step) { - printf("%s is monotonically increasing, max: %zu, max_step: %zu, " - "in: %dHz, out: " - "%dHz, block_size: %d, ratio: %lf\n", - what, max_value, max_step, source_rate, target_rate, block_size, - ratio); - } - // Arbitrary limit: if more than this number of frames has been buffered, - // print a message. - constexpr int BUFFER_SIZE_THRESHOLD = 20; - if (max_value > BUFFER_SIZE_THRESHOLD) { - printf("%s, unexpected large max buffering value, max: %zu, max_step: " - "%zu, in: %dHz, out: %dHz, block_size: %d, ratio: %lf\n", - what, max_value, max_step, source_rate, target_rate, block_size, - ratio); + ADD_FAILURE() << what + << " is monotonically increasing, max: " << max_value + << ", max_step: " << max_step << ", in: " << source_rate + << "Hz, out: " << target_rate + << "Hz, block_size: " << block_size; } + constexpr size_t BUFFER_SIZE_THRESHOLD = 20; + EXPECT_LE(max_value, BUFFER_SIZE_THRESHOLD) + << what << ", in: " << source_rate << "Hz, out: " << target_rate + << "Hz, block_size: " << block_size << ", max_step: " << max_step; } void set_new_value(size_t new_value) {