[FFmpeg-devel] [PATCH] AAC decoder

[Michael Niedermayer]

On Wed, May 21, 2008 at 03:34:26PM +0100, Robert Swain wrote:
> 2008/4/2 Michael Niedermayer <michaelni at gmx.at>:
[...]
> [...]
> 
> >> +static int coupling_channel_element(AACContext * ac, GetBitContext * gb, int id) {
> >
> >> +    float cc_scale[] = {
> >> +        pow(2, 1/8.), pow(2, 1/4.), pow(2, 0.5), 2.
> >> +    };
> >
> > static const
> >
> >
> >> +    int num_gain = 0;
> >> +    int c, g, sfb;
> >> +    int sign;
> >> +    float scale;
> >> +    sce_struct * sce;
> >> +    coupling_struct * coup;
> >
> >> +    if (!ac->che_cc[id]) {
> >> +        return -1;
> >> +    }
> >
> > somehow i think all these checks can be factored out.
> >
> >
> >> +    sce = &ac->che_cc[id]->ch;
> >> +    sce->mixing_gain = 1.0;
> >> +
> >> +    coup = &ac->che_cc[id]->coup;
> >> +
> >> +    coup->ind_sw = get_bits1(gb);
> >> +    //if (coup->ind_sw)
> >> +    //    av_log(ac->avccontext, AV_LOG_ERROR, "aac: independently switched coupling\n");
> >> +    coup->num_coupled = get_bits(gb, 3);
> >> +    for (c = 0; c <= coup->num_coupled; c++) {
> >> +        num_gain++;
> >> +        coup->is_cpe[c] = get_bits1(gb);
> >> +        coup->tag_select[c] = get_bits(gb, 4);
> >> +        if (coup->is_cpe[c]) {
> >> +            coup->l[c] = get_bits1(gb);
> >> +            coup->r[c] = get_bits1(gb);
> >> +            if (coup->l[c] && coup->r[c])
> >> +                num_gain++;
> >> +        }
> >> +    }
> >> +    coup->domain = get_bits1(gb);
> >> +    sign = get_bits(gb, 1);
> >> +    scale = cc_scale[get_bits(gb, 2)];
> >> +
> >> +    if (individual_channel_stream(ac, gb, 0, 0, sce))
> >> +        return -1;
> >> +
> >> +    for (c = 0; c < num_gain; c++) {
> >> +        int cge = 1;
> >> +        int gain = 0;
> >> +        float gain_cache = 1.;
> >> +        if (c != 0) {
> >> +            cge = coup->ind_sw ? 1 : get_bits1(gb);
> >> +            gain = cge ? get_vlc2(gb, ac->mainvlc.table, 7, 3) - 60: 0;
> >
> >> +            gain_cache = pow(scale, (float)gain);
> >
> > useless cast and cc_scale is useless too
> 
> I have two possibilities for this:
> 
> 20080521-remove_cc_scale-scale_idx.diff
> 20080521-remove_cc_scale-scale.diff
> 
> The latter avoids a division within the loops at the cost of an
> additional pow() call. I didn't know which would be preferred (though
> I would lean toward the latter at a guess) so I'm submitting both for
> review. :)

ok

> 
> [...]
>

> >> +static void tns_filter_tool(AACContext * ac, int decode, sce_struct * sce, float * coef) {
> >
> > bad name ...
> 
> The spec calls this function tns_decode_frame. Is that acceptable or
> if not, do you have any hints? TNS == Temporal Noise Shaping.

Ill think about it in the next iteration of the review, its hard to
suggest something without seeing the whole code ...

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- 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/20080522/8eb62580/attachment.pgp>