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

[Michael Niedermayer]

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 -

[...]
-- 
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151012/013d33cb/attachment.sig>