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

[Ronald S. Bultje]

Hi,

On Mon, Mar 15, 2010 at 7:07 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Mon, Mar 15, 2010 at 06:31:21PM -0400, Ronald S. Bultje wrote:
>> 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 ?

Will try.

>> 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()

Speaking for wmavoice (I'm guessing sipr is the same, I didn't check
all of them), the synth variable has FILTER_ORDER (10, 16, or so)
floats memory before synth[0], which contain history of previous
frames. We naturally cannot assume samples has such padding, hence the
separate variable.

>> 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

this isn't a memcpy(), it's an interpolation (incr is not 1, incr =
n_channels, so it goes from [chan0, samp0][chan0, samp1][chan0,
..][chan0, sampN][chan1,samp0][...] to
[chan0,samp0][chan1,samp0][...]. See also the second block of changes
to atrac1.c.

Ronald