[opus] [PATCH] Fix memory issue in Projection API

Jean-Marc Valin jmvalin at jmvalin.ca
Wed Nov 22 03:39:02 UTC 2017


Hi Drew,

Had a look at your patch. I think it's on the right track -- there's no
fundamental issue with it as far as I can tell. There's a few
implementation issues though.

1) In opus_multi_stream_decode_float(), you changed the pcm argument
from float* to opus_val16*. That's a mistake and will fail for
fixed-point. This should have shown up as an error on a fixed-point
build. Make sure you test that.

2) In opus_projection_decode(), you also changed the pcm argument to an
opus_val16*, which again will cause problems for the same reasons as 1).
Please check you haven't made that same mistake elsewhere.

3) In opus_projection_copy_channel_out_float() and
opus_projection_copy_channel_out_short(), you're zeroing the values with
this loop:
    for (i=0;i<frame_size;i++)
    {
      for (j=0;j<dst_stride;j++)
      {
        float_dst[i*dst_stride+j] = 0;
      }
    }
That looks wrong, since you'll be overwriting all channels instead of
just the one you're copying. That should have come up in testing, no?

4) You might want to have a look at
mapping_matrix_multiply_channel_out_short(). I can't quite follow what
it's doing, but it seems wrong since the whole point of adding the "tmp"
is to avoid rounding multiple times.

I haven't had time to thoroughly review the changes to mapping_matrix.c,
but I'll do that on your revised version.

Cheers,

	Jean-Marc




On 11/20/2017 05:15 PM, Drew Allen wrote:
> Hello,
> 
> Attached is a patch to resolve a memory issue using the Projection API
> when compiling using a psuedo-stack / limited memory.
> 
> Please let me any ?s you might have.
> 
> Cheers,
> Drew
> 
> 
> _______________________________________________
> opus mailing list
> opus at xiph.org
> http://lists.xiph.org/mailman/listinfo/opus
> 


More information about the opus mailing list