[FFmpeg-devel] [PATCH] avcodec/apedec: use int64_t for FFABS

[Ganesh Ajjanagadde]

On Mon, Oct 12, 2015 at 11:09 AM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Fri, Oct 09, 2015 at 01:48:10PM -0400, Ganesh Ajjanagadde wrote:
>> res, absres are currently int's, which on most platforms is 32 bits.
>> Unfortunately, data is untrusted, and on line 1267 res is manipulated
>> with data. Thus, res can take on INT32_MIN/INT_MIN with crafted data,
>> making FFABS on line 1282 unsafe.
>>
>> Once again, using FFNABS will make it less readable: logic is less
>> clear, diff is bigger, and there is scope for mistakes during the
>> expression rewrites.
>>
>> Tested with FATE.
>>
>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> ---
>>  libavcodec/apedec.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
>> index fcccfbe..e46558e 100644
>> --- a/libavcodec/apedec.c
>> +++ b/libavcodec/apedec.c
>> @@ -1254,8 +1254,8 @@ static void init_filter(APEContext *ctx, APEFilter *f, int16_t *buf, int order)
>>  static void do_apply_filter(APEContext *ctx, int version, APEFilter *f,
>>                              int32_t *data, int count, int order, int fracbits)
>>  {
>> -    int res;
>> -    int absres;
>> +    int64_t res;
>> +    int64_t absres;
>
> this makes the function slower:
> before: 1636364 decicycles in doapply,     128 runs,      0 skips
> after:  1710778 decicycles in doapply,     128 runs,      0 skips
>
> (this is on x86-64, i assume its a bigger difference on 32bit)
>
> tested with:
> ./ffmpeg -i fate-suite//lossless-audio/luckynight-partial.ape -f null -

Ok, but don't you agree that overflow is possible? int16 (essentially
the width after the scalar product) + int32 may not fit in int32. If
we want a lossless conversion, int64 must be used; there is no other
option AFAIK.

>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I do not agree with what you have to say, but I'll defend to the death your
> right to say it. -- Voltaire
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>