[FFmpeg-devel] [PATCH] aac_fixed: fix overflow in sbr_sum_square_c
michael at niedermayer.cc
Mon Nov 16 02:59:15 CET 2015
On Fri, Nov 13, 2015 at 10:32:47PM +0100, Andreas Cadhalpun wrote:
> On 13.11.2015 04:15, Michael Niedermayer wrote:
> > On Thu, Nov 12, 2015 at 08:43:42PM +0100, Andreas Cadhalpun wrote:
> >> Considering that the aac float decoder can decode such samples, I tend
> >> to think that the aac fixed decoder should be able to do that, too.
> > IMO this reasoning is flawed
> > if you write a float mpeg2 decoder and feed it a fuzzed file and
> > that decoder skips all checks then you can get >16bit coefficients
> > as input to the IDCT.
> > Changing the fixed point IDCTs to work with 32x32 bit mutiplies
> > would not fix anything it would just make everything much slower
> > either way the authors of the fixed point aac decoder should
> > decide on what needs to be done, they implemented it and know this
> > code much better and why it scales the values as they are scaled ...
> Yes, it would be great if they could comment on this.
> >> If the FFmpeg aac encoder produces valid aac files then, yes, lots of
> >> overflows can happen with valid aac files.
> >> The decoder overflows even with a FATE sample.
> > i think this is something the authors of the decoder should look
> > into
> > also if there are overflows with fate samples why does this not
> > show up on any test on fate.ffmpeg.org ?
> Because these samples aren't tested with the aac_fixed decoder:
> * aac/ct_faac-adts.aac is only used to test the aac demuxer.
> * aac/al07_96.mp4 is for some reason only tested with the aac decoder.
> There the overflow happens on lines like:
> che->ch.ret[j] = (int32_t)av_clipl_int32((int64_t)che->ch.ret[j]<<7)+0x8000;
> I guess the +0x8000 was meant to be inside av_clipl_int32.
also there was a different patch about a overflow in that
0729 11:58 Nedeljko Babic (1.8K) [FFmpeg-devel] [PATCH] avcodec/aacdec_fixed: Fix integer overflow
that case was a bug elsewhere though
in that light, my first question, is, is the overflowing value used
(aka affects the decoder output) ?
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 181 bytes
Desc: Digital signature
More information about the ffmpeg-devel