[FFmpeg-devel] [PATCH] Separate window function from autocorrelation.

[Justin Ruggles]

On 01/19/2011 03:12 PM, M?ns Rullg?rd wrote:

> Justin Ruggles <justin.ruggles at gmail.com> writes:
> 
>> This will allow the use of different window functions and also removes
>> variable-length arrays in autocorrelation functions.
>> ---
>>  libavcodec/dsputil.c            |    1 +
>>  libavcodec/dsputil.h            |    7 ++++++-
>>  libavcodec/lpc.c                |   30 ++++++++++++++----------------
>>  libavcodec/lpc.h                |    4 +++-
>>  libavcodec/x86/dsputil_mmx.h    |    3 ++-
>>  libavcodec/x86/dsputilenc_mmx.c |    1 +
>>  libavcodec/x86/lpc_mmx.c        |   20 ++++++--------------
>>  7 files changed, 33 insertions(+), 33 deletions(-)
>>
>>
>> diff --git a/libavcodec/dsputil.c b/libavcodec/dsputil.c
>> index cc60524..3b2d421 100644
>> --- a/libavcodec/dsputil.c
>> +++ b/libavcodec/dsputil.c
>> @@ -4432,6 +4432,7 @@ av_cold void dsputil_init(DSPContext* c, AVCodecContext *avctx)
>>      c->ac3_downmix = ff_ac3_downmix_c;
>>  #endif
>>  #if CONFIG_LPC
>> +    c->lpc_apply_welch_window = ff_lpc_apply_welch_window;
>>      c->lpc_compute_autocorr = ff_lpc_compute_autocorr;
>>  #endif
> 
> Maybe we should move the LPC functions out of DSPContext.  This is as
> good a time as any to do that.

indeed.  I was thinking that, but I was taking it one step at a time.

I'll send a new patch set.

>>      c->vector_fmul = vector_fmul_c;
>> diff --git a/libavcodec/dsputil.h b/libavcodec/dsputil.h
>> index 1571491..ad61188 100644
>> --- a/libavcodec/dsputil.h
>> +++ b/libavcodec/dsputil.h
>> @@ -376,7 +376,12 @@ typedef struct DSPContext {
>>      void (*vorbis_inverse_coupling)(float *mag, float *ang, int blocksize);
>>      void (*ac3_downmix)(float (*samples)[256], float (*matrix)[2], int out_ch, int in_ch, int len);
>>      /* no alignment needed */
>> -    void (*lpc_compute_autocorr)(const int32_t *data, int len, int lag, double *autoc);
>> +    void (*lpc_apply_welch_window)(const int32_t *data, int len, double *w_data);
>> +    /* no alignment needed.
>> +       data must have have at least lag*sizeof(double) valid bytes preceeding it.
>> +       data must be at least (len+1)*sizeof(double) in length if data is
>> +       16-byte aligned or (len+2)*sizeof(double) if data is unaligned. */
>> +    void (*lpc_compute_autocorr)(const double *data, int len, int lag, double *autoc);
> 
> A brief doxygen description of the function would be nice.

ok.

>> @@ -179,12 +170,19 @@ int ff_lpc_calc_coefs(DSPContext *s,
>>             lpc_type > AV_LPC_TYPE_FIXED);
>>  
>>      if (lpc_type == AV_LPC_TYPE_LEVINSON) {
>> -        s->lpc_compute_autocorr(samples, blocksize, max_order, autoc);
>> +        double *ws = av_mallocz((blocksize + max_order + 2) * sizeof(double));
> 
> Unchecked malloc.  Is it feasible to have this buffer allocate once
> per codec instance instead of for each call?

oops. will fix.

And I'll address per-codec-instance allocation in the next patch set.

Thanks,
Justin