Fixes a bunch of valgrind errors when decoding random junk
authorJean-Marc Valin <jean-marc.valin@usherbrooke.ca>
Tue, 5 Jul 2011 17:18:59 +0000 (13:18 -0400)
committerJean-Marc Valin <jean-marc.valin@usherbrooke.ca>
Tue, 5 Jul 2011 17:33:45 +0000 (13:33 -0400)
COPYING
libcelt/celt.c
silk/silk_PLC.c
src/opus_decoder.c
src/test_opus.c

diff --git a/COPYING b/COPYING
index 7112fd5..421be6c 100644 (file)
--- 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
 
index c2cf970..7419e17 100644 (file)
@@ -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;
index 594eb76..d8a9db3 100644 (file)
@@ -172,6 +172,9 @@ void silk_PLC_conceal(
     /* Find random noise component */\r
     /* Scale previous excitation signal */\r
     exc_buf_ptr = exc_buf;\r
+    /* FIXME: JMV: Is this the right fix? */\r
+    for (i=0;i<MAX_FRAME_LENGTH;i++)\r
+       exc_buf[i] = 0;\r
     for( k = ( psDec->nb_subfr >> 1 ); k < psDec->nb_subfr; k++ ) {\r
         for( i = 0; i < psDec->subfr_length; i++ ) {\r
             exc_buf_ptr[ i ] = ( SKP_int16 )SKP_RSHIFT( \r
index 75fb932..079fca7 100644 (file)
@@ -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);
index fa52d89..149d427 100644 (file)
@@ -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;