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

[Vitor Sessak]

Michael Niedermayer wrote:
> 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

Applied.

-Vitor