[FFmpeg-devel] [PATCH/RFC] Remove triplication of compute_lpc_coefs() function

[Justin Ruggles]

Vitor Sessak wrote:
> Hi
> 
> Michael Niedermayer wrote:
>> On Thu, Aug 28, 2008 at 02:28:50PM +0200, Vitor Sessak wrote:
>>> Hi all.
>>>
>>> I've finally found a more or less clean way of doing $subj. I also
>>> want to know if it is ok to remove the following useless loop:
>>>
>>>> for(i=0; i<max_order; i++) lpc_tmp[i] = 0;
>>
>> all useless loops can be removed ...
> 
> Gone.
> 
>>
>>
>> Some comments ...
>> Is the double based code really a problem speedwise? IIRC someone (mans?)
>> said it was very slow on some ARM, but that considered flac float vs.
>> double which included the autocorrelation stuff, not just
>> compute_lpc_coefs() float vs. double IIRC.
> 
> Even if compute_lpc_coefs() is not speed critical, to convert all
> input/output buffers to double/float before passing to the function is
> ugly and slow.
> 
>> It should be clarified how much time is actually spend in this code when
>> encoding flac.
>>
>> Also iam still curious if all variables must be double to maintain the
>> good
>> compression with flac ...
> 
> The following patch makes the flac encoder use a float version of
> compute_lpc_coefs() but keeps doubles for the intermediate variables. In
> my tests I've not seem a worsening of the encoded size, but I'd be glad
> if someone could test it independently.
> 
> Index: libavcodec/flacenc.c
> ===================================================================
> --- libavcodec/flacenc.c	(revision 15005)
> +++ libavcodec/flacenc.c	(working copy)
> @@ -595,7 +595,7 @@
>   * A Welch window function is applied before calculation.
>   */
>  void ff_flac_compute_autocorr(const int32_t *data, int len, int lag,
> -                              double *autoc)
> +                              float *autoc)
>  {
>      int i, j;
>      double tmp[len + lag + 1];

This is not compatible with dsputil, which uses doubles.

I am not against changing this, but the dsputil functions would need to
be modified as well.

-Justin