[FFmpeg-devel] [PATCH] audio conversion clipping/overflows
Michael Niedermayer
michaelni
Tue Mar 16 00:07:37 CET 2010
On Mon, Mar 15, 2010 at 06:31:21PM -0400, Ronald S. Bultje wrote:
> Hi,
>
> On Tue, Mar 2, 2010 at 7:36 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > also if you apply this, make sure you remove the cliping from every decoder
> > that matches SAMPLE_FMT_FLT
>
> OK, see attached patch. Maintainers of relevant decoders (Colin,
> Benjamin, Vitor, Kostya, Sascha, Roberto, Reynaldo), please bug me if
> there's a very specific reason that you would like me to NOT remove
> clipping from your code, else I'll apply this in +/- 3 days.
>
> Ronald
> libavcodec/amrnbdec.c | 11 ++++-------
> libavcodec/atrac1.c | 11 +++--------
> libavcodec/audioconvert.c | 18 ++++++++++++------
> libavcodec/qcelpdata.h | 10 ----------
> libavcodec/qcelpdec.c | 4 +---
> libavcodec/ra288.c | 4 ----
> libavcodec/sipr.c | 2 +-
> libavcodec/twinvq.c | 3 ---
> libavcodec/wmaprodec.c | 2 +-
> libavcodec/wmavoice.c | 3 +--
> libavutil/common.h | 11 +++++++++++
> 11 files changed, 34 insertions(+), 45 deletions(-)
> 956b42a420a8276c4fa540452f11195d927aaaf9 aconv-prevent-overflow.patch
> Index: ffmpeg-svn/libavcodec/audioconvert.c
> ===================================================================
> --- ffmpeg-svn.orig/libavcodec/audioconvert.c 2010-03-15 14:29:47.000000000 -0400
> +++ ffmpeg-svn/libavcodec/audioconvert.c 2010-03-15 14:33:41.000000000 -0400
> @@ -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))))
> else CONV(SAMPLE_FMT_FLT, float , SAMPLE_FMT_FLT, *(const float*)pi)
> else CONV(SAMPLE_FMT_DBL, double , SAMPLE_FMT_FLT, *(const float*)pi)
> - else CONV(SAMPLE_FMT_U8 , uint8_t, SAMPLE_FMT_DBL, lrint(*(const double*)pi * (1<<7)) + 0x80)
> - else CONV(SAMPLE_FMT_S16, int16_t, SAMPLE_FMT_DBL, lrint(*(const double*)pi * (1<<15)))
> - else CONV(SAMPLE_FMT_S32, int32_t, SAMPLE_FMT_DBL, lrint(*(const double*)pi * (1<<31)))
> + else CONV(SAMPLE_FMT_U8 , uint8_t, SAMPLE_FMT_DBL,
> + av_clip_uint8(lrint(*(const double*)pi * (1<<7)) + 0x80))
> + else CONV(SAMPLE_FMT_S16, int16_t, SAMPLE_FMT_DBL,
> + av_clip_int16(lrint(*(const double*)pi * (1<<15))))
> + else CONV(SAMPLE_FMT_S32, int32_t, SAMPLE_FMT_DBL,
> + av_clipl_int32(llrint(*(const double*)pi * (1<<31))))
> else CONV(SAMPLE_FMT_FLT, float , SAMPLE_FMT_DBL, *(const double*)pi)
> else CONV(SAMPLE_FMT_DBL, double , SAMPLE_FMT_DBL, *(const double*)pi)
> else return -1;
> Index: ffmpeg-svn/libavutil/common.h
> ===================================================================
> --- ffmpeg-svn.orig/libavutil/common.h 2010-03-15 14:29:25.000000000 -0400
> +++ ffmpeg-svn/libavutil/common.h 2010-03-15 14:33:41.000000000 -0400
> @@ -145,6 +145,17 @@
> }
>
> /**
> + * Clips a signed 64-bit integer value into the -2147483648,2147483647 range.
> + * @param a value to clip
> + * @return clipped value
> + */
> +static inline av_const int32_t av_clipl_int32(int64_t a)
> +{
> + if ((a+2147483648) & ~2147483647) return (a>>63) ^ 2147483647;
> + else return a;
> +}
> +
> +/**
> * Clips a float value into the amin-amax range.
> * @param a value to clip
> * @param amin minimum value of the clip range
> Index: ffmpeg-svn/libavcodec/amrnbdec.c
> ===================================================================
> --- ffmpeg-svn.orig/libavcodec/amrnbdec.c 2010-03-15 14:30:19.000000000 -0400
> +++ ffmpeg-svn/libavcodec/amrnbdec.c 2010-03-15 18:10:23.000000000 -0400
[...]
> @@ -1049,8 +1047,7 @@
> p->high_pass_mem, AMR_BLOCK_SIZE);
>
> for (i = 0; i < AMR_BLOCK_SIZE; i++)
> - buf_out[i] = av_clipf(buf_out[i] * AMR_SAMPLE_SCALE,
> - -1.0, 32767.0 / 32768.0);
> + buf_out[i] *= AMR_SAMPLE_SCALE;
cant this be merged into something ?
>
> /* Update averaged lsf vector (used for fixed gain smoothing).
> *
> Index: ffmpeg-svn/libavcodec/qcelpdata.h
> ===================================================================
> --- ffmpeg-svn.orig/libavcodec/qcelpdata.h 2010-03-15 17:49:20.000000000 -0400
> +++ ffmpeg-svn/libavcodec/qcelpdata.h 2010-03-15 18:15:26.000000000 -0400
> @@ -425,16 +425,6 @@
> #define QCELP_SCALE 8192.
>
> /**
> - * the upper boundary of the clipping, depends on QCELP_SCALE
> - */
> -#define QCELP_CLIP_UPPER_BOUND (8191.75/8192.)
> -
> -/**
> - * the lower boundary of the clipping, depends on QCELP_SCALE
> - */
> -#define QCELP_CLIP_LOWER_BOUND -1.
> -
> -/**
> * table for computing Ga (decoded linear codebook gain magnitude)
> *
> * @note The table could fit in int16_t in x*8 form, but it seems
> Index: ffmpeg-svn/libavcodec/ra288.c
> ===================================================================
> --- ffmpeg-svn.orig/libavcodec/ra288.c 2009-04-13 16:10:14.000000000 -0400
> +++ ffmpeg-svn/libavcodec/ra288.c 2010-03-15 18:02:29.000000000 -0400
> @@ -102,10 +102,6 @@
> gain_block[9] = 10 * log10(sum) - 32;
>
> ff_celp_lp_synthesis_filterf(block, ractx->sp_lpc, buffer, 5, 36);
> -
> - /* output */
> - for (i=0; i < 5; i++)
> - block[i] = av_clipf(block[i], -4095./4096., 4095./4096.);
> }
>
> /**
> Index: ffmpeg-svn/libavcodec/sipr.c
> ===================================================================
> --- ffmpeg-svn.orig/libavcodec/sipr.c 2010-03-15 14:30:18.000000000 -0400
> +++ ffmpeg-svn/libavcodec/sipr.c 2010-03-15 18:01:51.000000000 -0400
> @@ -496,7 +496,7 @@
> ctx->highpass_filt_mem,
> frame_size);
>
> - ctx->dsp.vector_clipf(out_data, synth, -1, 32767./(1<<15), frame_size);
> + memcpy(out_data, synth, frame_size * sizeof(synth[0]));
>
> }
please dont memcpy()
if its really impossible to reorganize things so as to avoid it we should
extend the API to allow returning a AVFrame with data[] pointing to the
samples like e do for video decoding
[...]
> Index: ffmpeg-svn/libavcodec/wmaprodec.c
> ===================================================================
> --- ffmpeg-svn.orig/libavcodec/wmaprodec.c 2010-03-15 14:30:19.000000000 -0400
> +++ ffmpeg-svn/libavcodec/wmaprodec.c 2010-03-15 18:13:44.000000000 -0400
> @@ -1351,7 +1351,7 @@
> ptr = s->samples + i;
>
> for (x = 0; x < s->samples_per_frame; x++) {
> - *ptr = av_clipf(*iptr++, -1.0, 32767.0 / 32768.0);
> + *ptr = *iptr++;
> ptr += incr;
> }
same
>
> Index: ffmpeg-svn/libavcodec/atrac1.c
> ===================================================================
> --- ffmpeg-svn.orig/libavcodec/atrac1.c 2010-03-15 14:29:47.000000000 -0400
> +++ ffmpeg-svn/libavcodec/atrac1.c 2010-03-15 18:12:00.000000000 -0400
> @@ -308,17 +308,12 @@
> /* round, convert to 16bit and interleave */
> if (q->channels == 1) {
> /* mono */
> - q->dsp.vector_clipf(samples, q->out_samples[0], -32700.0 / (1 << 15),
> - 32700.0 / (1 << 15), AT1_SU_SAMPLES);
> + memcpy(samples, q->out_samples[0], AT1_SU_SAMPLES * 4);
and again
[...]
> Index: ffmpeg-svn/libavcodec/qcelpdec.c
> ===================================================================
> --- ffmpeg-svn.orig/libavcodec/qcelpdec.c 2010-03-15 18:05:21.000000000 -0400
> +++ ffmpeg-svn/libavcodec/qcelpdec.c 2010-03-15 18:05:34.000000000 -0400
> @@ -798,9 +798,7 @@
> // TIA/EIA/IS-733 2.4.8.6
>
> formant_mem = q->formant_mem + 10;
> - for(i=0; i<160; i++)
> - *outbuffer++ = av_clipf(*formant_mem++, QCELP_CLIP_LOWER_BOUND,
> - QCELP_CLIP_UPPER_BOUND);
> + memcpy(outbuffer, formant_mem, 160 * sizeof(formant_mem[0]));
>
> memcpy(q->prev_lspf, quantized_lspf, sizeof(q->prev_lspf));
> q->prev_bitrate = q->bitrate;
...
> Index: ffmpeg-svn/libavcodec/wmavoice.c
> ===================================================================
> --- ffmpeg-svn.orig/libavcodec/wmavoice.c 2010-03-15 17:45:53.000000000 -0400
> +++ ffmpeg-svn/libavcodec/wmavoice.c 2010-03-15 18:29:02.000000000 -0400
> @@ -1117,8 +1117,7 @@
> av_log_missing_feature(ctx, "APF", 0);
> s->do_apf = 0;
> } //else
> - for (n = 0; n < 160; n++)
> - samples[n] = av_clipf(synth[n], -1.0, 1.0);
> + memcpy(samples, synth, 160 * sizeof(synth[0]));
>
> /* Cache values for next frame */
> s->frame_cntr++;
.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The educated differ from the uneducated as much as the living from the
dead. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100316/ee4894b6/attachment.pgp>
More information about the ffmpeg-devel
mailing list