[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:
> 
>> M?ns Rullg?rd wrote:
>>
>>> Justin Ruggles <justin.ruggles at gmail.com> writes:
>>>
>>>> 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(-)
>>>>>>>
>>>>>> 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.
>>> That's all very well, but doesn't really address the issue of the
>>> dsputil function needing to allocate unbounded amounts of scratch
>>> memory.
>> If the input is changed to have those constraints, that temporary memory
>> wouldn't need to be allocated at all.
> 
> Even better.

Great.  I'm working on the window function part now.  I'll try to have
something ready to submit by the end of this weekend.

-Justin