[FFmpeg-devel] [PATCH] aac: Clip large (legal) PNS/IS values

Robert Swain robert.swain
Tue Oct 13 21:52:17 CEST 2009

2009/10/13 Alex Converse <alex.converse at gmail.com>:
> On Tue, Oct 13, 2009 at 2:58 PM, Robert Swain <robert.swain at gmail.com> wrote:
>> 2009/10/10 Alex Converse <alex.converse at gmail.com>:
>>> On Fri, Oct 9, 2009 at 7:34 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>> On Fri, Oct 09, 2009 at 12:38:17PM -0400, Alex Converse wrote:
>>>>> On Fri, Oct 9, 2009 at 9:37 AM, Robert Swain <robert.swain at gmail.com> wrote:
>>>>> > 2009/10/6 Alex Converse <alex.converse at gmail.com>:
>>>>> >> Large intensity stereo and PNS indices are legal. Clip them instead of
>>>>> >> erroring out. A magnitude of 100 corresponds to 2^25 so the will most
>>>>> >> likely result in clipped output anyway.
>>>>> >>
>>>>> >> None of the conformance streams fall in the range that need to be clipped.
>>>>> >
>>>>> > Looks OK to me as long as it isn't arbitrary clipping but rather
>>>>> > something needed/allowed within and specified by the specification.
>>>>> >
>>>>> The clipping is semi-arbitrary. It was designed such that after the
>>>>> third patch the ranges are the same as regular scalefactors. However
>>>>> these large vales (even when clipped) should cause cause output
>>>>> clipping on any streams that aren't purely academic exercises.
>>>> IMHO, either
>>>> A. it can be in a valid stream -> we should support it
>>>> B. it can not be in a valid stream -> error concealment (silence /repeat
>>>> ? last block / something better) should be applied
>>>> cliping seems always wrong (to me) unless mandated by a spec.
>>>> switching between A and B, in the sense of "it could be valid but most
>>>> likely is not, lets ask the user" can be done by using
>>>> AVCodecContext.error_recognition
>>> It's a mix between that (the error recognition case) and the fact that
>>> even if you decode it properly there isn't going to be a difference
>>> between clipping and not.
>>> For instance let's look at intensity stereo. If the IS scalefactor is
>>> 100, associated then the gain is 2^-25 The only way for the channel
>>> receiving the intensity stereo to get something that doesn't round to
>>> zero is for the source channel to be clipped beyond belief.
>>> If that gain of 2**-25 seems reasonable to you, then consider another
>>> legal IS scalefactor, 512. 512 corresponds to a gain of 2**128. That
>>> is larger than the maximum finite float.
>>> IS scalefactors are coded differentially from 0 with a maximum step of
>>> +/-60 which is another reason why very large values are unlikely.
>> As long as the scalefactor values are valid within the specification,
>> I think we should only clip them if it is provable that doing so does
>> not alter the output of any valid stream, academic exercise or not.
> Well right now we are throwing out valid streams rather than clipping
> them which is worse IMHO.

True, but I think we could just as well 'perfect' the scalefactor
decoding rather than 'just' improve it a bit.

Honestly, I'm fine with your clipping because I think in a world where
people write sane encoders that use sane values it should all be
peachy. It's just that there's probably someone out there who wants to
use the insane values for some purpose. As far as I'm concerned, the
patch is OK. If anyone else wants to reject it, feel free.


More information about the ffmpeg-devel mailing list