[FFmpeg-devel] [PATCH] audio conversion clipping/overflows

[Måns Rullgård]

"Ronald S. Bultje" <rsbultje at gmail.com> writes:

> Hi,
>
> float 1.0 rounds to int INT_MIN (whatever int size). This leads to
> considerable audio clipping artifacts.
>
> Attached patch is a lame attempt of mine to fix this. It probably
> makes the code slower, so sue me. :-).
>
> Ronald
>
> Index: libavcodec/audioconvert.c
> ===================================================================
> --- libavcodec/audioconvert.c	(revision 22027)
> +++ libavcodec/audioconvert.c	(working copy)
> @@ -226,14 +226,20 @@
>          else CONV(SAMPLE_FMT_S32, int32_t, SAMPLE_FMT_S32,  *(const int32_t*)pi)
>          else CONV(SAMPLE_FMT_FLT, float  , SAMPLE_FMT_S32,  *(const int32_t*)pi*(1.0 / (1<<31)))
>          else CONV(SAMPLE_FMT_DBL, double , SAMPLE_FMT_S32,  *(const int32_t*)pi*(1.0 / (1<<31)))
> -        else CONV(SAMPLE_FMT_U8 , uint8_t, SAMPLE_FMT_FLT, lrintf(*(const float*)pi * (1<<7)) + 0x80)
> -        else CONV(SAMPLE_FMT_S16, int16_t, SAMPLE_FMT_FLT, lrintf(*(const float*)pi * (1<<15)))
> -        else CONV(SAMPLE_FMT_S32, int32_t, SAMPLE_FMT_FLT, lrintf(*(const float*)pi * (1<<31)))
> +        else CONV(SAMPLE_FMT_U8 , uint8_t, SAMPLE_FMT_FLT,
> +                  av_clip_uint8(lrintf(*(const float*)pi * (1<<7)) + 0x80))
> +        else CONV(SAMPLE_FMT_S16, int16_t, SAMPLE_FMT_FLT,
> +                  av_clip_int16(lrintf(*(const float*)pi * (1<<15))))
> +        else CONV(SAMPLE_FMT_S32, int32_t, SAMPLE_FMT_FLT,
> +                  av_clipl_int32(llrintf(*(const float*)pi * (1<<31))))

This has two problems: 1) it's slow, and 2) it clips the signal.

The best solution to this problem would be if a suitable output range
could be requested directly from the decoders.  For some decoders this
is easy in principle, for others it is not.

This proper solution is obviously quite involved, so an interim fix
for the cases now resulting in undefined/implementation-defined
behaviour is called for.

I would suggest scaling the float values by (1<<N)-1.  This would
sacrifice the INT_MIN output value in return for an unclipped output
as well as speed.

-- 
M?ns Rullg?rd
mans at mansr.com