[FFmpeg-devel] [PATCH 6/6] lavc/flacdsp: document limitations of the LPC encoder

[Christophe Gisquet]

Hi,

2014-08-12 23:22 GMT+02:00 James Darnley <james.darnley at gmail.com>:
> ---
>  libavcodec/flacdsp.h |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/libavcodec/flacdsp.h b/libavcodec/flacdsp.h
> index 272cf2a..36cd904 100644
> --- a/libavcodec/flacdsp.h
> +++ b/libavcodec/flacdsp.h
> @@ -27,6 +27,13 @@ typedef struct FLACDSPContext {
>                             int len, int shift);
>      void (*lpc)(int32_t *samples, const int coeffs[32], int order,
>                  int qlevel, int len);
> +    /**
> +     * This function has some limitations with various configurations:
> +     * - when CONFIG_SMALL is 0 there is an unrolled loop which assumes the
> +     *   maximum value of order is 32.
> +     * - when SSE4 (or newer) is available on x86 there is an unrolled copy
> +     *   which also assumes the maximum value of order is 0.
> +     */
>      void (*lpc_encode)(int32_t *res, const int32_t *smp, int len, int order,
>                         const int32_t *coefs, int shift);
>  } FLACDSPContext;

Some comments:
- The decoder passes int coeffs[32]; I don't see how order > 32 would then work
- Why not do the same for lpc_encode, and document that maximum order
value is 32
- The decoder actually reads the order (see decode_subframe and
flacdec.c:366) on 6 bits (there's a type >= 32 check though but I
don't see what I'm missing)
- The encoder has min and max bounds in (min,max)_prediction_order in
arrays (see flacenc.c:312) that are below 32 so the encoder should
never use an order above 32 (as seen with again the coding on 6bits on
1126)

I guess some of your comments in the asm and here could be dropped then ?

-- 
Christophe