[FFmpeg-devel] [PATCH] E-AC-3 decoder, round 3

[Justin Ruggles]

Michael Niedermayer wrote:
> On Wed, Aug 27, 2008 at 08:36:03PM -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:
> [...]
>>>> +{
>>>> +    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
>> i guess gaq_gain_group_code would be more accurate, but that's long.
>> how about group_code?
> 
> ok

fixed for next patch.

> [...]
>>>> +    } 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?
>> It is not.  It only prevents from having to check all the flags again
>> when removing dithering.
> 
> And what is the point?
> Setting it with:
>         s->dither_all = 1;
>         for (ch = 1; ch <= fbw_channels; ch++) {
>             s->dither_flag[ch] = get_bits1(gbc);
>             if(!s->dither_flag[ch])
>                 s->dither_all = 0;
>         }
> does a check per fbw_channels
> to avoid a check per fbw_channels later in remove_dithering()
> ?

you're right. it is kinda useless. it avoids a function call and the
extra looping through channels, but that's trivial.  the checks are
still there (plus 1 for checking dither_all).  i have removed it.

an updated E-AC-3 patch will be sent soon.

Thanks,
Justin