[FFmpeg-cvslog] r20485 - in trunk/libavcodec: lsp.c lsp.h qcelpdec.c

[Michael Niedermayer]

On Mon, Nov 09, 2009 at 06:07:07PM +0100, Vitor Sessak wrote:
> M?ns Rullg?rd wrote:
>> vitor <subversion at mplayerhq.hu> writes:
>>> Author: vitor
>>> Date: Mon Nov  9 13:06:19 2009
>>> New Revision: 20485
>>>
>>> Log:
>>> Do not hardcode filter order in ff_acelp_lspd2lpc()
>>>
>>> Modified:
>>>    trunk/libavcodec/lsp.c
>>>    trunk/libavcodec/lsp.h
>>>    trunk/libavcodec/qcelpdec.c
>>>
>>> Modified: trunk/libavcodec/lsp.c
>>> ==============================================================================
>>> --- trunk/libavcodec/lsp.c	Mon Nov  9 10:11:35 2009	(r20484)
>>> +++ trunk/libavcodec/lsp.c	Mon Nov  9 13:06:19 2009	(r20485)
>>> @@ -155,20 +155,19 @@ static void lsp2polyf(const double *lsp,
>>>      }
>>>  }
>>>
>>> -void ff_acelp_lspd2lpc(const double *lsp, float *lpc)
>>> +void ff_acelp_lspd2lpc(const double *lsp, float *lpc, int lp_half_order)
>>>  {
>>> -    double pa[6], qa[6];
>>> -    int   i;
>>> +    double pa[lp_half_order+1], qa[lp_half_order+1];
>> Sorry I didn't spot this earlier, but we really should avoid
>> variable-length arrays.  Compilers do the silliest things with them.
>> For instance, gcc will not inline a function with a VLA.  
>
> I can vaguely understand why a compiler would do that (at least if the 
> caller do not pass the length as a constant).
>
>> Some
>> compilers call malloc() to allocate the array, and still some silently
>> miscompile the code.  That is in addition to being an inherently bad
>> idea.  If the allocation fails, you have no chance in hell of
>> recovering.
>
> I suppose the compiler is trying to work-around stupid coders that allocate 
> several MB this way :p
>
>> In this case, I would suggest setting a sensible upper limit, maybe 8
>> or 16, and using fixed-size arrays.
>
> Fine by me, patch attached.
>
> -Vitor

>  lsp.c |    2 +-
>  lsp.h |    5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> d9a8d3b2afe7cd22569c969236e030fc464b4596  lspd2lpc_VLA.diff

ok with an assert() that checks the max

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you really think that XML is the answer, then you definitly missunderstood
the question -- Attila Kinali
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20091109/91037c97/attachment.pgp>