On Mon, Oct 12, 2015 at 11:09 AM, Michael Niedermayer <mich...@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 <gajjanaga...@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@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel