[FFmpeg-devel] [PATCH] AAC decoder

[Michael Niedermayer]

On Mon, May 26, 2008 at 12:42:37PM +0100, Robert Swain wrote:
> 2008/5/25 Michael Niedermayer <michaelni at gmx.at>:
> > On Fri, May 23, 2008 at 04:06:10PM +0100, Robert Swain wrote:
> >> 2008/5/21 Robert Swain <robert.swain at gmail.com>:
> >> > 2008/4/2 Michael Niedermayer <michaelni at gmx.at>:
> >> >> On Tue, Apr 01, 2008 at 04:56:48PM +0200, Andreas ?man wrote:
> >> >>> Andreas ?man wrote:
> >> >
> >> > [...]
> >> >
> >> >>> +/**
> >> >>> + * Decode scale_factor_data
> >> >>> + * reference: Table 4.47
> >> >>> + */
> >> >>> +static int scale_factor_data(AACContext * ac, GetBitContext * gb, float mix_gain, unsigned int global_gain, ics_struct * ics, const int cb[][64], float sf[][64]) {
> >> >>
> >> >> Maybe rename the function to what the comment says
> >> >>
> >> >>
> >> >>> +    int g, i;
> >> >>> +    unsigned int intensity = 100; // normalization for intensity_tab lookup table
> >> >>> +    int noise = global_gain - 90;
> >> >>> +    int noise_flag = 1;
> >> >>> +    ics->intensity_present = 0;
> >> >>> +    for (g = 0; g < ics->num_window_groups; g++) {
> >> >>> +        for (i = 0; i < ics->max_sfb; i++) {
> >> >>> +            if (cb[g][i] == ZERO_HCB) {
> >> >>> +                sf[g][i] = 0.;
> >> >>
> >> >>> +            } else if (cb[g][i] == INTENSITY_HCB || cb[g][i] == INTENSITY_HCB2) {
> >> >>> +                ics->intensity_present = 1;
> >> >>> +                intensity += get_vlc2(gb, ac->mainvlc.table, 7, 3) - 60;
> >> >>> +                if(intensity > 255) {
> >> >>> +                    av_log(ac->avccontext, AV_LOG_ERROR,
> >> >>> +                           "Intensity (%d) out of range", intensity);
> >> >>> +                    return -1;
> >> >>> +                }
> >> >>> +                sf[g][i] = ac->intensity_tab[intensity];
> >> >>> +            } else if (cb[g][i] == NOISE_HCB) {
> >> >>> +                if (noise_flag) {
> >> >>> +                    noise_flag = 0;
> >> >>> +                    noise += get_bits(gb, 9) - 256;
> >> >>> +                } else {
> >> >>> +                    noise += get_vlc2(gb, ac->mainvlc.table, 7, 3) - 60;
> >> >>> +                }
> >> >>> +                sf[g][i] = pow(2.0, 0.25 * noise) * ac->sf_scale;
> >> >>> +            } else {
> >> >>> +                global_gain += get_vlc2(gb, ac->mainvlc.table, 7, 3) - 60;
> >> >>> +                if(global_gain > 255) {
> >> >>> +                    av_log(ac->avccontext, AV_LOG_ERROR,
> >> >>> +                           "Global gain (%d) out of range", global_gain);
> >> >>> +                    return -1;
> >> >>> +                }
> >> >>> +                sf[g][i] = ac->pow2sf_tab[global_gain];
> >> >>> +            }
> >> >>
> >> >> The identical code is implemented here 3 times, 2 times with seperate
> >> >> LUTs and once by directly calling pow()
> >> >> The differenes look like bugs. like forgetting to check validity or
> >> >> performing the scaling needed for the float2int16 transform.
> >> >>
> >> >> if(cb[g][i] == NOISE_HCB && noise_flag-- > 0)
> >> >>    gain[index] += get_bits(gb, 9) - 256;
> >> >> else
> >> >>    gain[index] += get_vlc2(gb, ac->mainvlc.table, 7, 3) - 60;
> >> >> if(gain[index] > 255)
> >> >>    ...
> >> >> sf[g][i] = ac->pow2sf_tab[gain[index]];
> >> >
> >> > I tried this and audibly it sounds fine on a few files I tested. I
> >> > want to compare the two tables and pow() * ac->sf_scale call before
> >> > submitting a patch though.
> >>
> >> The intensity case is different to the noise and global cases. See
> >> attached patch.
> >
> > It may be different, but it has to be scaled by sf_scale somewhere.
> > intensity_tab is not and i dont see it anywhere afterwards either.
> > if intensity_tab would be scaled then the difference would disapear.
> > all just IMHO and after a quick look ...
> 
> It doesn't seem to be scaled by sf_scale anywhere as far as I can see.
> I guess this is a bug, right? Andreas said he fixed a couple of issues
> regarding intensity stereo in aac.c r2035 and r2036 and he's tested it
> against FAAD so either FAAD is broken too or there is no bug, somehow.
> 

> The tables are still different as intensity uses pow(0.5, (i-100)/4.)
> and the other cases use pow(2.0, (i-100)/4.). 

pow(0.5, (i-100)/4.) == pow(2.0, (100-i)/4.)

and

pow(2.0, (100-i)/4.) / 1024 == pow(2.0, (100-i)/4.-10) ==pow(2.0, (100-i-40)/4.)

possibly these allow the 2 tables to be merged, i mean

pow2sf_tab[i] and intensity_tab[i]
to
pow2sf_tab[i+C] and pow2sf_tab[-i]

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- 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/20080526/394ed17f/attachment.pgp>