[FFmpeg-devel] [PATCH] E-AC-3 spectral extension
Michael Niedermayer
michaelni
Fri Jul 31 21:18:24 CEST 2009
On Sat, Jun 06, 2009 at 05:24:23PM -0400, Justin Ruggles wrote:
> Justin Ruggles wrote:
> > Michael Niedermayer wrote:
> >> On Tue, Jun 02, 2009 at 09:19:23PM -0400, Justin Ruggles wrote:
> >> [...]
> >>> - /* TODO: spectral extension coordinates */
> >>> + /* spectral extension coordinates */
> >>> + if (s->spx_in_use) {
> >>> + for (ch = 1; ch <= fbw_channels; ch++) {
> >>> + if (s->channel_in_spx[ch]) {
> >>> + if (s->first_spx_coords[ch] || get_bits1(gbc)) {
> >>> + int bin;
> >>> + float spx_blend;
> >>> + int master_spx_coord;
> >>> + s->first_spx_coords[ch] = 0;
> >>> + spx_blend = get_bits(gbc, 5) * 0.03125f;
> >>> + master_spx_coord = get_bits(gbc, 2) * 3;
> >>> + bin = s->spx_start_freq;
> >> an empty line in there somewhere would improve readability IMHO
> >
> > fixed.
> >
> >> [...]
> >>> + /* decode spx coordinates */
> >>> + spx_coord_exp = get_bits(gbc, 4);
> >>> + spx_coord_mant = get_bits(gbc, 2);
> >>> + if (spx_coord_exp == 15)
> >>> + spx_coord = spx_coord_mant * 8.0f;
> >>> + else
> >>> + spx_coord = (spx_coord_mant + 4) * 4.0f;
> >>> + spx_coord /= 1 << (spx_coord_exp + master_spx_coord);
> >> something based on the following would avoid the /
> >> spx_coord *= (1<<123) >> (spx_coord_exp + master_spx_coord)
> >>
> >> also *4 can be factored out of the if/else and into the factor above
> >
> > fixed (I think).
> >
> >> [...]
> >>> @@ -66,6 +62,96 @@ typedef enum {
> >>>
> >>> #define EAC3_SR_CODE_REDUCED 3
> >>>
> >>> +void ff_eac3_apply_spectral_extension(AC3DecodeContext *s)
> >>> +{
> >>> + int bin, bnd, ch, i;
> >>> + uint8_t wrapflag[SPX_MAX_BANDS]={0,}, num_copy_sections, copy_sizes[SPX_MAX_BANDS];
> >>> + float rms_energy[SPX_MAX_BANDS];
> >>> +
> >>> + /* Set copy index mapping table. Set wrap flags to apply a notch filter at
> >>> + wrap points later on. */
> >>> + wrapflag[0] = 1;
> >> double initialization
> >
> > fixed.
> >
> >>> + bin = s->spx_copy_start_freq;
> >>> + num_copy_sections = 0;
> >>> + for (bnd = 0; bnd < s->num_spx_bands; bnd++) {
> >>> + int copysize;
> >>> + int bandsize = s->spx_band_sizes[bnd];
> >>> + if ((bin + bandsize) > s->spx_start_freq) {
> >> redundant ()
> >
> > fixed.
> >
> > New patch attached.
> >
> > Thanks,
> > Justin
>
> oops. empty patch...
>
>
> Changelog | 1
> libavcodec/ac3dec.c | 108 ++++++++++++++++++++++++++++++++++++++++++-----
> libavcodec/ac3dec.h | 25 ++++++++++
> libavcodec/ac3dec_data.c | 45 +++++++++++++++++++
> libavcodec/ac3dec_data.h | 2
> libavcodec/eac3dec.c | 106 +++++++++++++++++++++++++++++++++++++++++-----
> 6 files changed, 266 insertions(+), 21 deletions(-)
> 111d70ea4cab08f26a7f8a5b9b1a9738c5fe3e30 eac3_spx_5.diff
> diff --git a/Changelog b/Changelog
> index 677c972..3534886 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -23,6 +23,7 @@ version <next>:
> - QCP demuxer
> - SoX native format muxer and demuxer
> - AMR-NB decoding/encoding, AMR-WB decoding via OpenCORE libraries
> +- spectral extension support in the E-AC-3 decoder
>
>
>
> diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
> index 5feb189..31f650f 100644
> --- a/libavcodec/ac3dec.c
> +++ b/libavcodec/ac3dec.c
> @@ -819,14 +819,95 @@ static int decode_audio_block(AC3DecodeContext *s, int blk)
>
> /* spectral extension strategy */
> if (s->eac3 && (!blk || get_bits1(gbc))) {
> - if (get_bits1(gbc)) {
> - ff_log_missing_feature(s->avctx, "Spectral extension", 1);
> - return -1;
> + s->spx_in_use = get_bits1(gbc);
> + if (s->spx_in_use) {
> + int begf, endf;
these should use descriptive names or some comment that explains what they are
> + int spx_end_subband;
> +
> + /* determine which channels use spx */
> + if (s->channel_mode == AC3_CHMODE_MONO) {
> + s->channel_in_spx[1] = 1;
> + } else {
> + for (ch = 1; ch <= fbw_channels; ch++)
> + s->channel_in_spx[ch] = get_bits1(gbc);
> + }
> +
> + s->spx_copy_start_freq = get_bits(gbc, 2) * 12 + 25;
> + begf = get_bits(gbc, 3);
> + endf = get_bits(gbc, 3);
> + s->spx_start_subband = begf < 6 ? begf+2 : 2*begf-3;
iam not sure, but maybe the following is easier to make sense of:
begf + 2 + (begf >= 6 ? begf-5 : 0);
> + spx_end_subband = endf < 4 ? endf+5 : 2*endf+3;
> + if (s->spx_start_subband >= spx_end_subband) {
> + av_log(s->avctx, AV_LOG_ERROR, "invalid spectral extension range (%d >= %d)\n",
> + s->spx_start_subband, spx_end_subband);
> + return -1;
> + }
would it be possible not to write values in the context and validate them
afterwards?
doing this requires me to proof that the value is not used afterwards
<- more time for doing a review, more chances for bugs ...
als it would need to be documented so that noone mistakely uses invalid values
through a change in the code beliving the values in the contex would
be validted
its really easier to move checks before the storing in the context
[...]
> @@ -66,6 +62,95 @@ typedef enum {
>
> #define EAC3_SR_CODE_REDUCED 3
>
> +void ff_eac3_apply_spectral_extension(AC3DecodeContext *s)
> +{
> + int bin, bnd, ch, i;
> + uint8_t wrapflag[SPX_MAX_BANDS]={1,0,}, num_copy_sections, copy_sizes[SPX_MAX_BANDS];
> + float rms_energy[SPX_MAX_BANDS];
> +
> + /* Set copy index mapping table. Set wrap flags to apply a notch filter at
> + wrap points later on. */
> + bin = s->spx_copy_start_freq;
> + num_copy_sections = 0;
> + for (bnd = 0; bnd < s->num_spx_bands; bnd++) {
> + int copysize;
> + int bandsize = s->spx_band_sizes[bnd];
> + if (bin + bandsize > s->spx_start_freq) {
> + copy_sizes[num_copy_sections++] = bin - s->spx_copy_start_freq;
> + bin = s->spx_copy_start_freq;
> + wrapflag[bnd] = 1;
> + }
> + for (i = 0; i < bandsize; i += copysize) {
> + if (bin == s->spx_start_freq) {
> + copy_sizes[num_copy_sections++] = bin - s->spx_copy_start_freq;
> + bin = s->spx_copy_start_freq;
> + }
> + copysize = FFMIN(bandsize - i, s->spx_start_freq - bin);
> + bin += copysize;
> + }
> + }
> + copy_sizes[num_copy_sections++] = bin - s->spx_copy_start_freq;
> +
> + for (ch = 1; ch <= s->fbw_channels; ch++) {
> + if (!s->channel_in_spx[ch])
> + continue;
> +
> + /* Copy coeffs from normal bands to extension bands */
> + bin = s->spx_start_freq;
> + for (i = 0; i < num_copy_sections; i++) {
> + memcpy(&s->transform_coeffs[ch][bin],
> + &s->transform_coeffs[ch][s->spx_copy_start_freq],
> + copy_sizes[i]*sizeof(float));
> + bin += copy_sizes[i];
> + }
> +
> + /* Calculate RMS energy for each SPX band. */
> + bin = s->spx_start_freq;
> + for (bnd = 0; bnd < s->num_spx_bands; bnd++) {
> + int bandsize = s->spx_band_sizes[bnd];
> + float accum = 0.0f;
> + for (i = 0; i < bandsize; i++) {
> + float coeff = s->transform_coeffs[ch][bin++];
> + accum += coeff * coeff;
> + }
> + rms_energy[bnd] = sqrtf(accum / bandsize);
> + }
> +
> + /* Apply a notch filter at transitions between normal and extension
> + bands and at all wrap points. */
> + if (s->spx_atten_code[ch] >= 0) {
> + const float *atten_tab = ff_eac3_spx_atten_tab[s->spx_atten_code[ch]];
> + bin = s->spx_start_freq - 2;
> + for (bnd = 0; bnd < s->num_spx_bands; bnd++) {
> + if (wrapflag[bnd]) {
> + float *coeffs = &s->transform_coeffs[ch][bin];
> + coeffs[0] *= atten_tab[0];
> + coeffs[1] *= atten_tab[1];
> + coeffs[2] *= atten_tab[2];
> + coeffs[3] *= atten_tab[1];
> + coeffs[4] *= atten_tab[0];
> + }
> + bin += s->spx_band_sizes[bnd];
> + }
> + }
> +
> + /* Apply noise-blended coefficient scaling based on previously
> + calculated RMS energy, blending factors, and SPX coordinates for
> + each band. */
> + bin = s->spx_start_freq;
> + for (bnd = 0; bnd < s->num_spx_bands; bnd++) {
> + float nscale = s->spx_noise_blend[ch][bnd] * rms_energy[bnd] * (1.0f/(1<<31));
> + float sscale = s->spx_signal_blend[ch][bnd];
> + for (i = 0; i < s->spx_band_sizes[bnd]; i++) {
> + float noise = nscale * (int)av_lfg_get(&s->dith_state);
have you considered that int may be 32 and 64 bit on different platforms ?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
No great genius has ever existed without some touch of madness. -- 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/20090731/a23f7931/attachment.pgp>
More information about the ffmpeg-devel
mailing list