[FFmpeg-devel] [PATCH] E-AC-3 decoder, round 3
Michael Niedermayer
michaelni
Sun Aug 31 03:27:10 CEST 2008
On Sat, Aug 30, 2008 at 07:30:55PM -0400, Justin Ruggles wrote:
> Michael Niedermayer wrote:
> > On Thu, Aug 28, 2008 at 10:51:26PM -0400, Justin Ruggles wrote:
> >> Michael Niedermayer wrote:
> >>> 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.
> >> New patch attached.
> >>
> >> -Justin
> >>
> >
> >> Index: libavcodec/ac3dec.c
> >> ===================================================================
> >> --- libavcodec/ac3dec.c (revision 15015)
> >> +++ libavcodec/ac3dec.c (working copy)
> [...]
> >> @@ -769,7 +766,7 @@
> >> /* TODO: spectral extension coordinates */
> >>
> >> /* coupling strategy */
> >> - if (get_bits1(gbc)) {
> >> + if (s->eac3 ? s->cpl_strategy_exists[blk] : get_bits1(gbc)) {
> >> memset(bit_alloc_stages, 3, AC3_MAX_CHANNELS);
> >> if (!s->eac3)
> >> s->cpl_in_use[blk] = get_bits1(gbc);
> >> @@ -855,8 +852,9 @@
> >>
> >> for (ch = 1; ch <= fbw_channels; ch++) {
> >> if (s->channel_in_cpl[ch]) {
> >> - if (get_bits1(gbc)) {
> >> + if ((s->eac3 && s->first_cpl_coords[ch]) || get_bits1(gbc)) {
> >> int master_cpl_coord, cpl_coord_exp, cpl_coord_mant;
> >> + s->first_cpl_coords[ch] = 0;
> >> cpl_coords_exist = 1;
> >> master_cpl_coord = 3 * get_bits(gbc, 2);
> >> for (bnd = 0; bnd < s->num_cpl_bands; bnd++) {
> >> @@ -872,6 +870,9 @@
> >> av_log(s->avctx, AV_LOG_ERROR, "new coupling coordinates must be present in block 0\n");
> >> return -1;
> >> }
> >> + } else {
> >> + /* channel not in coupling */
> >> + s->first_cpl_coords[ch] = 1;
> >> }
> >
> > ok
>
> applied.
>
> >
> >> }
> >> /* phase flags */
> >> @@ -960,18 +961,35 @@
> >> }
> >>
> >> /* signal-to-noise ratio offsets and fast gains (signal-to-mask ratios) */
> >> - if (get_bits1(gbc)) {
> >> + if(!s->eac3 || !blk){
> >> + if(s->snr_offset_strategy && get_bits1(gbc)) {
> >> + int snr = 0;
> >> int csnr;
> >> csnr = (get_bits(gbc, 6) - 15) << 4;
> >> - for (ch = !cpl_in_use; ch <= s->channels; ch++) { /* snr offset and fast gain */
> >> - s->snr_offset[ch] = (csnr + get_bits(gbc, 4)) << 2;
> >> + for (i = ch = !cpl_in_use; ch <= s->channels; ch++) {
> >> + /* snr offset */
> >> + if (ch == i || s->snr_offset_strategy == 2)
> >> + snr = (csnr + get_bits(gbc, 4)) << 2;
> >> + /* run at least last bit allocation stage if snr offset changes */
> >> + if(blk && s->snr_offset[ch] != snr) {
> >> + bit_alloc_stages[ch] = FFMAX(bit_alloc_stages[ch], 1);
> >> + }
> >> + s->snr_offset[ch] = snr;
> >> +
> >> + /* fast gain (normal AC-3 only) */
> >> + if (!s->eac3) {
> >> + int prev = s->fast_gain[ch];
> >> s->fast_gain[ch] = ff_ac3_fast_gain_tab[get_bits(gbc, 3)];
> >> - }
> >> - memset(bit_alloc_stages, 3, AC3_MAX_CHANNELS);
> >> - } else if (!blk) {
> >> + /* run last 2 bit allocation stages if fast gain changes */
> >> + if(blk && prev != s->fast_gain[ch])
> >> + bit_alloc_stages[ch] = FFMAX(bit_alloc_stages[ch], 2);
> >> + }
> >> + }
> >
> >> + } else if (!s->eac3 && !blk) {
> >
> > this can never be true, as its truth implicates the if() above being
> > true
>
> I don't see it. Maybe the indentation made it hard to read. I've changed
> it for the new patch. Let me know if I'm just missing something here...
ive just been confused by the indention ...
>
> >
> >> av_log(s->avctx, AV_LOG_ERROR, "new snr offsets must be present in block 0\n");
> >> return -1;
> >> }
> >> + }
> >>
> >> /* fast gain (E-AC-3 only) */
> >> if (s->fast_gain_syntax && get_bits1(gbc)) {
> >> @@ -994,14 +1012,22 @@
> >>
> >> /* coupling leak information */
> >> if (cpl_in_use) {
> >> - if (get_bits1(gbc)) {
> >> - s->bit_alloc_params.cpl_fast_leak = get_bits(gbc, 3);
> >> - s->bit_alloc_params.cpl_slow_leak = get_bits(gbc, 3);
> >> + if (s->first_cpl_leak || get_bits1(gbc)) {
> >> + int fl = get_bits(gbc, 3);
> >> + int sl = get_bits(gbc, 3);
> >> + /* run last 2 bit allocation stages for coupling channel if
> >> + coupling leak changes */
> >
> >> + if(blk && (fl != s->bit_alloc_params.cpl_fast_leak ||
> >> + sl != s->bit_alloc_params.cpl_slow_leak)) {
> >
> > these can be vertically aligned
>
> fixed.
>
> >
> >> bit_alloc_stages[CPL_CH] = FFMAX(bit_alloc_stages[CPL_CH], 2);
> >> - } else if (!blk) {
> >> + }
> >> + s->bit_alloc_params.cpl_fast_leak = fl;
> >> + s->bit_alloc_params.cpl_slow_leak = sl;
> >> + } else if (!s->eac3 && !blk) {
> >> av_log(s->avctx, AV_LOG_ERROR, "new coupling leak info must be present in block 0\n");
> >> return -1;
> >> }
> >> + s->first_cpl_leak = 0;
> >> }
> >>
> >> /* delta bit allocation information */
> >> Index: libavcodec/Makefile
> >> ===================================================================
> >> --- libavcodec/Makefile (revision 15015)
> >> +++ 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 15015)
> >> +++ libavcodec/eac3dec.c (working copy)
> >> @@ -78,3 +78,414 @@
> >> pre_mant[4] = even1 - odd1;
> >> pre_mant[5] = even0 - odd0;
> >> }
> >> +
> >> +void ff_eac3_decode_transform_coeffs_aht_ch(AC3DecodeContext *s, int ch)
> >> +{
> >> + 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++) {
> >
> > ok
> >
> >
> >> + if (s->bap[ch][bin] > 7 && s->bap[ch][bin] < end_bap) {
> >
> > end_bap is guranteed to be 17 here, ok otherwise
>
> fixed and applied.
>
> >
> >> [remainder of the file]
> >
> > ok
>
> applied.
>
> new patch attached.
ok
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- 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/20080831/e7988bfa/attachment.pgp>
More information about the ffmpeg-devel
mailing list