[FFmpeg-devel] [PATCH] aac_fixed: fix overflow in sbr_sum_square_c
Nedeljko.Babic at imgtec.com
Mon Nov 16 15:39:01 CET 2015
>> On 11.11.2015 13:46, Michael Niedermayer wrote:
>> > On Sun, Nov 08, 2015 at 09:26:21PM +0100, Andreas Cadhalpun wrote:
>> >> On 08.11.2015 20:17, Michael Niedermayer wrote:
>> >>> but the patch does not look like an optimal solution
>> >> It's certainly not pretty, but it fixes the crashes/assertion failures.
>> > at the expense of making the code rather slow and hard to implement in
>> > SIMD.
>> > This would be perfectly ok if that is neccessary but is it ?
>> > (i dont know)
>> That depends how you define necessary. It's possible to avoid the problem
>> with simpler, faster means, but those are less correct mathematically.
>I define neccessary as what is neccessary for a well working fixed
>point aac decoder.
>There are 2 possible cases i think
>A. The large input values are invalid then the correct solution is
> to detect that and error out / do error concealment
>B. The large values are valid, in this case the implementation is
> broken. And the question would arrise how to best support the valid
> range, can the values or their products be scaled down by a fixed
> shift? or does this affect quality, do we need a variable shift
> if so how to implement this best (like 2pass, find max, choose
> shift at a earlier stage possibly, some kind of floats, ...)
>> > do we know the valid input range?
>> I don't know about valid, but the possible input range currently seems
>> to be any 32-bit integer.
>yes, and neither do i know the valid range but
>SBR builds the high frequency signal and adds that to the base AAC
>low frequency signal. These cannot be arbitrary large as the output
>cannot be arbitrary large so there is a practical limit on how large
>these values can be unless its possible and allowed to have much
>larger intermediates at some point.
>> > or maybe "valid" is not the correct word (in case the spec does not
>> > specify that directly or indirectly ...)
>> > if thats the case then it could be a question if any encoder could
>> > ever have a reason to use values in a specific range
>> Anyway, the decoder has to deal with such values somehow.
>> It could also error out, but I don't know which error check to use.
>> And it would be a bit strange if the aac_fixed decoder errors out,
>> while the float aac decoder can handle such samples.
>can you check if there are any overflows happening in valid aac files
>if none of them overflows anywhere then i would guess the supported
>ranges are not that far off from what is used by actual encoders
>and just erroring out with a request for a sample might be an easy
>solution and much faster than converting all to some float emulation
>Comments fro AAC and SBR experts very welcome!
This code was developed a while ago, but based on informations that I have
this part of code was analysed regarding possibility of overflow and conclusion
was that there is no valid way for causing overflow here.
And regarding valid range, if I remember correctly it should be 29, not 32.
More information about the ffmpeg-devel