On 06/26/2011 05:01 PM, Måns Rullgård wrote:
> Justin Ruggles <[email protected]> writes:
>
>> This ensures that any processing between the MDCT and exponent extraction
>> will
>> be using clipped coefficients.
>> ---
>> libavcodec/ac3enc.c | 10 +++++++++-
>> libavcodec/ac3enc.h | 8 ++++++++
>> libavcodec/ac3enc_template.c | 16 +++++++++++++++-
>> 3 files changed, 32 insertions(+), 2 deletions(-)
>>
>>
>> diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
>> index 3426bd2..e5e2edc 100644
>> --- a/libavcodec/ac3enc.c
>> +++ b/libavcodec/ac3enc.c
>> @@ -1898,7 +1898,10 @@ int ff_ac3_encode_frame(AVCodecContext *avctx,
>> unsigned char *frame,
>>
>> s->apply_mdct(s);
>>
>> - s->scale_coefficients(s);
>> + if (s->fixed_point)
>> + s->scale_coefficients(s);
>> +
>> + s->clip_coefficients(s);
>>
>> s->cpl_on = s->cpl_enabled;
>> compute_coupling_strategy(s);
>> @@ -1908,6 +1911,9 @@ int ff_ac3_encode_frame(AVCodecContext *avctx,
>> unsigned char *frame,
>>
>> s->compute_rematrixing_strategy(s);
>>
>> + if (!s->fixed_point)
>> + s->scale_coefficients(s);
>> +
>> apply_rematrixing(s);
>>
>> process_exponents(s);
>> @@ -2354,6 +2360,7 @@ av_cold int ff_ac3_encode_init(AVCodecContext *avctx)
>> s->allocate_sample_buffers =
>> ff_ac3_fixed_allocate_sample_buffers;
>> s->deinterleave_input_samples =
>> ff_ac3_fixed_deinterleave_input_samples;
>> s->apply_mdct = ff_ac3_fixed_apply_mdct;
>> + s->clip_coefficients = ff_ac3_fixed_clip_coefficients;
>> s->apply_channel_coupling =
>> ff_ac3_fixed_apply_channel_coupling;
>> s->compute_rematrixing_strategy =
>> ff_ac3_fixed_compute_rematrixing_strategy;
>> } else if (CONFIG_AC3_ENCODER || CONFIG_EAC3_ENCODER) {
>> @@ -2364,6 +2371,7 @@ av_cold int ff_ac3_encode_init(AVCodecContext *avctx)
>> s->allocate_sample_buffers =
>> ff_ac3_float_allocate_sample_buffers;
>> s->deinterleave_input_samples =
>> ff_ac3_float_deinterleave_input_samples;
>> s->apply_mdct = ff_ac3_float_apply_mdct;
>> + s->clip_coefficients = ff_ac3_float_clip_coefficients;
>> s->apply_channel_coupling =
>> ff_ac3_float_apply_channel_coupling;
>> s->compute_rematrixing_strategy =
>> ff_ac3_float_compute_rematrixing_strategy;
>> }
>> diff --git a/libavcodec/ac3enc.h b/libavcodec/ac3enc.h
>> index bf25298..ff8a389 100644
>> --- a/libavcodec/ac3enc.h
>> +++ b/libavcodec/ac3enc.h
>> @@ -50,12 +50,16 @@
>> #if CONFIG_AC3ENC_FLOAT
>> #define AC3_NAME(x) ff_ac3_float_ ## x
>> #define MAC_COEF(d,a,b) ((d)+=(a)*(b))
>> +#define COEF_MIN (-16777215.0/16777216.0)
>> +#define COEF_MAX ( 16777215.0/16777216.0)
>
> Why that range?
So that the integer range will be correct after scaling by 1<<24.
I'm working on a patch set that will remove the scaling from the float
encoder, but even in that case this range is needed to avoid having to
do additional exponent clipping.
>> typedef float SampleType;
>> typedef float CoefType;
>> typedef float CoefSumType;
>> #else
>> #define AC3_NAME(x) ff_ac3_fixed_ ## x
>> #define MAC_COEF(d,a,b) MAC64(d,a,b)
>> +#define COEF_MIN -16777215
>> +#define COEF_MAX 16777215
>> typedef int16_t SampleType;
>> typedef int32_t CoefType;
>> typedef int64_t CoefSumType;
>> @@ -235,6 +239,7 @@ typedef struct AC3EncodeContext {
>> void (*deinterleave_input_samples)(struct AC3EncodeContext *s,
>> const SampleType *samples);
>> void (*apply_mdct)(struct AC3EncodeContext *s);
>> + void (*clip_coefficients)(struct AC3EncodeContext *s);
>> void (*apply_channel_coupling)(struct AC3EncodeContext *s);
>> void (*compute_rematrixing_strategy)(struct AC3EncodeContext *s);
>>
>> @@ -289,6 +294,9 @@ void
>> ff_ac3_float_deinterleave_input_samples(AC3EncodeContext *s,
>> void ff_ac3_fixed_apply_mdct(AC3EncodeContext *s);
>> void ff_ac3_float_apply_mdct(AC3EncodeContext *s);
>>
>> +void ff_ac3_fixed_clip_coefficients(AC3EncodeContext *s);
>> +void ff_ac3_float_clip_coefficients(AC3EncodeContext *s);
>> +
>> void ff_ac3_fixed_apply_channel_coupling(AC3EncodeContext *s);
>> void ff_ac3_float_apply_channel_coupling(AC3EncodeContext *s);
>>
>> diff --git a/libavcodec/ac3enc_template.c b/libavcodec/ac3enc_template.c
>> index f6248a8..2e4b01a 100644
>> --- a/libavcodec/ac3enc_template.c
>> +++ b/libavcodec/ac3enc_template.c
>> @@ -107,6 +107,20 @@ void AC3_NAME(apply_mdct)(AC3EncodeContext *s)
>> }
>>
>> +void AC3_NAME(clip_coefficients)(AC3EncodeContext *s)
>> +{
>> +#if CONFIG_AC3ENC_FLOAT
>> +#define vector_clip_coef s->dsp.vector_clipf
>> +#else
>> +#define vector_clip_coef s->dsp.vector_clip_int32
>> +#endif
>> + int chan_size = AC3_MAX_COEFS * AC3_MAX_BLOCKS;
>> + vector_clip_coef(s->mdct_coef_buffer + chan_size,
>> + s->mdct_coef_buffer + chan_size,
>> + COEF_MIN, COEF_MAX, chan_size * s->channels);
>> +}
>
> Would it perhaps be prettier to put this function in the _fixed/float.c
> files instead?
I could go either way with this... I have several other functions to add
that have a similar pattern. The point was to reduce code duplication,
especially in separate files. Another option would be to put the
conditional parts in ac3enc.h.
>> +
>> /**
>> * Calculate a single coupling coordinate.
>> */
>> @@ -161,7 +175,7 @@ void AC3_NAME(apply_channel_coupling)(AC3EncodeContext
>> *s)
>> }
>>
>> /* coefficients must be clipped to +/- 1.0 in order to be encoded */
>> - s->dsp.vector_clipf(cpl_coef, cpl_coef, -1.0f, 1.0f, num_cpl_coefs);
>> + s->dsp.vector_clipf(cpl_coef, cpl_coef, COEF_MIN, COEF_MAX,
>> num_cpl_coefs);
>
> That comment is slightly inaccurate now, but I guess it doesn't really
> matter.
Yeah, slightly. I'll just remove the +/- 1.0 part.
>> /* scale coupling coefficients from float to 24-bit fixed-point */
>> s->ac3dsp.float_to_fixed24(&block->fixed_coef[CPL_CH][cpl_start],
>
> Looks OK otherwise.
I'll send a new patch after comments about the template vs fixed/float
pattern.
Thanks,
Justin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
