[FFmpeg-devel] [PATCH 1/4] Move lpc_compute_autocorr() from DSPContext to a new struct LPCContext.

[Måns Rullgård]

Justin Ruggles <justin.ruggles at gmail.com> writes:

> ---
>  libavcodec/alacenc.c            |   14 +++++++-------
>  libavcodec/dsputil.c            |    4 ----
>  libavcodec/dsputil.h            |    2 --
>  libavcodec/flacenc.c            |    7 +++----
>  libavcodec/lpc.c                |   22 ++++++++++++++++++----
>  libavcodec/lpc.h                |   30 +++++++++++++++++++++++++++---
>  libavcodec/ra144.h              |    4 ++--
>  libavcodec/ra144enc.c           |    5 ++---
>  libavcodec/x86/dsputilenc_mmx.c |    4 ----
>  libavcodec/x86/lpc_mmx.c        |   10 ++++++++++
>  10 files changed, 69 insertions(+), 33 deletions(-)
>
>
> diff --git a/libavcodec/lpc.c b/libavcodec/lpc.c
> index 112a78d..5fd1e19 100644
> --- a/libavcodec/lpc.c
> +++ b/libavcodec/lpc.c
> @@ -20,7 +20,7 @@
>   */
>  
>  #include "libavutil/lls.h"
> -#include "dsputil.h"
> +#include "libavutil/cpu.h"

I don't see that you'd need anything from cpu.h here.

>  #define LPC_USE_DOUBLE
>  #include "lpc.h"
> @@ -55,7 +55,7 @@ static void apply_welch_window(const int32_t *data, int len, double *w_data)
>   * Calculate autocorrelation data from audio samples
>   * A Welch window function is applied before calculation.
>   */
> -void ff_lpc_compute_autocorr(const int32_t *data, int len, int lag,
> +static void lpc_compute_autocorr_c(const int32_t *data, int len, int lag,
>                               double *autoc)
>  {
>      int i, j;
> @@ -88,6 +88,12 @@ void ff_lpc_compute_autocorr(const int32_t *data, int len, int lag,
>      }
>  }
>  
> +void ff_lpc_compute_autocorr(LPCContext *c, const int32_t *data, int len,
> +                             int lag, double *autoc)
> +{
> +    c->lpc_compute_autocorr(data, len, lag, autoc);
> +}

What is the purpose of this indirection?

>  /**
>   * Quantize LPC coefficients
>   */
> @@ -162,7 +168,7 @@ static int estimate_best_order(double *ref, int min_order, int max_order)
>   * 1  = LPC with coeffs determined by Levinson-Durbin recursion
>   * 2+ = LPC with coeffs determined by Cholesky factorization using (use_lpc-1) passes.
>   */
> -int ff_lpc_calc_coefs(DSPContext *s,
> +int ff_lpc_calc_coefs(LPCContext *s,
>                        const int32_t *samples, int blocksize, int min_order,
>                        int max_order, int precision,
>                        int32_t coefs[][MAX_LPC_ORDER], int *shift,
> @@ -179,7 +185,7 @@ 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);
> +        ff_lpc_compute_autocorr(s, samples, blocksize, max_order, autoc);

Please call s->lpc_compute_autocorr() directly.

>          compute_lpc_coefs(autoc, max_order, &lpc[0][0], MAX_LPC_ORDER, 0, 1);
>  
> @@ -236,3 +242,11 @@ int ff_lpc_calc_coefs(DSPContext *s,
>  
>      return opt_order;
>  }
> +
> +av_cold void ff_lpc_init(LPCContext *s)
> +{
> +    s->lpc_compute_autocorr = lpc_compute_autocorr_c;
> +
> +    if (HAVE_MMX)
> +        ff_lpc_init_x86(s);
> +}
> diff --git a/libavcodec/lpc.h b/libavcodec/lpc.h
> index 1c595f6..0a30988 100644
> --- a/libavcodec/lpc.h
> +++ b/libavcodec/lpc.h
> @@ -36,18 +36,42 @@
>  #define MAX_LPC_ORDER       32
>  
>  
> +typedef struct LPCContext {
> +    void (*lpc_compute_autocorr)(const int32_t *data, int len, int lag,
> +                                 double *autoc);
> +} LPCContext;
> +
> +
>  /**
>   * Calculate LPC coefficients for multiple orders
>   */
> -int ff_lpc_calc_coefs(DSPContext *s,
> +int ff_lpc_calc_coefs(LPCContext *s,
>                        const int32_t *samples, int blocksize, int min_order,
>                        int max_order, int precision,
>                        int32_t coefs[][MAX_LPC_ORDER], int *shift,
>                        enum AVLPCType lpc_type, int lpc_passes,
>                        int omethod, int max_shift, int zero_shift);
>  
> -void ff_lpc_compute_autocorr(const int32_t *data, int len, int lag,
> -                             double *autoc);
> +/**
> + * Perform autocorrelation on input samples with delay of 0 to lag.
> + * @param data  input samples.
> + *              constraints: no alignment needed, but must have have at least
> + *              lag*sizeof(double) valid bytes preceeding it, and size must be
> + *              at least (len+1)*sizeof(double) if data is 16-byte aligned or
> + *              (len+2)*sizeof(double) if data is unaligned.
> + * @param len   number of input samples to process
> + * @param lag   maximum delay to calculate
> + * @param autoc output autocorrelation coefficients.
> + *              constraints: array size must be at least lag+1.
> + */

Please move this doxymentation to the struct above and...

> +void ff_lpc_compute_autocorr(LPCContext *c, const int32_t *data, int len,
> +                             int lag, double *autoc);

... remove this prototype.

Rest of patch looks good.

-- 
M?ns Rullg?rd
mans at mansr.com