[FFmpeg-devel] [PATCH] AAC decoder

[Michael Niedermayer]

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.
* changing the 4 different structs to 1 array of the same struct
* code simplifications
* indention fixup to otherwise unchanged lines

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 ...


[...]
> +        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


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

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- 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/20080612/e15dd9a8/attachment.pgp>