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

[James Darnley]

On 2014-08-13 09:21, Christophe Gisquet wrote:
> 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 ?

You are correct on these points.  Please read my reply to Michael to see
what I was thinking.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 618 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140813/a4b74a4e/attachment.asc>