shithub: opus

Download patch

ref: de95da9bf1c98b80368cf643e3b7da45f3ca108b
parent: 6930d90c341fca347f3fdc1e10c43f3265174aed
author: Gregory Maxwell <[email protected]>
date: Sat Oct 27 09:42:48 EDT 2012 

Fix several issues with multistream argument validation.

As reported by Mark Warner opus_multistream_*_create were depending on
 the behavior of malloc(0) in order to correctly report some kinds of
 argument errors. Bad arguments could be incorrectly reported as
 allocation failures. This changes multistream to explicitly check the
 arguments like the single stream _create functions. The unit tests were
 enough to catch this on systems where malloc(0) returns NULL but didn't
 catch it on other systems because the later _init call would catch the
 bad arguments and trigger the correct error if and only if the malloc
 didn't return a null pointer.

In multistream_encoder_init failures of the internal non-multistream
 init calls were not being caught and propagated. Decode didn't have
 this problem. This propagates the errors and adds additional tests
 (the multistream encoder api is sill under tested) that would have
 detected this error.

Plus add some stronger tests for things like error==NULL for the _create
 functions that take a pointer for error output.


--- a/src/opus_multistream.c
+++ b/src/opus_multistream.c
@@ -159,7 +159,7 @@
 {
    int coupled_size;
    int mono_size;
-   int i;
+   int i, ret;
    char *ptr;
 
    if ((channels>255) || (channels<1) || (coupled_streams>streams) ||
@@ -180,12 +180,14 @@
 
    for (i=0;i<st->layout.nb_coupled_streams;i++)
    {
-      opus_encoder_init((OpusEncoder*)ptr, Fs, 2, application);
+      ret = opus_encoder_init((OpusEncoder*)ptr, Fs, 2, application);
+      if(ret!=OPUS_OK)return ret;
       ptr += align(coupled_size);
    }
    for (;i<st->layout.nb_streams;i++)
    {
-      opus_encoder_init((OpusEncoder*)ptr, Fs, 1, application);
+      ret = opus_encoder_init((OpusEncoder*)ptr, Fs, 1, application);
+      if(ret!=OPUS_OK)return ret;
       ptr += align(mono_size);
    }
    return OPUS_OK;
@@ -202,7 +204,15 @@
 )
 {
    int ret;
-   OpusMSEncoder *st = (OpusMSEncoder *)opus_alloc(opus_multistream_encoder_get_size(streams, coupled_streams));
+   OpusMSEncoder *st;
+   if ((channels>255) || (channels<1) || (coupled_streams>streams) ||
+       (coupled_streams+streams>255) || (streams<1) || (coupled_streams<0))
+   {
+      if (error)
+         *error = OPUS_BAD_ARG;
+      return NULL;
+   }
+   st = (OpusMSEncoder *)opus_alloc(opus_multistream_encoder_get_size(streams, coupled_streams));
    if (st==NULL)
    {
       if (error)
@@ -649,7 +659,15 @@
 )
 {
    int ret;
-   OpusMSDecoder *st = (OpusMSDecoder *)opus_alloc(opus_multistream_decoder_get_size(streams, coupled_streams));
+   OpusMSDecoder *st;
+   if ((channels>255) || (channels<1) || (coupled_streams>streams) ||
+       (coupled_streams+streams>255) || (streams<1) || (coupled_streams<0))
+   {
+      if (error)
+         *error = OPUS_BAD_ARG;
+      return NULL;
+   }
+   st = (OpusMSDecoder *)opus_alloc(opus_multistream_decoder_get_size(streams, coupled_streams));
    if (st==NULL)
    {
       if (error)
@@ -665,8 +683,6 @@
       st = NULL;
    }
    return st;
-
-
 }
 
 typedef void (*opus_copy_channel_out_func)(
--- a/tests/test_opus_api.c
+++ b/tests/test_opus_api.c
@@ -126,6 +126,9 @@
          dec = opus_decoder_create(fs, c, &err);
          if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
          cfgs++;
+         dec = opus_decoder_create(fs, c, 0);
+         if(dec!=NULL)test_failed();
+         cfgs++;
          dec=malloc(opus_decoder_get_size(2));
          if(dec==NULL)test_failed();
          err = opus_decoder_init(dec,fs,c);
@@ -366,6 +369,9 @@
          dec = opus_multistream_decoder_create(fs, c, 1, c-1, mapping, &err);
          if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
          cfgs++;
+         dec = opus_multistream_decoder_create(fs, c, 1, c-1, mapping, 0);
+         if(dec!=NULL)test_failed();
+         cfgs++;
          dec=malloc(opus_multistream_decoder_get_size(1,1));
          if(dec==NULL)test_failed();
          err = opus_multistream_decoder_init(dec,fs,c,1,c-1, mapping);
@@ -375,117 +381,129 @@
       }
    }
 
-   VG_UNDEF(&err,sizeof(err));
-   dec = opus_multistream_decoder_create(48000, 2, 1, 0, mapping, &err);
-   VG_CHECK(&err,sizeof(err));
-   if(err==OPUS_OK || dec!=NULL)test_failed();
-   cfgs++;
+   for(c=0;c<2;c++)
+   {
+      int *ret_err;
+      ret_err = c?0:&err;
 
-   VG_UNDEF(&err,sizeof(err));
-   mapping[0]=mapping[1]=0;
-   dec = opus_multistream_decoder_create(48000, 2, 1, 0, mapping, &err);
-   VG_CHECK(&err,sizeof(err));
-   if(err!=OPUS_OK || dec==NULL)test_failed();
-   cfgs++;
-   opus_multistream_decoder_destroy(dec);
-   cfgs++;
+      mapping[0]=0;
+      mapping[1]=1;
+      for(i=2;i<256;i++)VG_UNDEF(&mapping[i],sizeof(unsigned char));
 
-   VG_UNDEF(&err,sizeof(err));
-   dec = opus_multistream_decoder_create(48000, 1, 4, 1, mapping, &err);
-   VG_CHECK(&err,sizeof(err));
-   if(err!=OPUS_OK || dec==NULL)test_failed();
-   cfgs++;
+      VG_UNDEF(ret_err,sizeof(*ret_err));
+      dec = opus_multistream_decoder_create(48000, 2, 1, 0, mapping, ret_err);
+      if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+      if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+      cfgs++;
 
-   err = opus_multistream_decoder_init(dec,48000, 1, 0, 0, mapping);
-   if(err!=OPUS_BAD_ARG)test_failed();
-   cfgs++;
+      VG_UNDEF(ret_err,sizeof(*ret_err));
+      mapping[0]=mapping[1]=0;
+      dec = opus_multistream_decoder_create(48000, 2, 1, 0, mapping, ret_err);
+      if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+      if((ret_err && *ret_err!=OPUS_OK) || dec==NULL)test_failed();
+      cfgs++;
+      opus_multistream_decoder_destroy(dec);
+      cfgs++;
 
-   err = opus_multistream_decoder_init(dec,48000, 1, 1, -1, mapping);
-   if(err!=OPUS_BAD_ARG)test_failed();
-   cfgs++;
+      VG_UNDEF(ret_err,sizeof(*ret_err));
+      dec = opus_multistream_decoder_create(48000, 1, 4, 1, mapping, ret_err);
+      if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+      if((ret_err && *ret_err!=OPUS_OK) || dec==NULL)test_failed();
+      cfgs++;
 
-   opus_multistream_decoder_destroy(dec);
-   cfgs++;
+      err = opus_multistream_decoder_init(dec,48000, 1, 0, 0, mapping);
+      if(err!=OPUS_BAD_ARG)test_failed();
+      cfgs++;
 
-   VG_UNDEF(&err,sizeof(err));
-   dec = opus_multistream_decoder_create(48000, 2, 1, 1, mapping, &err);
-   VG_CHECK(&err,sizeof(err));
-   if(err!=OPUS_OK || dec==NULL)test_failed();
-   cfgs++;
-   opus_multistream_decoder_destroy(dec);
-   cfgs++;
+      err = opus_multistream_decoder_init(dec,48000, 1, 1, -1, mapping);
+      if(err!=OPUS_BAD_ARG)test_failed();
+      cfgs++;
 
-   VG_UNDEF(&err,sizeof(err));
-   dec = opus_multistream_decoder_create(48000, 255, 255, 1, mapping, &err);
-   VG_CHECK(&err,sizeof(err));
-   if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
-   cfgs++;
+      opus_multistream_decoder_destroy(dec);
+      cfgs++;
 
-   VG_UNDEF(&err,sizeof(err));
-   dec = opus_multistream_decoder_create(48000, -1, 1, 1, mapping, &err);
-   VG_CHECK(&err,sizeof(err));
-   if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
-   cfgs++;
+      VG_UNDEF(ret_err,sizeof(*ret_err));
+      dec = opus_multistream_decoder_create(48000, 2, 1, 1, mapping, ret_err);
+      if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+      if((ret_err && *ret_err!=OPUS_OK) || dec==NULL)test_failed();
+      cfgs++;
+      opus_multistream_decoder_destroy(dec);
+      cfgs++;
 
-   VG_UNDEF(&err,sizeof(err));
-   dec = opus_multistream_decoder_create(48000, 0, 1, 1, mapping, &err);
-   VG_CHECK(&err,sizeof(err));
-   if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
-   cfgs++;
+      VG_UNDEF(ret_err,sizeof(*ret_err));
+      dec = opus_multistream_decoder_create(48000, 255, 255, 1, mapping, ret_err);
+      if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+      if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+      cfgs++;
 
-   VG_UNDEF(&err,sizeof(err));
-   dec = opus_multistream_decoder_create(48000, 1, -1, 2, mapping, &err);
-   VG_CHECK(&err,sizeof(err));
-   if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
-   cfgs++;
+      VG_UNDEF(ret_err,sizeof(*ret_err));
+      dec = opus_multistream_decoder_create(48000, -1, 1, 1, mapping, ret_err);
+      if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+      if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+      cfgs++;
 
-   VG_UNDEF(&err,sizeof(err));
-   dec = opus_multistream_decoder_create(48000, 1, -1, -1, mapping, &err);
-   VG_CHECK(&err,sizeof(err));
-   if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
-   cfgs++;
+      VG_UNDEF(ret_err,sizeof(*ret_err));
+      dec = opus_multistream_decoder_create(48000, 0, 1, 1, mapping, ret_err);
+      if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+      if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+      cfgs++;
 
-   VG_UNDEF(&err,sizeof(err));
-   dec = opus_multistream_decoder_create(48000, 256, 255, 1, mapping, &err);
-   VG_CHECK(&err,sizeof(err));
-   if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
-   cfgs++;
+      VG_UNDEF(ret_err,sizeof(*ret_err));
+      dec = opus_multistream_decoder_create(48000, 1, -1, 2, mapping, ret_err);
+      if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+      if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+      cfgs++;
 
-   VG_UNDEF(&err,sizeof(err));
-   dec = opus_multistream_decoder_create(48000, 256, 255, 0, mapping, &err);
-   VG_CHECK(&err,sizeof(err));
-   if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
-   cfgs++;
+      VG_UNDEF(ret_err,sizeof(*ret_err));
+      dec = opus_multistream_decoder_create(48000, 1, -1, -1, mapping, ret_err);
+      if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+      if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+      cfgs++;
 
-   VG_UNDEF(&err,sizeof(err));
-   mapping[0]=255;
-   mapping[1]=1;
-   mapping[2]=2;
-   dec = opus_multistream_decoder_create(48000, 3, 2, 0, mapping, &err);
-   VG_CHECK(&err,sizeof(err));
-   if(err!=OPUS_BAD_ARG || dec!=NULL)test_failed();
-   cfgs++;
+      VG_UNDEF(ret_err,sizeof(*ret_err));
+      dec = opus_multistream_decoder_create(48000, 256, 255, 1, mapping, ret_err);
+      if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+      if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+      cfgs++;
 
-   VG_UNDEF(&err,sizeof(err));
-   mapping[0]=0;
-   mapping[1]=0;
-   mapping[2]=0;
-   dec = opus_multistream_decoder_create(48000, 3, 2, 1, mapping, &err);
-   VG_CHECK(&err,sizeof(err));
-   if(err!=OPUS_OK || dec==NULL)test_failed();
-   cfgs++;
-   opus_multistream_decoder_destroy(dec);
-   cfgs++;
+      VG_UNDEF(ret_err,sizeof(*ret_err));
+      dec = opus_multistream_decoder_create(48000, 256, 255, 0, mapping, ret_err);
+      if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+      if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+      cfgs++;
 
-   mapping[0]=0;
-   mapping[1]=255;
-   mapping[2]=1;
-   mapping[3]=2;
-   mapping[4]=3;
-   dec = opus_multistream_decoder_create(48001, 5, 4, 1, mapping, 0);
-   if(dec!=NULL)test_failed();
-   cfgs++;
+      VG_UNDEF(ret_err,sizeof(*ret_err));
+      mapping[0]=255;
+      mapping[1]=1;
+      mapping[2]=2;
+      dec = opus_multistream_decoder_create(48000, 3, 2, 0, mapping, ret_err);
+      if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+      if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+      cfgs++;
 
+      VG_UNDEF(ret_err,sizeof(*ret_err));
+      mapping[0]=0;
+      mapping[1]=0;
+      mapping[2]=0;
+      dec = opus_multistream_decoder_create(48000, 3, 2, 1, mapping, ret_err);
+      if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+      if((ret_err && *ret_err!=OPUS_OK) || dec==NULL)test_failed();
+      cfgs++;
+      opus_multistream_decoder_destroy(dec);
+      cfgs++;
+
+      VG_UNDEF(ret_err,sizeof(*ret_err));
+      mapping[0]=0;
+      mapping[1]=255;
+      mapping[2]=1;
+      mapping[3]=2;
+      mapping[4]=3;
+      dec = opus_multistream_decoder_create(48001, 5, 4, 1, mapping, ret_err);
+      if(ret_err){VG_CHECK(ret_err,sizeof(*ret_err));}
+      if((ret_err && *ret_err!=OPUS_BAD_ARG) || dec!=NULL)test_failed();
+      cfgs++;
+   }
+
    VG_UNDEF(&err,sizeof(err));
    mapping[0]=0;
    mapping[1]=255;
@@ -1060,6 +1078,9 @@
          VG_UNDEF(&err,sizeof(err));
          enc = opus_encoder_create(fs, c, OPUS_APPLICATION_VOIP, &err);
          if(err!=OPUS_BAD_ARG || enc!=NULL)test_failed();
+         cfgs++;
+         enc = opus_encoder_create(fs, c, OPUS_APPLICATION_VOIP, 0);
+         if(enc!=NULL)test_failed();
          cfgs++;
          opus_encoder_destroy(enc);
          enc=malloc(opus_encoder_get_size(2));
--- a/tests/test_opus_encode.c
+++ b/tests/test_opus_encode.c
@@ -144,6 +144,29 @@
    enc = opus_encoder_create(48000, 2, OPUS_APPLICATION_VOIP, &err);
    if(err != OPUS_OK || enc==NULL)test_failed();
 
+   for(i=0;i<2;i++)
+   {
+      int *ret_err;
+      ret_err = i?0:&err;
+      MSenc = opus_multistream_encoder_create(8000, 2, 2, 0, mapping, OPUS_UNIMPLEMENTED, ret_err);
+      if((ret_err && *ret_err != OPUS_BAD_ARG) || MSenc!=NULL)test_failed();
+
+      MSenc = opus_multistream_encoder_create(8000, 0, 1, 0, mapping, OPUS_APPLICATION_VOIP, ret_err);
+      if((ret_err && *ret_err != OPUS_BAD_ARG) || MSenc!=NULL)test_failed();
+
+      MSenc = opus_multistream_encoder_create(44100, 2, 2, 0, mapping, OPUS_APPLICATION_VOIP, ret_err);
+      if((ret_err && *ret_err != OPUS_BAD_ARG) || MSenc!=NULL)test_failed();
+
+      MSenc = opus_multistream_encoder_create(8000, 2, 2, 3, mapping, OPUS_APPLICATION_VOIP, ret_err);
+      if((ret_err && *ret_err != OPUS_BAD_ARG) || MSenc!=NULL)test_failed();
+
+      MSenc = opus_multistream_encoder_create(8000, 2, -1, 0, mapping, OPUS_APPLICATION_VOIP, ret_err);
+      if((ret_err && *ret_err != OPUS_BAD_ARG) || MSenc!=NULL)test_failed();
+
+      MSenc = opus_multistream_encoder_create(8000, 256, 2, 0, mapping, OPUS_APPLICATION_VOIP, ret_err);
+      if((ret_err && *ret_err != OPUS_BAD_ARG) || MSenc!=NULL)test_failed();
+   }
+
    MSenc = opus_multistream_encoder_create(8000, 2, 2, 0, mapping, OPUS_APPLICATION_AUDIO, &err);
    if(err != OPUS_OK || MSenc==NULL)test_failed();