[FFmpeg-devel] [PATCH] E-AC-3 decoder, round 3
Michael Niedermayer
michaelni
Mon Aug 25 00:07:56 CEST 2008
On Sun, Aug 24, 2008 at 11:58:23AM -0400, Justin Ruggles wrote:
> Michael Niedermayer wrote:
> > On Tue, Aug 19, 2008 at 09:16:24PM -0400, Justin Ruggles wrote:
> >> Michael Niedermayer wrote:
> >>> On Tue, Aug 19, 2008 at 07:43:31PM -0400, Justin Ruggles wrote:
> >>>> Michael Niedermayer wrote:
> >>>>> On Tue, Aug 19, 2008 at 06:54:35PM -0400, Justin Ruggles wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> Thanks for the review.
> >>>>>>
> >>>>>> Michael Niedermayer wrote:
> >>>>>>> On Sun, Aug 17, 2008 at 07:30:26PM -0400, Justin Ruggles wrote:
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> Here is a new patch to complete support for E-AC-3 decoding within the
> >>>>>>>> current AC-3 decoder. It will be followed up by a cosmetic commit to
> >>>>>>>> indent and align.
> >>>>>>>>
> >>>>>>>> -Justin
> >>>>>>>>
> >>>>>>> [...]
> >>>>>>>> @@ -533,10 +547,27 @@
> >>>>>>>> }
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> +static void get_transform_coeffs_ch(AC3DecodeContext *s, int blk, int ch,
> >>>>>>>> + mant_groups *m)
> >>>>>>>> +{
> >>>>>>>> + if (!s->channel_uses_aht[ch]) {
> >>>>>>>> + ac3_get_transform_coeffs_ch(s, ch, m);
> >>>>>>>> + } else {
> >>>>>>>> + /* if AHT is used, mantissas for all blocks are encoded in the first
> >>>>>>>> + block of the frame. */
> >>>>>>>> + int bin;
> >>>>>>>> + if (!blk)
> >>>>>>>> + ff_eac3_get_transform_coeffs_aht_ch(s, ch);
> >>>>>>> am i blind? or where is this function, i cannot find it in this patch
> >>>>>>> nor in svn
> >>>>>> oops! I forgot to svn add eac3dec.c. I have attached the whole file
> >>>>>> here. It would be applied in the same commit with the rest of these
> >>>>>> changes (minus the part you said to commit separately).
> >>>>> does any of the changes i ok-ed depend on eac3dec.c ?
> >>>>> if not you could commit them and resubmit what is left + eac3dec.c
> >>>> Well, sort of. I could apply all the parts OKed so far, but I would
> >>>> have to comment out the 2 calls to functions which are in eac3dec.c and
> >>>> leave out the part which actually detects the frame as being E-AC-3.
> >>> fine
> >>> every part commited moves us a step closer to full EAC3 support
> >> done. new patch attached.
>
> Updated patch attached with fixes as suggested in ffmpeg-soc mailing list.
>
> Thanks,
> Justin
>
> Index: libavcodec/ac3dec.c
> ===================================================================
> --- libavcodec/ac3dec.c (revision 14945)
> +++ libavcodec/ac3dec.c (working copy)
> @@ -314,12 +314,12 @@
> s->first_cpl_leak = 0;
> s->dba_syntax = 1;
> s->skip_syntax = 1;
> + memset(s->cpl_strategy_exists, 0, sizeof(s->cpl_strategy_exists));
> memset(s->channel_uses_aht, 0, sizeof(s->channel_uses_aht));
> return ac3_parse_header(s);
> } else {
> - /*s->eac3 = 1;
> - return ff_eac3_parse_header(s);*/
> - return AC3_PARSE_ERROR_BSID;
> + s->eac3 = 1;
> + return ff_eac3_parse_header(s);
> }
> }
>
ok
> @@ -443,7 +443,7 @@
> * Get the transform coefficients for a particular channel
> * reference: Section 7.3 Quantization and Decoding of Mantissas
> */
> -static void get_transform_coeffs_ch(AC3DecodeContext *s, int ch_index, mant_groups *m)
> +static void ac3_get_transform_coeffs_ch(AC3DecodeContext *s, int ch_index, mant_groups *m)
> {
> GetBitContext *gbc = &s->gbc;
> int i, gcode, tbap, start, end;
> @@ -548,7 +548,6 @@
> }
> }
>
> -#if 0
> static void get_transform_coeffs_ch(AC3DecodeContext *s, int blk, int ch,
> mant_groups *m)
> {
> @@ -565,12 +564,11 @@
> }
> }
> }
> -#endif
IMHO its messy to call get_transform_coeffs_ch() for every block but
then ignore all but the first.
but i see no alternative so ok
>
> /**
> * Get the transform coefficients.
> */
> -static void get_transform_coeffs(AC3DecodeContext *s)
> +static void get_transform_coeffs(AC3DecodeContext *s, int blk)
id name it decode_... but thats nitpicking, feel free to ignore me ...
> {
> int ch, end;
> int got_cplchan = 0;
> @@ -580,12 +578,12 @@
>
> for (ch = 1; ch <= s->channels; ch++) {
> /* transform coefficients for full-bandwidth channel */
> - get_transform_coeffs_ch(s, ch, &m);
> + get_transform_coeffs_ch(s, blk, ch, &m);
> /* tranform coefficients for coupling channel come right after the
> coefficients for the first coupled channel*/
> if (s->channel_in_cpl[ch]) {
> if (!got_cplchan) {
> - get_transform_coeffs_ch(s, CPL_CH, &m);
> + get_transform_coeffs_ch(s, blk, CPL_CH, &m);
> calc_transform_coeffs_cpl(s);
> got_cplchan = 1;
> }
ok
> @@ -773,7 +771,7 @@
> /* TODO: spectral extension coordinates */
>
> /* coupling strategy */
> - if (get_bits1(gbc)) {
> + if (s->cpl_strategy_exists[blk] || (!s->eac3 && get_bits1(gbc))) {
if(s->eac3 ? s->cpl_strategy_exists[blk] : get_bits1(gbc))
is clearer IMHO, this also may be usefull to simplify other similar
cases
note if above works then the init of cpl_strategy_exists may become
unneeded for AC3
> memset(bit_alloc_stages, 3, AC3_MAX_CHANNELS);
> if (!s->eac3)
> s->cpl_in_use[blk] = get_bits1(gbc);
> @@ -824,6 +822,9 @@
> s->cpl_band_struct[bnd] = get_bits1(gbc);
> }
> } else if (!blk) {
> + memcpy(s->cpl_band_struct,
> + &ff_eac3_default_cpl_band_struct[cpl_begin_freq+1],
> + s->num_cpl_subbands-1);
> }
> s->cpl_band_struct[s->num_cpl_subbands-1] = 0;
>
ok
> @@ -856,7 +857,17 @@
>
> for (ch = 1; ch <= fbw_channels; ch++) {
> if (s->channel_in_cpl[ch]) {
> - if (get_bits1(gbc)) {
> + int new_cpl_coords = 0;
> +
> + /* determine if coupling coordinates are new or reused */
> + if (s->eac3 && s->first_cpl_coords[ch]) {
> + new_cpl_coords = 1;
> + s->first_cpl_coords[ch] = 0;
> + } else {
> + new_cpl_coords = get_bits1(gbc);
> + }
> +
> + if (new_cpl_coords) {
if(s->eac3 ? s->first_cpl_coords[ch] : get_bits1(gbc)){
s->first_cpl_coords[ch] = 0;
[...]
> @@ -898,10 +912,9 @@
> }
>
> /* exponent strategies for each channel */
> - s->exp_strategy[blk][CPL_CH] = EXP_REUSE;
> - s->exp_strategy[blk][s->lfe_ch] = EXP_REUSE;
> for (ch = !cpl_in_use; ch <= s->channels; ch++) {
> - s->exp_strategy[blk][ch] = get_bits(gbc, 2 - (ch == s->lfe_ch));
> + if (!s->eac3)
> + s->exp_strategy[blk][ch] = get_bits(gbc, 2 - (ch == s->lfe_ch));
> if(s->exp_strategy[blk][ch] != EXP_REUSE)
> bit_alloc_stages[ch] = 3;
> }
ok
[...]
> @@ -1075,7 +1113,7 @@
>
> /* unpack the transform coefficients
> this also uncouples channels if coupling is in use. */
> - get_transform_coeffs(s);
> + get_transform_coeffs(s, blk);
>
> /* TODO: generate enhanced coupling coordinates and uncouple */
>
ok
> Index: libavcodec/ac3dec.h
> ===================================================================
> --- libavcodec/ac3dec.h (revision 14945)
> +++ libavcodec/ac3dec.h (working copy)
> @@ -82,7 +82,7 @@
> int phase_flags[18]; ///< phase flags (phsflg)
> int num_cpl_subbands; ///< number of coupling sub bands (ncplsubnd)
> int num_cpl_bands; ///< number of coupling bands (ncplbnd)
> - int cpl_band_struct[18]; ///< coupling band structure (cplbndstrc)
> + uint8_t cpl_band_struct[18]; ///< coupling band structure (cplbndstrc)
> int firstchincpl; ///< first channel in coupling
> int first_cpl_coords[AC3_MAX_CHANNELS]; ///< first coupling coordinates states (firstcplcos)
> int cpl_coords[AC3_MAX_CHANNELS][18]; ///< coupling coordinates (cplco)
> @@ -168,4 +168,16 @@
> ///@}
> } AC3DecodeContext;
>
> +/**
> + * Parse the E-AC-3 frame header.
> + * This parses both the bit stream info and audio frame header.
> + */
> +int ff_eac3_parse_header(AC3DecodeContext *s);
> +
> +/**
> + * Decode mantissas in a single channel for the entire frame.
> + * This is used when AHT mode is enabled.
> + */
> +void ff_eac3_get_transform_coeffs_aht_ch(AC3DecodeContext *s, int ch);
> +
> #endif /* FFMPEG_AC3DEC_H */
ok
> Index: libavcodec/Makefile
> ===================================================================
> --- libavcodec/Makefile (revision 14945)
> +++ libavcodec/Makefile (working copy)
> @@ -27,7 +27,7 @@
>
> OBJS-$(CONFIG_AAC_DECODER) += aac.o aactab.o mdct.o fft.o
> OBJS-$(CONFIG_AASC_DECODER) += aasc.o
> -OBJS-$(CONFIG_AC3_DECODER) += ac3dec.o ac3tab.o ac3dec_data.o ac3.o mdct.o fft.o
> +OBJS-$(CONFIG_AC3_DECODER) += eac3dec.o ac3dec.o ac3tab.o ac3dec_data.o ac3.o mdct.o fft.o
> OBJS-$(CONFIG_AC3_ENCODER) += ac3enc.o ac3tab.o ac3.o
> OBJS-$(CONFIG_ALAC_DECODER) += alac.o
> OBJS-$(CONFIG_ALAC_ENCODER) += alacenc.o lpc.o
> Index: libavcodec/eac3dec.c
> ===================================================================
> --- libavcodec/eac3dec.c (revision 14945)
> +++ libavcodec/eac3dec.c (working copy)
> @@ -78,3 +78,409 @@
> pre_mant[4] = even1 - odd1;
> pre_mant[5] = even0 - odd0;
> }
> +
> +void ff_eac3_get_transform_coeffs_aht_ch(AC3DecodeContext *s, int ch)
i think decode is a better word than get, this probably applies
to a few other functions.
> +{
> + int bin, blk, gs;
> + int end_bap, gaq_mode;
> + GetBitContext *gbc = &s->gbc;
> + int gaq_gain[AC3_MAX_COEFS];
> +
> + gaq_mode = get_bits(gbc, 2);
> + end_bap = (gaq_mode < 2) ? 12 : 17;
> +
> + /* if GAQ gain is used, decode gain codes for bins with hebap between
> + 8 and end_bap */
> + gs = 0;
> + if (gaq_mode == EAC3_GAQ_12 || gaq_mode == EAC3_GAQ_14) {
> + /* read 1-bit GAQ gain codes */
> + for (bin = s->start_freq[ch]; bin < s->end_freq[ch]; bin++) {
> + if (s->bap[ch][bin] > 7 && s->bap[ch][bin] < end_bap)
> + gaq_gain[gs++] = get_bits1(gbc) << (gaq_mode-1);
> + }
> + } else if (gaq_mode == EAC3_GAQ_124) {
> + /* read 1.67-bit GAQ gain codes (3 codes in 5 bits) */
> + int gc = 2;
> + for (bin = s->start_freq[ch]; bin < s->end_freq[ch]; bin++) {
> + if (s->bap[ch][bin] > 7 && s->bap[ch][bin] < end_bap) {
> + if (gc++ == 2) {
> + int group_gain = get_bits(gbc, 5);
> + if (group_gain > 26) {
> + av_log(s->avctx, AV_LOG_WARNING, "GAQ gain value out-of-range\n");
> + group_gain = 26;
> + }
> + gaq_gain[gs++] = ff_ac3_ungroup_3_in_5_bits_tab[group_gain][0];
> + gaq_gain[gs++] = ff_ac3_ungroup_3_in_5_bits_tab[group_gain][1];
> + gaq_gain[gs++] = ff_ac3_ungroup_3_in_5_bits_tab[group_gain][2];
group_gain is not the gain of the group, its a index into a table of gain
triples. it seems the name is misleading
> + gc = 0;
> + }
> + }
> + }
> + }
> +
> + gs=0;
> + for (bin = s->start_freq[ch]; bin < s->end_freq[ch]; bin++) {
> + int hebap = s->bap[ch][bin];
what does hebap mean?
ive by now figured out bap are band types, and iam not sure why i didnt
suggest yet that they could be renamed to band_type[][]
but i have no faint clue why they are called hebap here instead of bap ...
> + int bits = ff_eac3_bits_vs_hebap[hebap];
> + if (!hebap) {
> + /* zero-mantissa dithering */
> + for (blk = 0; blk < 6; blk++) {
> + s->pre_mantissa[ch][bin][blk] = (av_lfg_get(&s->dith_state) & 0x7FFFFF) - 0x400000;
> + }
> + } else if (hebap < 8) {
> + /* Vector Quantization */
> + int v = get_bits(gbc, bits);
> + for (blk = 0; blk < 6; blk++) {
> + s->pre_mantissa[ch][bin][blk] = ff_eac3_vq_hebap[hebap][v][blk] << 8;
ff_eac3_vq_hebap is a VQ tables of coefficient mantissas if i understand
correctly, why is this not reflected in its name?
ff_eac3_mantissa_vq
ff_eac3_coeff_mantissa_vq
seem better names to me
> + }
> + } else {
> + /* Gain Adaptive Quantization */
> + int gbits, log_gain;
> + if (gaq_mode != EAC3_GAQ_NO && hebap < end_bap) {
> + log_gain = gaq_gain[gs++];
> + } else {
> + log_gain = 0;
> + }
> + gbits = bits - log_gain;
> +
> + for (blk = 0; blk < 6; blk++) {
> + int mant = get_sbits(gbc, gbits);
> + if (mant == -(1 << (gbits-1))) {
> + /* large mantissa */
> + int b;
> + mant = get_sbits(gbc, bits-2+log_gain) << (26-log_gain-bits);
> + /* remap mantissa value to correct for asymmetric quantization */
> + if (mant >= 0)
> + b = 32768 >> log_gain;
> + else
> + b = ff_eac3_gaq_remap_2_4_b[hebap-8][log_gain-1];
isnt the first of the 2 missing a >>8 correction? or did i miss it?
[...]
> +
> +int ff_eac3_parse_header(AC3DecodeContext *s)
> +{
> + int i, blk, ch;
> + int ac3_exponent_strategy, parse_aht_info, parse_spx_atten_data;
can you explain the difference between _info and _data variables?
[..]
> + /* skip mixing configuration information */
> + if (get_bits1(gbc)) {
> + if (s->num_blocks == 1) {
> + skip_bits(gbc, 5);
> + } else {
> + for (blk = 0; blk < s->num_blocks; blk++) {
> + if (get_bits1(gbc)) {
> + skip_bits(gbc, 5);
> + }
> + }
> + }
for (blk = 0; blk < s->num_blocks; blk++) {
if (s->num_blocks == 1 || get_bits1(gbc)) {
skip_bits(gbc, 5);
}
}
[...]
> +
> + /* additional bitstream info */
> + if (get_bits1(gbc)) {
> + int addbsil = get_bits(gbc, 6);
> + for (i = 0; i < addbsil + 1; i++) {
> + skip_bits(gbc, 8); // skip additional bit stream info
> + }
> + }
> +
> + /* audio frame syntax flags, strategy data, and per-frame data */
> +
> + if (s->num_blocks == 6) {
> + ac3_exponent_strategy = get_bits1(gbc);
> + parse_aht_info = get_bits1(gbc);
vertical align
> + } else {
> + /* less than 6 blocks, so use AC-3-style exponent strategy syntax, and
> + do not use AHT */
> + ac3_exponent_strategy = 1;
> + parse_aht_info = 0;
> + }
> +
> + s->snr_offset_strategy = get_bits(gbc, 2);
> + parse_transient_proc_info = get_bits1(gbc);
> +
> + s->block_switch_syntax = get_bits1(gbc);
> + if (!s->block_switch_syntax)
> + memset(s->block_switch, 0, sizeof(s->block_switch));
> +
> + s->dither_flag_syntax = get_bits1(gbc);
> + if (!s->dither_flag_syntax) {
> + s->dither_all = 1;
is this variable really needed?
[...]
> Index: libavcodec/ac3dec_data.c
> ===================================================================
> --- libavcodec/ac3dec_data.c (revision 14945)
> +++ libavcodec/ac3dec_data.c (working copy)
> @@ -87,18 +87,19 @@
> /**
> * Table E3.6, Gk=2 & Gk=4, B
> * Large mantissa inverse quantization, negative mantissa remapping offsets
> + * Table values from the spec are right-shifted by 8 to simplify calculations.
> * ff_eac3_gaq_remap_3_4_b[hebap-8][Gk=2,4]
> */
> const int16_t ff_eac3_gaq_remap_2_4_b[9][2] = {
> - { -5461, -1170},
> - { -11703, -4915},
> - { -14199, -6606},
> - { -15327, -7412},
> - { -15864, -7805},
> - { -16126, -7999},
> - { -16255, -8096},
> - { -16320, -8144},
> - { -16352, -8168}
> + { -22, -5 },
> + { -46, -20 },
> + { -56, -26 },
> + { -60, -29 },
> + { -62, -31 },
> + { -63, -32 },
> + { -64, -32 },
> + { -64, -32 },
> + { -64, -32 },
> };
>
ok, btw the table fits in int8_t now
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- 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/20080825/2337e028/attachment.pgp>
More information about the ffmpeg-devel
mailing list