[MPlayer-cvslog] r33811 - trunk/libaf/af_format.c

[Ivan Kalvachev]

On 7/5/11, Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
>
>
> On 5 Jul 2011, at 00:24, iive <subversion at mplayerhq.hu> wrote:
>
>> Author: iive
>> Date: Tue Jul  5 00:24:52 2011
>> New Revision: 33811
>>
>> Log:
>> Audio format conversion from float to int is not checked for overflow.
>> The float may be slightly off the [-1;1] range thus causing audible
>> crackling noise.
>>
>> Add proper clipping using av_clip functions.
>
> That is not all you did.
> You changed the multipliers, you added a use of ldexp for half the cases and
> you made the code throw away up to 8 bit precision when converting to int
> (for files decoding to float in the range [-1/256;1/256]).
> Also float mantissa has 24 bits precision not 23.

I really missed to describe this in the message.

The exact multiples of 2 constants are the proper ones. You can see
them used in integet->float conversion case.
The decreased by 1 constants were used as a mean to avoid overflow
when we have 1.0, however they shrink the range and it could  lead to
distortion/fadedown if there are multiple back and forth conversions.
Indeed the 1.0 would be clipped to 0.996093475, but all other values
would be stable.

The ldexp() is used only as a faster way to multiply, as it changes
only the exponent. It have same requirement as lrint (C99,
_OPEN_SOURCE 600, etc). Do you prefer constant multiply or do you want
all cases replaced with ldexp()?

The float is 32 bits total, 1 bit sign, 8 bits exponent and 23 bits
mantissa. But you are right, there is one bit that is not stored, but
assumed. You are also correct for the loss of precision in this case.
Do you have an idea how to do the clipping quickly? The variants I'm
aware of are:

1. Using llrint, getting int64_t and clipping it with
av_clipl_int32(). Could be quite slower on 32 bit CPU.

2. Clipping the float to [-1;1] range. However this still overflow
even with the (1<<31)-1 constant, thanks to the 23+1 bit mantissa.
Using (1<<31)-(1<<8) could work, but the distortion would be even
bigger.

3. Doing the multiply of 2^31 first, and then clipping float numbers
before doing rounding. Thanks to lrint default rounding we'd have to
use (1<<31)-(1<<8) again. Either use trunc() or simple i=(int)f;
conversion to avoid it.

4. Using aliasing trick, where float number is assessed as integer and
we work with mantissa and exponent directly. That trick was present in
FFmpeg and was one of the things removed by the Masters during their
reign. It involves adding a bias with specially crafted constant,
checking for overflow with another constant ... It's tricky, it could
easily be slower.