From 99589c65558675187b318d355094bf73abe3bacf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lionel=20Koenig=20G=C3=A9las?= Date: Wed, 6 Mar 2024 16:42:14 +0100 Subject: [PATCH 1/3] Add test to catch out of bound array access. This is to repro a bug reported in #208 --- tests/CMakeLists.txt | 4 + tests/vari_streaming_test.c | 160 ++++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+) create mode 100644 tests/vari_streaming_test.c diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index eefa60b..491a0d8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -21,6 +21,10 @@ add_executable(misc_test misc_test.c util.c util.h) target_link_libraries(misc_test PRIVATE samplerate) add_test(NAME misc_test COMMAND misc_test WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/src) +add_executable(vari_streaming_test vari_streaming_test.c util.c util.h) +target_link_libraries(vari_streaming_test PRIVATE samplerate) +add_test(NAME vari_streaming_test COMMAND vari_streaming_test WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/src) + add_executable(termination_test termination_test.c util.c util.h) target_link_libraries(termination_test PRIVATE samplerate) add_test(NAME termination_test COMMAND termination_test WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/src) diff --git a/tests/vari_streaming_test.c b/tests/vari_streaming_test.c new file mode 100644 index 0000000..764a942 --- /dev/null +++ b/tests/vari_streaming_test.c @@ -0,0 +1,160 @@ +/* +** Copyright (c) 2002-2016, Erik de Castro Lopo +** All rights reserved. +** +** This code is released under 2-clause BSD license. Please see the +** file at : https://github.com/libsndfile/libsamplerate/blob/master/COPYING +*/ + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include +#include +#include +#include +#include +#include + +#include "util.h" + +#define BUFFER_LEN (1 << 15) +#define INPUT_BUFFER_LEN (BUFFER_LEN) +#define OUTPUT_BUFFER_LEN (BUFFER_LEN) +#define BLOCK_LEN 192 + +static void vari_stream_test(int converter, double ratio); + +int main(void) { + static double src_ratios[] = {0.3, 0.9, 1.1, 3.0}; + + int k; + + puts("\n Zero Order Hold interpolator:"); + for (k = 0; k < ARRAY_LEN(src_ratios); k++) + vari_stream_test(SRC_ZERO_ORDER_HOLD, src_ratios[k]); + + puts("\n Linear interpolator:"); + for (k = 0; k < ARRAY_LEN(src_ratios); k++) + vari_stream_test(SRC_LINEAR, src_ratios[k]); + + puts("\n Sinc interpolator:"); + for (k = 0; k < ARRAY_LEN(src_ratios); k++) + vari_stream_test(SRC_SINC_FASTEST, src_ratios[k]); + + puts(""); + + return 0; +} /* main */ + +struct generate_waveform_state_s { + float phase; + float phase_increment; +}; + +static void generate_waveform(struct generate_waveform_state_s* state, float* output, size_t len) { + for(size_t n = 0; n < len; ++n) { + output[n] = state->phase; + state->phase += state->phase_increment; + if (state->phase > 1.0 || state->phase < -1.0) + state->phase = state->phase > 0 ? -1.0 : 1.0; + } +} + +static void vari_stream_test(int converter, double src_ratio) { + float input[INPUT_BUFFER_LEN], output[OUTPUT_BUFFER_LEN]; + + SRC_STATE *src_state; + + int error, terminate; + + printf("\tvari_streaming_test (SRC ratio = %6.4f) ........... ", src_ratio); + fflush(stdout); + + /* Perform sample rate conversion. */ + if ((src_state = src_new(converter, 1, &error)) == NULL) { + printf("\n\nLine %d : src_new() failed : %s\n\n", __LINE__, + src_strerror(error)); + exit(1); + }; + + struct generate_waveform_state_s generator = { + .phase = 0.0, + .phase_increment = 2 / 48000, // 1Hz at 48000kHz samplerate. + }; + + double resampled_time = 0; + const size_t input_period_sizes[] = { + BLOCK_LEN, + BLOCK_LEN, + BLOCK_LEN, + 1, + BLOCK_LEN, + }; + + int num_input_frame_used = 0; + int num_frames_outputed = 0; + int total_num_input_frames = 0; + + int current_num_frames = 0; + for (size_t n = 0; n < ARRAY_LEN(input_period_sizes); ++n) { + generate_waveform(&generator, input, input_period_sizes[n]); + total_num_input_frames += input_period_sizes[n]; + current_num_frames += input_period_sizes[n]; + SRC_DATA src_data = { + .data_in = input, + .data_out = output, + .input_frames = input_period_sizes[n], + .output_frames = OUTPUT_BUFFER_LEN, + .input_frames_used = 0, + .output_frames_gen = 0, + .end_of_input = n == ARRAY_LEN(input_period_sizes) - 1, + .src_ratio = src_ratio, + }; + // printf("eoi: %d\n", src_data.end_of_input); + if ((error = src_process(src_state, &src_data))) { + printf("\n\nLine %d : %s\n\n", __LINE__, src_strerror(error)); + + printf("src_data.input_frames : %ld\n", src_data.input_frames); + printf("src_data.output_frames : %ld\n", src_data.output_frames); + + exit(1); + }; + + + num_input_frame_used += src_data.input_frames_used; + num_frames_outputed += src_data.output_frames_gen; + + memmove( + input, + input + src_data.input_frames_used, + (INPUT_BUFFER_LEN - src_data.input_frames_used) * sizeof(input[0]) + ); + current_num_frames -= src_data.input_frames_used; + if (src_data.end_of_input) break; + }; + + src_state = src_delete(src_state); + + terminate = (int)ceil((src_ratio >= 1.0) ? src_ratio : 1.0 / src_ratio); + + if (fabs(num_frames_outputed - src_ratio * total_num_input_frames) > 2 * terminate) { + printf("\n\nLine %d : bad output data length %d should be %d.\n", __LINE__, + num_frames_outputed, (int)floor(src_ratio * total_num_input_frames)); + printf("\tsrc_ratio : %.4f\n", src_ratio); + printf("\tinput_len : %d\n\toutput_len : %d\n\n", total_num_input_frames, num_frames_outputed); + exit(1); + }; + + if (num_input_frame_used != total_num_input_frames) { + printf("\n\nLine %d : unused %d input frames.\n", __LINE__, current_num_frames); + printf("\tinput_len : %d\n", total_num_input_frames); + printf("\tinput_frames_used : %d\n\n", num_input_frame_used); + exit(1); + }; + + puts("ok"); + + return; +} /* stream_test */ From e7c6d89854844f86986548d57275d82bbbe72512 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lionel=20Koenig=20G=C3=A9las?= Date: Wed, 6 Mar 2024 16:41:36 +0100 Subject: [PATCH 2/3] Add streaming_test into the test suits. --- tests/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 491a0d8..1e4aef8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -21,6 +21,10 @@ add_executable(misc_test misc_test.c util.c util.h) target_link_libraries(misc_test PRIVATE samplerate) add_test(NAME misc_test COMMAND misc_test WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/src) +add_executable(streaming_test streaming_test.c util.c util.h) +target_link_libraries(streaming_test PRIVATE samplerate) +add_test(NAME streaming_test COMMAND streaming_test WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/src) + add_executable(vari_streaming_test vari_streaming_test.c util.c util.h) target_link_libraries(vari_streaming_test PRIVATE samplerate) add_test(NAME vari_streaming_test COMMAND vari_streaming_test WORKING_DIRECTORY ${PROJECT_BINARY_DIR}/src) From 8e62284632d3fcfa2a3e18c07f67509096d8b216 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lionel=20Koenig=20G=C3=A9las?= Date: Thu, 7 Mar 2024 08:54:55 +0100 Subject: [PATCH 3/3] Fix out of bound access in src_linear MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #208 about out of bount access in linear resampler. Also migrates to count the sumber of frames instead of the number of samples and remove some redundant member of the private state. Signed-off-by: Lionel Koenig Gélas --- src/src_linear.c | 87 ++++++++++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 47 deletions(-) diff --git a/src/src_linear.c b/src/src_linear.c index 43a0fd4..8d34625 100644 --- a/src/src_linear.c +++ b/src/src_linear.c @@ -32,9 +32,7 @@ static void linear_close (SRC_STATE *state) ; typedef struct { int linear_magic_marker ; - bool dirty ; - long in_count, in_used ; - long out_count, out_gen ; + bool initialized ; float *last_value ; } LINEAR_DATA ; @@ -54,7 +52,6 @@ static SRC_ERROR linear_vari_process (SRC_STATE *state, SRC_DATA *data) { LINEAR_DATA *priv ; double src_ratio, input_index, rem ; - int ch ; if (data->input_frames <= 0) return SRC_ERR_NO_ERROR ; @@ -64,16 +61,14 @@ linear_vari_process (SRC_STATE *state, SRC_DATA *data) priv = (LINEAR_DATA*) state->private_data ; - if (!priv->dirty) + if (!priv->initialized) { /* If we have just been reset, set the last_value data. */ - for (ch = 0 ; ch < state->channels ; ch++) + for (int ch = 0 ; ch < state->channels ; ch++) priv->last_value [ch] = data->data_in [ch] ; - priv->dirty = true ; + priv->initialized = true ; } ; - priv->in_count = data->input_frames * state->channels ; - priv->out_count = data->output_frames * state->channels ; - priv->in_used = priv->out_gen = 0 ; + data->input_frames_used = data->output_frames_gen = 0 ; src_ratio = state->last_ratio ; @@ -83,72 +78,70 @@ linear_vari_process (SRC_STATE *state, SRC_DATA *data) input_index = state->last_position ; /* Calculate samples before first sample in input array. */ - while (input_index < 1.0 && priv->out_gen < priv->out_count) + float* current_out = data->data_out; + while (input_index < 1.0 && data->output_frames_gen < data->output_frames) { - if (priv->in_used + state->channels * (1.0 + input_index) >= priv->in_count) - break ; + if (data->output_frames > 0 && fabs (state->last_ratio - data->src_ratio) > SRC_MIN_RATIO_DIFF) + src_ratio = state->last_ratio + data->output_frames_gen * (data->src_ratio - state->last_ratio) / data->output_frames ; - if (priv->out_count > 0 && fabs (state->last_ratio - data->src_ratio) > SRC_MIN_RATIO_DIFF) - src_ratio = state->last_ratio + priv->out_gen * (data->src_ratio - state->last_ratio) / priv->out_count ; - - for (ch = 0 ; ch < state->channels ; ch++) - { data->data_out [priv->out_gen] = (float) (priv->last_value [ch] + input_index * + for (int ch = 0 ; ch < state->channels ; ch++) + { *current_out++ = (float) (priv->last_value [ch] + input_index * ((double) data->data_in [ch] - priv->last_value [ch])) ; - priv->out_gen ++ ; } ; + data->output_frames_gen ++ ; /* Figure out the next index. */ input_index += 1.0 / src_ratio ; } ; rem = fmod_one (input_index) ; - priv->in_used += state->channels * psf_lrint (input_index - rem) ; + data->input_frames_used += psf_lrint (input_index - rem) ; input_index = rem ; /* Main processing loop. */ - while (priv->out_gen < priv->out_count && priv->in_used + state->channels * input_index < priv->in_count) - { - if (priv->out_count > 0 && fabs (state->last_ratio - data->src_ratio) > SRC_MIN_RATIO_DIFF) - src_ratio = state->last_ratio + priv->out_gen * (data->src_ratio - state->last_ratio) / priv->out_count ; - #if SRC_DEBUG - if (priv->in_used < state->channels && input_index < 1.0) - { printf ("Whoops!!!! in_used : %ld channels : %d input_index : %f\n", priv->in_used, state->channels, input_index) ; - exit (1) ; - } ; + assert(data->output_frames_gen >= data->output_frames || data->input_frames_used > 0); #endif + while (data->output_frames_gen < data->output_frames && data->input_frames_used + input_index < data->input_frames) + { + if (data->output_frames > 0 && fabs (state->last_ratio - data->src_ratio) > SRC_MIN_RATIO_DIFF) + src_ratio = state->last_ratio + data->output_frames_gen * (data->src_ratio - state->last_ratio) / data->output_frames ; - for (ch = 0 ; ch < state->channels ; ch++) - { data->data_out [priv->out_gen] = (float) (data->data_in [priv->in_used - state->channels + ch] + input_index * - ((double) data->data_in [priv->in_used + ch] - data->data_in [priv->in_used - state->channels + ch])) ; - priv->out_gen ++ ; - } ; + const float* current_in = data->data_in + data->input_frames_used * state->channels; + const float* prev_in = current_in - state->channels; + for (int ch = 0 ; ch < state->channels ; ch++) + { + *current_out++ = (float) (prev_in[ch] + input_index * + ((double) current_in[ch] - prev_in[ch])) ; + } + data->output_frames_gen ++ ; /* Figure out the next index. */ input_index += 1.0 / src_ratio ; rem = fmod_one (input_index) ; - priv->in_used += state->channels * psf_lrint (input_index - rem) ; + const int num_frame_used = psf_lrint (input_index - rem); + data->input_frames_used += num_frame_used ; input_index = rem ; - } ; + } - if (priv->in_used > priv->in_count) - { input_index += (priv->in_used - priv->in_count) / state->channels ; - priv->in_used = priv->in_count ; - } ; + if (data->input_frames_used > data->input_frames) + { + input_index += (data->input_frames_used - data->input_frames) ; + data->input_frames_used = data->input_frames ; + } state->last_position = input_index ; - if (priv->in_used > 0) - for (ch = 0 ; ch < state->channels ; ch++) - priv->last_value [ch] = data->data_in [priv->in_used - state->channels + ch] ; + if (data->input_frames_used > 0) { + const float *last_value = data->data_in + (data->input_frames_used - 1) * state->channels; + for (int ch = 0 ; ch < state->channels ; ch++) + priv->last_value [ch] = last_value[ch]; + } /* Save current ratio rather then target ratio. */ state->last_ratio = src_ratio ; - data->input_frames_used = priv->in_used / state->channels ; - data->output_frames_gen = priv->out_gen / state->channels ; - return SRC_ERR_NO_ERROR ; } /* linear_vari_process */ @@ -237,7 +230,7 @@ linear_reset (SRC_STATE *state) if (priv == NULL) return ; - priv->dirty = false ; + priv->initialized = false ; memset (priv->last_value, 0, sizeof (priv->last_value [0]) * state->channels) ; return ;