Skip to content
Snippets Groups Projects
Commit dc58579c authored by Timothy B. Terriberry's avatar Timothy B. Terriberry
Browse files

Fix several memory errors in the SILK resampler.

1) The memcpy's were using sizeof(opus_int32), but the type of the
    local buffer was opus_int16.
2) Because the size was wrong, this potentially allowed the source
    and destination regions of the memcpy overlap.
   I _believe_ that nSamplesIn is at least fs_in_khZ, which is at
    least 8.
   Since RESAMPLER_ORDER_FIR_12 is only 8, I don't think that's a
    problem once you fix the type size.
3) The size of the buffer used RESAMPLER_MAX_BATCH_SIZE_IN, but the
    data stored in it was actually _twice_ the input batch size
    (nSamplesIn<<1).

Because this never blew up in testing, I suspect that in practice
 the batch sizes are reasonable enough that none of these things
 was ever a problem, but proving that seems non-obvious.

This patch just converts the whole thing to use CELT's vararrays.
This fixes the buffer size problems (since we allocate a buffer
 with the actual size we use) and gets these large buffers off the
 stack on devices using the pseudo-stack.
It also fixes the memcpy problems by changing the sizeof to
 opus_int16.
It turns out sFIR, which saved state between calls, was being used
 elsewhere as opus_int32, so this converts it to a union to make
 this sharing explicit.
parent c41a8168
No related branches found
No related tags found
No related merge requests found
......@@ -31,6 +31,7 @@ POSSIBILITY OF SUCH DAMAGE.
#include "SigProc_FIX.h"
#include "resampler_private.h"
#include "stack_alloc.h"
static inline opus_int16 *silk_resampler_private_IIR_FIR_INTERPOL(
opus_int16 *out,
......@@ -71,10 +72,13 @@ void silk_resampler_private_IIR_FIR(
silk_resampler_state_struct *S = (silk_resampler_state_struct *)SS;
opus_int32 nSamplesIn;
opus_int32 max_index_Q16, index_increment_Q16;
opus_int16 buf[ RESAMPLER_MAX_BATCH_SIZE_IN + RESAMPLER_ORDER_FIR_12 ];
VARDECL( opus_int16, buf );
SAVE_STACK;
ALLOC( buf, 2 * S->batchSize + RESAMPLER_ORDER_FIR_12, opus_int16 );
/* Copy buffered samples to start of buffer */
silk_memcpy( buf, S->sFIR, RESAMPLER_ORDER_FIR_12 * sizeof( opus_int32 ) );
silk_memcpy( buf, S->sFIR.i16, RESAMPLER_ORDER_FIR_12 * sizeof( opus_int16 ) );
/* Iterate over blocks of frameSizeIn input samples */
index_increment_Q16 = S->invRatio_Q16;
......@@ -91,13 +95,13 @@ void silk_resampler_private_IIR_FIR(
if( inLen > 0 ) {
/* More iterations to do; copy last part of filtered signal to beginning of buffer */
silk_memcpy( buf, &buf[ nSamplesIn << 1 ], RESAMPLER_ORDER_FIR_12 * sizeof( opus_int32 ) );
silk_memcpy( buf, &buf[ nSamplesIn << 1 ], RESAMPLER_ORDER_FIR_12 * sizeof( opus_int16 ) );
} else {
break;
}
}
/* Copy last part of filtered signal to the state for the next call */
silk_memcpy( S->sFIR, &buf[ nSamplesIn << 1 ], RESAMPLER_ORDER_FIR_12 * sizeof( opus_int32 ) );
silk_memcpy( S->sFIR.i16, &buf[ nSamplesIn << 1 ], RESAMPLER_ORDER_FIR_12 * sizeof( opus_int16 ) );
RESTORE_STACK;
}
......@@ -155,7 +155,7 @@ void silk_resampler_private_down_FIR(
const opus_int16 *FIR_Coefs;
/* Copy buffered samples to start of buffer */
silk_memcpy( buf, S->sFIR, S->FIR_Order * sizeof( opus_int32 ) );
silk_memcpy( buf, S->sFIR.i32, S->FIR_Order * sizeof( opus_int32 ) );
FIR_Coefs = &S->Coefs[ 2 ];
......@@ -185,5 +185,5 @@ void silk_resampler_private_down_FIR(
}
/* Copy last part of filtered signal to the state for the next call */
silk_memcpy( S->sFIR, &buf[ nSamplesIn ], S->FIR_Order * sizeof( opus_int32 ) );
silk_memcpy( S->sFIR.i32, &buf[ nSamplesIn ], S->FIR_Order * sizeof( opus_int32 ) );
}
......@@ -37,7 +37,10 @@ extern "C" {
typedef struct _silk_resampler_state_struct{
opus_int32 sIIR[ SILK_RESAMPLER_MAX_IIR_ORDER ]; /* this must be the first element of this struct */
opus_int32 sFIR[ SILK_RESAMPLER_MAX_FIR_ORDER ];
union{
opus_int32 i32[ SILK_RESAMPLER_MAX_FIR_ORDER ];
opus_int16 i16[ SILK_RESAMPLER_MAX_FIR_ORDER ];
} sFIR;
opus_int16 delayBuf[ 48 ];
opus_int resampler_function;
opus_int batchSize;
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment