From a0653ed1e7c63aaa5f078b81caf1e474e7e47033 Mon Sep 17 00:00:00 2001 From: Jean-Marc Valin <jean-marc.valin@usherbrooke.ca> Date: Tue, 5 Jul 2011 13:18:59 -0400 Subject: [PATCH] Fixes a bunch of valgrind errors when decoding random junk --- COPYING | 2 +- libcelt/celt.c | 4 +++- silk/silk_PLC.c | 3 +++ src/opus_decoder.c | 49 ++++++++++++++++++++++++++++++++-------------- src/test_opus.c | 37 ++++++++++++++++++++++------------ 5 files changed, 66 insertions(+), 29 deletions(-) diff --git a/COPYING b/COPYING index 7112fd575..421be6c34 100644 --- a/COPYING +++ b/COPYING @@ -1,4 +1,4 @@ -Copyright 2001-2010 Xiph.Org, Skype, Octasic, +Copyright 2001-2011 Xiph.Org, Skype, Octasic, Jean-Marc Valin, Timothy B. Terriberry, CSIRO, and other contributors diff --git a/libcelt/celt.c b/libcelt/celt.c index c2cf970de..7419e174f 100644 --- a/libcelt/celt.c +++ b/libcelt/celt.c @@ -2411,7 +2411,9 @@ int celt_decode_with_ec_float(CELTDecoder * restrict st, const unsigned char *da total_bits = len*8; tell = ec_tell(dec); - if (tell==1) + if (tell >= total_bits) + silence = 1; + else if (tell==1) silence = ec_dec_bit_logp(dec, 15); else silence = 0; diff --git a/silk/silk_PLC.c b/silk/silk_PLC.c index 594eb76a7..d8a9db3fe 100644 --- a/silk/silk_PLC.c +++ b/silk/silk_PLC.c @@ -172,6 +172,9 @@ void silk_PLC_conceal( /* Find random noise component */ /* Scale previous excitation signal */ exc_buf_ptr = exc_buf; + /* FIXME: JMV: Is this the right fix? */ + for (i=0;i<MAX_FRAME_LENGTH;i++) + exc_buf[i] = 0; for( k = ( psDec->nb_subfr >> 1 ); k < psDec->nb_subfr; k++ ) { for( i = 0; i < psDec->subfr_length; i++ ) { exc_buf_ptr[ i ] = ( SKP_int16 )SKP_RSHIFT( diff --git a/src/opus_decoder.c b/src/opus_decoder.c index 75fb9325a..079fca7b8 100644 --- a/src/opus_decoder.c +++ b/src/opus_decoder.c @@ -153,7 +153,8 @@ static int opus_decode_frame(OpusDecoder *st, const unsigned char *data, silk_DecControlStruct DecControl; SKP_int32 silk_frame_size; short pcm_celt[960*2]; - short pcm_transition[960*2]; + short pcm_transition[480*2]; + int audiosize; int mode; int transition=0; @@ -163,28 +164,33 @@ static int opus_decode_frame(OpusDecoder *st, const unsigned char *data, int celt_to_silk=0; short redundant_audio[240*2]; int c; - int F2_5, F5, F10; + int F2_5, F5, F10, F20; const celt_word16 *window; silk_dec = (char*)st+st->silk_dec_offset; celt_dec = (CELTDecoder*)((char*)st+st->celt_dec_offset); - F10 = st->Fs/100; + F20 = st->Fs/50; + F10 = F20>>1; F5 = F10>>1; F2_5 = F5>>1; /* Payloads of 1 (2 including ToC) or 0 trigger the PLC/DTX */ if (len<=1) + { data = NULL; - - audiosize = st->frame_size; + /* In that case, don't conceal more than what the ToC says */ + frame_size = IMIN(frame_size, st->frame_size); + } if (data != NULL) { + audiosize = st->frame_size; mode = st->mode; ec_dec_init(&dec,(unsigned char*)data,len); } else { + audiosize = frame_size; if (st->prev_mode == 0) { /* If we haven't got any packet yet, all we can do is return zeros */ - for (i=0;i<frame_size;i++) + for (i=0;i<audiosize;i++) pcm[i] = 0; return audiosize; } else { @@ -198,7 +204,7 @@ static int opus_decode_frame(OpusDecoder *st, const unsigned char *data, { transition = 1; if (mode == MODE_CELT_ONLY) - opus_decode_frame(st, NULL, 0, pcm_transition, IMAX(F10, audiosize), 0); + opus_decode_frame(st, NULL, 0, pcm_transition, IMIN(F10, audiosize), 0); } if (audiosize > frame_size) { @@ -245,8 +251,13 @@ static int opus_decode_frame(OpusDecoder *st, const unsigned char *data, silk_ret = silk_Decode( silk_dec, &DecControl, lost_flag, first_frame, &dec, pcm_ptr, &silk_frame_size ); if( silk_ret ) { - fprintf (stderr, "SILK decode error\n"); - /* Handle error */ + if (lost_flag) { + /* PLC failure should not be fatal */ + silk_frame_size = frame_size; + for (i=0;i<frame_size*st->channels;i++) + pcm_ptr[i] = 0; + } else + return OPUS_CORRUPTED_DATA; } pcm_ptr += silk_frame_size * st->channels; decoded_samples += silk_frame_size; @@ -266,11 +277,18 @@ static int opus_decode_frame(OpusDecoder *st, const unsigned char *data, celt_to_silk = ec_dec_bit_logp(&dec, 1); if (mode == MODE_HYBRID) redundancy_bytes = 2 + ec_dec_uint(&dec, 256); - else + else { redundancy_bytes = len - ((ec_tell(&dec)+7)>>3); + /* Can only happen on an invalid packet */ + if (redundancy_bytes<0) + { + redundancy_bytes = 0; + redundancy = 0; + } + } len -= redundancy_bytes; if (len<0) - return CELT_CORRUPTED_DATA; + return OPUS_CORRUPTED_DATA; /* Shrink decoder because of raw bits */ dec.storage -= redundancy_bytes; } @@ -305,7 +323,7 @@ static int opus_decode_frame(OpusDecoder *st, const unsigned char *data, transition = 0; if (transition && mode != MODE_CELT_ONLY) - opus_decode_frame(st, NULL, 0, pcm_transition, IMAX(F10, audiosize), 0); + opus_decode_frame(st, NULL, 0, pcm_transition, IMIN(F10, audiosize), 0); /* 5 ms redundant frame for CELT->SILK*/ if (redundancy && celt_to_silk) @@ -322,9 +340,10 @@ static int opus_decode_frame(OpusDecoder *st, const unsigned char *data, if (mode != MODE_SILK_ONLY) { + int celt_frame_size = IMIN(F20, frame_size); /* Decode CELT */ - celt_ret = celt_decode_with_ec(celt_dec, decode_fec?NULL:data, len, pcm_celt, frame_size, &dec); - for (i=0;i<frame_size*st->channels;i++) + celt_ret = celt_decode_with_ec(celt_dec, decode_fec?NULL:data, len, pcm_celt, celt_frame_size, &dec); + for (i=0;i<celt_frame_size*st->channels;i++) pcm[i] = SAT16(pcm[i] + (int)pcm_celt[i]); } @@ -404,7 +423,7 @@ int opus_decode(OpusDecoder *st, const unsigned char *data, if (len==0 || data==NULL) return opus_decode_frame(st, NULL, 0, pcm, frame_size, 0); else if (len<0) - return CELT_BAD_ARG; + return OPUS_BAD_ARG; st->mode = opus_packet_get_mode(data); st->bandwidth = opus_packet_get_bandwidth(data); st->frame_size = opus_packet_get_samples_per_frame(data, st->Fs); diff --git a/src/test_opus.c b/src/test_opus.c index fa52d8999..149d427f6 100644 --- a/src/test_opus.c +++ b/src/test_opus.c @@ -158,6 +158,7 @@ int main(int argc, char *argv[]) forcemono = 0; use_dtx = 0; packet_loss_perc = 0; + int max_frame_size = 960*3; args = 5; while( args < argc - 2 ) { @@ -316,7 +317,7 @@ int main(int argc, char *argv[]) fprintf(stderr, "Encoding %d Hz input at %.3f kb/s in %s mode with %d-sample frames.\n", sampling_rate, bitrate_bps*0.001, bandwidth_string, frame_size); in = (short*)malloc(frame_size*channels*sizeof(short)); - out = (short*)malloc(frame_size*channels*sizeof(short)); + out = (short*)malloc(max_frame_size*channels*sizeof(short)); data[0] = (unsigned char*)calloc(max_payload_bytes,sizeof(char)); if( use_inbandfec ) { data[1] = (unsigned char*)calloc(max_payload_bytes,sizeof(char)); @@ -328,6 +329,11 @@ int main(int argc, char *argv[]) unsigned char ch[4]; err = fread(ch, 1, 4, fin); len[toggle] = char_to_int(ch); + if (len[toggle]>max_payload_bytes || len[toggle]<0) + { + fprintf(stderr, "Invalid payload length\n"); + break; + } err = fread(ch, 1, 4, fin); enc_final_range[toggle] = char_to_int(ch); err = fread(data[toggle], 1, len[toggle], fin); @@ -365,28 +371,32 @@ int main(int argc, char *argv[]) fwrite(int_field, 1, 4, fout); fwrite(data[toggle], 1, len[toggle], fout); } else { + int output_samples; lost = rand()%100 < packet_loss_perc || len[toggle]==0; if( count >= use_inbandfec ) { /* delay by one packet when using in-band FEC */ if( use_inbandfec ) { if( lost_prev ) { /* attempt to decode with in-band FEC from next packet */ - opus_decode(dec, lost ? NULL : data[toggle], len[toggle], out, frame_size, 1); + output_samples = opus_decode(dec, lost ? NULL : data[toggle], len[toggle], out, max_frame_size, 1); } else { /* regular decode */ - opus_decode(dec, data[1-toggle], len[1-toggle], out, frame_size, 0); + output_samples = opus_decode(dec, data[1-toggle], len[1-toggle], out, max_frame_size, 0); } } else { - opus_decode(dec, lost ? NULL : data[toggle], len[toggle], out, frame_size, 0); + output_samples = opus_decode(dec, lost ? NULL : data[toggle], len[toggle], out, max_frame_size, 0); } - write_samples = frame_size-skip; - tot_written += write_samples*channels; - if (tot_written > tot_read) + if (output_samples>0) { - write_samples -= (tot_written-tot_read)/channels; + write_samples = output_samples-skip; + tot_written += write_samples*channels; + if (tot_written > tot_read) + { + write_samples -= (tot_written-tot_read)/channels; + } + fwrite(out+skip, sizeof(short), write_samples*channels, fout); + skip = 0; } - fwrite(out+skip, sizeof(short), write_samples*channels, fout); - skip = 0; } } @@ -405,8 +415,11 @@ int main(int argc, char *argv[]) bits += len[toggle]*8; if( count >= use_inbandfec ) { nrg = 0.0; - for ( k = 0; k < frame_size * channels; k++ ) { - nrg += in[ k ] * (double)in[ k ]; + if (!decode_only) + { + for ( k = 0; k < frame_size * channels; k++ ) { + nrg += in[ k ] * (double)in[ k ]; + } } if ( ( nrg / ( frame_size * channels ) ) > 1e5 ) { bits_act += len[toggle]*8; -- GitLab