[FFmpeg-devel] [PATCH] AAC decoder

[Robert Swain]

2008/6/12 Michael Niedermayer <michaelni at gmx.at>:
> On Tue, Jun 03, 2008 at 01:26:55AM +0100, Robert Swain wrote:
>> 2008/6/2 Robert Swain <robert.swain at gmail.com>:
>> > You'll be happy to know that I'm working on flattening the channel element
>> > structs (sce_struct, cpe_struct, cc_struct) into one (che_struct). When I
>> > submit the patch, I expect it will take an iteration or two to be clean
>> > enough to commit but it has simplified a lot of the code so far. :) There
>> > will also be some considerations about whether to dynamically allocate some
>> > things if they're too large to be allocated with every channel element.
>>
>> See attached. It's a rather large patch and I tried to make it clean
>> (not affecting indentation, etc. as I went along) but maybe you will
>> want it to be split into a number of commits somehow. I don't know
>> whether it will be particularly beneficial but the simplifications
>> resulting from the changes to the structs could be committed
>> separately. It's been tested with stereo and 5.1 files and seems to be
>> working fine. Comments welcome.
>
> Id like it split in a number of patches first.

OK. :)

> * changing the 4 different structs to 1 array of the same struct

Some of the other structs are still needed as all the functions
operate on a single channel element struct. I'll make the combined
struct, remove the unnecessary ones and convert to using the new
struct.

> * code simplifications
> * indention fixup to otherwise unchanged lines

I have this one locally.

> being a possibility
>
>
> [...]
>> +        for(j=0; j<4; j++) {
>> +        if(pcs->che_type[j][i] && !ac->che[j][i])
>> +            ac->che[j][i] = av_mallocz(sizeof(che_struct));
>> +        else
>> +            che_freep(&ac->che[j][i]);
>> +        }
>
> indention
>
>
>>      }
>>
>>      /* Setup default 1:1 output mapping.
>> @@ -545,37 +525,30 @@
>>
>>      ch = 0;
>>      for(i = 0; i < MAX_TAGID; i++) {
>> +        for(j=0; j<4; j++) {
>> +            if(j != ID_CCE && pcs->che_type[j][i]) {
>> +            ac->output_data[ch++] = ac->che[j][i]->ch[0].ret;
>> +            ac->che[j][i]->ch[0].mixing_gain = 1.0f;
>> +        if(j == ID_CPE) {
>> +            ac->output_data[ch++] = ac->che[j][i]->ch[1].ret;
>> +            ac->che[j][i]->ch[1].mixing_gain = 1.0f;
>
> again the indention of the new code is not correct
>
>
> [...]
>> +            for(i=0; i<3; i++)
>> +            ac->mm[i] = mixdown[i];
>
> and again the indention
>
>
> [...]
>> @@ -1175,7 +1147,7 @@
>>  }
>>  #endif /* AAC_SSR */
>>
>> -static void decode_ms_data(AACContext * ac, GetBitContext * gb, cpe_struct * cpe) {
>> +static void decode_ms_data(AACContext * ac, GetBitContext * gb, che_struct * cpe) {
>>      ms_struct * ms = &cpe->ms;
>>      int g, i;
>>      ms->present = get_bits(gb, 2);
>> @@ -1354,7 +1326,7 @@
>>      return 0;
>>  }
>>
>> -static void ms_tool(AACContext * ac, cpe_struct * cpe) {
>> +static void ms_tool(AACContext * ac, che_struct * cpe) {
>>      const ms_struct * ms = &cpe->ms;
>>      const ics_struct * ics = &cpe->ch[0].ics;
>>      float *ch0 = cpe->ch[0].coeffs;
>> @@ -1382,7 +1354,7 @@
>>  }
>>
>>
>> -static void intensity_tool(AACContext * ac, cpe_struct * cpe) {
>> +static void intensity_tool(AACContext * ac, che_struct * cpe) {
>>      const ics_struct * ics = &cpe->ch[1].ics;
>>      sce_struct * sce1 = &cpe->ch[1];
>>      float *coef0 = cpe->ch[0].coeffs, *coef1 = cpe->ch[1].coeffs;
>> @@ -1414,9 +1386,9 @@
>>   */
>>  static int decode_cpe(AACContext * ac, GetBitContext * gb, int id) {
>>      int i;
>> -    cpe_struct * cpe;
>> +    che_struct * cpe;
>
> cosmetics ...

I changed the struct being used...

> [...]
>> +        for(j=0; j<4; j++) {
>> +        if (ac->che[j][i]) {
>> +            sce_trans(ac, &ac->che[j][i]->ch[0]);
>> +        if (j == ID_CPE)
>> +            sce_trans(ac, &ac->che[j][i]->ch[1]);
>
> more odd indention

The strange looking indentation is for lines of code that I altered I
left them at the same level of indentation to avoid mixing cosmetic
and functional changes. Is this what is wanted or if I edit lines, do
you want me to alter their indentation to be correct in the same
commit?

New patches will arrive shortly.

Rob