[FFmpeg-devel] [PATCH] AAC decoder

[Robert Swain]

2008/6/2 Michael Niedermayer <michaelni at gmx.at>:
> On Mon, Jun 02, 2008 at 12:33:57PM +0100, Robert Swain wrote:
>> 2008/4/2 Michael Niedermayer <michaelni at gmx.at>:
>> > On Tue, Apr 01, 2008 at 04:56:48PM +0200, Andreas ?man wrote:
>> >> +
>> >> +/**
>> >> + * Decode spectral data
>> >> + * reference: Table 4.50
>> >> + */
>> >> +static int spectral_data(AACContext * ac, GetBitContext * gb, const ics_struct * ics, const int cb[][64], int * icoef) {
>> >
>> >> +    static const int unsigned_cb[] = { 0, 0, 1, 1, 0, 0, 1, 1, 1, 1, 1 };
>> >
>> > unsigned_cb[x] == x&10
>> >
>> >
>> >> +    int i, k, g;
>> >> +    const uint16_t * offsets = ics->swb_offset;
>> >> +
>> >> +    for (g = 0; g < ics->num_window_groups; g++) {
>> >> +        for (i = 0; i < ics->max_sfb; i++) {
>> >> +            const int cur_cb = cb[g][i];
>> >> +            const int dim = cur_cb >= FIRST_PAIR_HCB ? 2 : 4;
>> >> +            int group;
>> >> +            if (cur_cb == INTENSITY_HCB2 || cur_cb == INTENSITY_HCB) {
>> >> +                continue;
>> >> +            }
>> >> +            if (cur_cb == NOISE_HCB) {
>> >> +                for (group = 0; group < ics->group_len[g]; group++) {
>> >> +                    for (k = offsets[i]; k < offsets[i+1]; k++)
>> >> +                        icoef[group*128+k] = av_random(&ac->random_state) & 0x0000FFFF;
>> >> +                }
>> >> +                continue;
>> >> +            }
>> >> +            if (cur_cb == ZERO_HCB) {
>> >> +                for (group = 0; group < ics->group_len[g]; group++) {
>> >> +                    memset(icoef + group * 128 + offsets[i], 0, (offsets[i+1] - offsets[i])*sizeof(int));
>> >> +                }
>> >> +                continue;
>> >> +            }
>> > [...]
>> >
>> >> +                                while (get_bits1(gb)) n++;
>> >> +                                ptr[j] = (1<<n) + get_bits(gb, n);
>> >> +                            }
>> >> +                        }
>> >> +                    }
>> >> +                    for (j = 0; j < dim; j++)
>> >> +                        icoef[group*128+k+j] = sign[j] * ptr[j];
>> >
>> > This is really ugly, so much unneeded code, reading signs in a temporary array
>> > copying values and then multplying and throwig the intermediate arrays away.
>>
>> How about the attached patch? Any further suggestions?
>>
>> Rob
>
>> Index: aac.c
>> ===================================================================
>> --- aac.c     (revision 2326)
>> +++ aac.c     (working copy)
>> @@ -1214,23 +1214,20 @@
>>              }else if (cur_cb != INTENSITY_HCB2 && cur_cb != INTENSITY_HCB) {
>>                  for (group = 0; group < ics->group_len[g]; group++) {
>>                      for (k = offsets[i]; k < offsets[i+1]; k += dim) {
>> -                        int index = get_vlc2(gb, ac->books[cur_cb - 1].table, 6, 3);
>> +                        const int index = get_vlc2(gb, ac->books[cur_cb - 1].table, 6, 3);
>> +                        const int idx1 = index * dim, idx2 = (group << 7) + k;
>>                          int j;
>> -                        int sign[4] = {1,1,1,1};
>> -                        int ptr[4];
>>                          if (index == -1) {
>>                              av_log(ac->avccontext, AV_LOG_ERROR, "Error in spectral data\n");
>>                              return -1;
>>                          }
>
>> -                        memcpy(ptr, &ac->vq[cur_cb - 1][index * dim], dim*sizeof(int));
>
> whatever *ptr= &ac->vq[cur_cb - 1][index * dim]
> might be cleaner thabn n the repeated use of ac->vq[cur_cb - 1][index * dim]

I spotted a bug. I was using icoef[] instead of sign[] but I didn't
initialise the icoef[] values to 1 beforehand. Applying with this bug
fixed and the ptr suggestion above.

Rob