[FFmpeg-devel] [PATCH 07/12] lpc: remove VLA in ff_lpc_compute_autocorr

[Justin Ruggles]

M?ns Rullg?rd wrote:

> Justin Ruggles <justin.ruggles at gmail.com> writes:
> 
>> Mans Rullgard wrote:
>>
>>> ---
>>>  libavcodec/lpc.c |    4 +++-
>>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/libavcodec/lpc.c b/libavcodec/lpc.c
>>> index 49e41d8..6b3cf01 100644
>>> --- a/libavcodec/lpc.c
>>> +++ b/libavcodec/lpc.c
>>> @@ -59,7 +59,7 @@ void ff_lpc_compute_autocorr(const int32_t *data, int len, int lag,
>>>                               double *autoc)
>>>  {
>>>      int i, j;
>>> -    double tmp[len + lag + 1];
>>> +    double *tmp = av_malloc((len + lag + 1) * sizeof(*tmp));
>>>      double *data1= tmp + lag;
>>>  
>>>      apply_welch_window(data, len, data1);
>>> @@ -86,6 +86,8 @@ void ff_lpc_compute_autocorr(const int32_t *data, int len, int lag,
>>>          }
>>>          autoc[j] = sum;
>>>      }
>>> +
>>> +    av_free(tmp);
>>>  }
>> looks fine to me.  But I think the sse2 version of this function also
>> has the same VLA.
> 
> Yes, it does, as I noticed later.  Requiring dsputil functions to
> allocate unknown amounts of memory us generally a bad idea.  Can we
> think of a better solution?  Make the caller pass a scratch buffer?

Well, I am currently working on revising the autocorrelation function,
mainly for the ALS encoder.  I'm considering splitting out the
windowing+int32-to-double part into a separate function or set of
functions.  So maybe we can change the autocorrelation function to take
double* input, with constraints that it be aligned and that at least
[-lag] be accessible (with negative array values either already zero or
containing actual sample values).  This is important for ALS since
previous sample values need to be included in certain situations.

-Justin