[FFmpeg-devel] [PATCH 1/4] libavcodec: Implementation of AC3 fixed point decoder

Michael Niedermayer michaelni at gmx.at
Thu Oct 10 21:01:41 CEST 2013


On Wed, Sep 18, 2013 at 03:38:30PM +0200, Nedeljko Babic wrote:
[...]
> index 20b4b61..6427840 100644
> --- a/libavcodec/ac3dec.c
> +++ b/libavcodec/ac3dec.c
> @@ -64,7 +64,7 @@ static const uint8_t quantization_tab[16] = {
>  static float dynamic_range_tab[256];
>  
>  /** Adjustments in dB gain */
> -static const float gain_levels[9] = {
> +static const INTFLOAT AC3_RENAME(gain_levels)[9] = {
>      LEVEL_PLUS_3DB,
>      LEVEL_PLUS_1POINT5DB,
>      LEVEL_ONE,
> @@ -168,14 +168,22 @@ static av_cold int ac3_decode_init(AVCodecContext *avctx)
>      ac3_tables_init();
>      ff_mdct_init(&s->imdct_256, 8, 1, 1.0);
>      ff_mdct_init(&s->imdct_512, 9, 1, 1.0);
> -    ff_kbd_window_init(s->window, 5.0, 256);
> +    AC3_RENAME(ff_kbd_window_init)(s->window, 5.0, 256);
>      ff_dsputil_init(&s->dsp, avctx);
> +#if CONFIG_AC3_FIXED
> +    avpriv_fixed_dsp_init(&s->fdsp, avctx->flags & CODEC_FLAG_BITEXACT);
> +#else
>      avpriv_float_dsp_init(&s->fdsp, avctx->flags & CODEC_FLAG_BITEXACT);
> +#endif
>      ff_ac3dsp_init(&s->ac3dsp, avctx->flags & CODEC_FLAG_BITEXACT);
>      ff_fmt_convert_init(&s->fmt_conv, avctx);
>      av_lfg_init(&s->dith_state, 0);
>  

> +#if CONFIG_AC3_FIXED
> +    avctx->sample_fmt = AV_SAMPLE_FMT_S16P;
> +#else
>      avctx->sample_fmt = AV_SAMPLE_FMT_FLTP;
> +#endif

if (CONFIG_AC3_FIXED)
can be used here, no need for ifdefs


[...]
> @@ -322,22 +330,23 @@ static void set_downmix_coeffs(AC3DecodeContext *s)
>      }
>  
>      /* renormalize */
> -    norm0 = norm1 = 0.0;
> +    norm0 = norm1 = FIXR(0.0);
>      for (i = 0; i < s->fbw_channels; i++) {
>          norm0 += s->downmix_coeffs[i][0];
>          norm1 += s->downmix_coeffs[i][1];
>      }
> -    norm0 = 1.0f / norm0;
> -    norm1 = 1.0f / norm1;
> +
> +    norm0 = AC3_NORM(norm0);
> +    norm1 = AC3_NORM(norm1);
> +
>      for (i = 0; i < s->fbw_channels; i++) {
> -        s->downmix_coeffs[i][0] *= norm0;
> -        s->downmix_coeffs[i][1] *= norm1;
> +        s->downmix_coeffs[i][0] = (SHORTFLOAT)AC3_MUL(s->downmix_coeffs[i][0],norm0);
> +        s->downmix_coeffs[i][1] = (SHORTFLOAT)AC3_MUL(s->downmix_coeffs[i][1],norm1);
>      }
>  
>      if (s->output_mode == AC3_CHMODE_MONO) {
>          for (i = 0; i < s->fbw_channels; i++)
> -            s->downmix_coeffs[i][0] = (s->downmix_coeffs[i][0] +
> -                                       s->downmix_coeffs[i][1]) * LEVEL_MINUS_3DB;
> +            s->downmix_coeffs[i][0] = AC3_LEVEL(s->downmix_coeffs[i][0] + s->downmix_coeffs[i][1]);
>      }
>  }

i think this function has no speed relevance
and a simple
for all
    downmix_coeffs_fixed[x][y] = FIXR(downmix_coeffs[x][y]);

should work as well


>  
> @@ -601,25 +610,66 @@ static inline void do_imdct(AC3DecodeContext *s, int channels)
>      for (ch = 1; ch <= channels; ch++) {
>          if (s->block_switch[ch]) {
>              int i;
> -            float *x = s->tmp_output + 128;
> +            FFTSample *x = s->tmp_output + 128;
>              for (i = 0; i < 128; i++)
>                  x[i] = s->transform_coeffs[ch][2 * i];
>              s->imdct_256.imdct_half(&s->imdct_256, s->tmp_output, x);
> +#if CONFIG_AC3_FIXED
> +            s->fdsp.vector_fmul_window_fixed_scaled(s->outptr[ch - 1], s->delay[ch - 1],
> +                                       s->tmp_output, s->window, 128, 8);
> +#else
>              s->fdsp.vector_fmul_window(s->outptr[ch - 1], s->delay[ch - 1],
>                                         s->tmp_output, s->window, 128);
> +#endif
>              for (i = 0; i < 128; i++)
>                  x[i] = s->transform_coeffs[ch][2 * i + 1];
>              s->imdct_256.imdct_half(&s->imdct_256, s->delay[ch - 1], x);
>          } else {
>              s->imdct_512.imdct_half(&s->imdct_512, s->tmp_output, s->transform_coeffs[ch]);
> +#if CONFIG_AC3_FIXED
> +            s->fdsp.vector_fmul_window_fixed_scaled(s->outptr[ch - 1], s->delay[ch - 1],
> +                                       s->tmp_output, s->window, 128, 8);
> +#else
>              s->fdsp.vector_fmul_window(s->outptr[ch - 1], s->delay[ch - 1],
>                                         s->tmp_output, s->window, 128);
> -            memcpy(s->delay[ch - 1], s->tmp_output + 128, 128 * sizeof(float));
> +#endif
> +            memcpy(s->delay[ch - 1], s->tmp_output + 128, 128 * sizeof(FFTSample));
>          }
>      }
>  }

why do these use #ifdefs and dont just use fixed point implementations
of the existing vector_fmul_window API ?



>  
>  /**
> + * Downmix samples from original signal to stereo or mono (this is for 16-bit samples
> + * and fixed point decoder - original (for 32-bit samples) is in ac3dsp.c).
> + */
> +#if CONFIG_AC3_FIXED
> +static void ac3_downmix_c_fixed16(int16_t **samples, int16_t (*matrix)[2],
> +                      int out_ch, int in_ch, int len)
> +{
> +    int i, j;
> +    int v0, v1;
> +    if (out_ch == 2) {
> +        for (i = 0; i < len; i++) {
> +            v0 = v1 = 0;
> +            for (j = 0; j < in_ch; j++) {
> +                v0 += samples[j][i] * matrix[j][0];
> +                v1 += samples[j][i] * matrix[j][1];
> +            }
> +            samples[0][i] = (v0+2048)>>12;
> +            samples[1][i] = (v1+2048)>>12;
> +        }
> +    } else if (out_ch == 1) {
> +        for (i = 0; i < len; i++) {
> +            v0 = 0;
> +            for (j = 0; j < in_ch; j++)
> +                v0 += samples[j][i] * matrix[j][0];
> +            samples[0][i] = (v0+2048)>>12;
> +        }
> +    }
> +}
> +#endif

the #if doesnt seem needed


> +
> +/**
>   * Upmix delay samples from stereo to original channel layout.
>   */
>  static void ac3_upmix_delay(AC3DecodeContext *s)
> @@ -747,10 +797,9 @@ static int decode_audio_block(AC3DecodeContext *s, int blk)
>      i = !s->channel_mode;
>      do {
>          if (get_bits1(gbc)) {
> -            s->dynamic_range[i] = ((dynamic_range_tab[get_bits(gbc, 8)] - 1.0) *
> -                                  s->drc_scale) + 1.0;
> +            s->dynamic_range[i] = AC3_DYNAMIC_RANGE(get_bits(gbc, 8));
>          } else if (blk == 0) {
> -            s->dynamic_range[i] = 1.0f;
> +            s->dynamic_range[i] = AC3_DYNAMIC_RANGE1;
>          }
>      } while (i--);
>  

> @@ -776,6 +825,10 @@ static int decode_audio_block(AC3DecodeContext *s, int blk)
>              if (start_subband > 7)
>                  start_subband += start_subband - 7;
>              end_subband    = get_bits(gbc, 3) + 5;
> +#if CONFIG_AC3_FIXED
> +            s->spx_dst_end_freq = end_freq_inv_tab[end_subband];
> +            end_subband += 5;
> +#endif
>              if (end_subband   > 7)
>                  end_subband   += end_subband   - 7;
>              dst_start_freq = dst_start_freq * 12 + 25;
> @@ -796,7 +849,9 @@ static int decode_audio_block(AC3DecodeContext *s, int blk)
>  
>              s->spx_dst_start_freq = dst_start_freq;
>              s->spx_src_start_freq = src_start_freq;
> +#if !CONFIG_AC3_FIXED
>              s->spx_dst_end_freq   = dst_end_freq;
> +#endif
>  
>              decode_band_structure(gbc, blk, s->eac3, 0,
>                                    start_subband, end_subband,

why is a variable with one and the same name used to represet
different things between fixed and float versions ?



[...]
> @@ -1203,6 +1288,17 @@ static int decode_audio_block(AC3DecodeContext *s, int blk)
>      if (s->channel_mode == AC3_CHMODE_STEREO)
>          do_rematrixing(s);
>  
> +#if CONFIG_AC3_FIXED
> +    for (ch = 1; ch <= s->channels; ch++) {
> +        int gain;
> +        if(s->channel_mode == AC3_CHMODE_DUALMONO) {
> +            gain = s->dynamic_range[2-ch];
> +        } else {
> +            gain = s->dynamic_range[0];
> +        }
> +        scale_coefs(s->transform_coeffs[ch], s->fixed_coeffs[ch], gain, 256);
> +    }
> +#else
>      /* apply scaling to coefficients (headroom, dynrng) */
>      for (ch = 1; ch <= s->channels; ch++) {
>          float gain = 1.0 / 4194304.0f;
> @@ -1214,6 +1310,7 @@ static int decode_audio_block(AC3DecodeContext *s, int blk)
>          s->fmt_conv.int32_to_float_fmul_scalar(s->transform_coeffs[ch],
>                                                 s->fixed_coeffs[ch], gain, 256);
>      }
> +#endif

the if() can be factored out
gain can be changed to INTFLOAT


>  
>      /* apply spectral extension to high frequency bins */
>      if (s->spx_in_use && CONFIG_EAC3_DECODER) {

    > @@ -1235,23 +1332,26 @@ static int decode_audio_block(AC3DecodeContext *s, int blk)
>          }
>  
>          do_imdct(s, s->channels);
> -
>          if (downmix_output) {

unrelated


> +#if CONFIG_AC3_FIXED
> +            ac3_downmix_c_fixed16(s->outptr, s->downmix_coeffs,
> +                              s->out_channels, s->fbw_channels, 256);
> +#else
>              s->ac3dsp.downmix(s->outptr, s->downmix_coeffs,
>                                s->out_channels, s->fbw_channels, 256);
> +#endif

why is s->ac3dsp.downmix not set to ac3_downmix_c_fixed16 ?


[...]
> +#include "ac3dec.c"
> +
> +static const AVOption options[] = {
> +    { "drc_scale", "percentage of dynamic range compression to apply", OFFSET(drc_scale), AV_OPT_TYPE_INT, {.i64 = 1}, 0, 1, PAR },
> +
> +{"dmix_mode", "Preferred Stereo Downmix Mode", OFFSET(preferred_stereo_downmix), AV_OPT_TYPE_INT, {.i64 = -1 }, -1, 2, 0, "dmix_mode"},
> +{"ltrt_cmixlev",   "Lt/Rt Center Mix Level",   OFFSET(ltrt_center_mix_level),    AV_OPT_TYPE_INT, {.i64 = -1 }, -1, 2, 0},
> +{"ltrt_surmixlev", "Lt/Rt Surround Mix Level", OFFSET(ltrt_surround_mix_level),  AV_OPT_TYPE_INT, {.i64 = -1 }, -1, 2, 0},
> +{"loro_cmixlev",   "Lo/Ro Center Mix Level",   OFFSET(loro_center_mix_level),    AV_OPT_TYPE_INT, {.i64 = -1 }, -1, 2, 0},
> +{"loro_surmixlev", "Lo/Ro Surround Mix Level", OFFSET(loro_surround_mix_level),  AV_OPT_TYPE_INT, {.i64 = -1 }, -1, 2, 0},

in the float decoder these have a range from -1.0 to 2.0 or 0.0 to
1.0 and here they are integers with range from 0 to 1 and -1 to 2

that dramatic loss of precission looks odd


[...]
> diff --git a/libavcodec/kbdwin.c b/libavcodec/kbdwin.c
> index 5a62e9d..c6cdabc 100644
> --- a/libavcodec/kbdwin.c
> +++ b/libavcodec/kbdwin.c
> @@ -45,3 +45,26 @@ av_cold void ff_kbd_window_init(float *window, float alpha, int n)
>     for (i = 0; i < n; i++)
>         window[i] = sqrt(local_window[i] / sum);
>  }
> +
> +av_cold void ff_kbd_window_init_fixed(int32_t *window, float alpha, int n)
> +{
> +    int i, j;
> +    double sum = 0.0, bessel, tmp;
> +    double local_window[FF_KBD_WINDOW_MAX];
> +    double alpha2 = (alpha * M_PI / n) * (alpha * M_PI / n);
> +
> +    av_assert0(n <= FF_KBD_WINDOW_MAX);
> +
> +    for (i = 0; i < n; i++) {
> +        tmp = i * (n - i) * alpha2;
> +        bessel = 1.0;
> +        for (j = BESSEL_I0_ITER; j > 0; j--)
> +            bessel = bessel * tmp / (j * j) + 1;
> +        sum += bessel;
> +        local_window[i] = sum;
> +    }
> +
> +    sum++;
> +    for (i = 0; i < n; i++)
> +       window[i] = (int)floor(2147483648.0 * sqrt(local_window[i] / sum) + 0.5);
> +}

this can be implemneted by caling the existing function and converting
its output to int instead of duplicating the code


[...]
> +static av_always_inline int fixed_sqrt(int x, int bits)

why isnt ff_sqrt() used ?

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131010/f9152f03/attachment.asc>


More information about the ffmpeg-devel mailing list