[FFmpeg-devel] [PATCH] AAC decoder

[Robert Swain]

2008/6/19 Michael Niedermayer <michaelni at gmx.at>:
> On Fri, Jun 13, 2008 at 01:47:29PM +0100, Robert Swain wrote:
>> 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.
>> > * changing the 4 different structs to 1 array of the same struct
>> > * code simplifications
>> > * indention fixup to otherwise unchanged lines
>> >
>> > being a possibility
>>
>> See attached.
>>
>> Note that I ordered the elements of the che_struct such that the
>> cpe_struct code didn't need to be changed until removal of cpe_struct
>> was desirable. I hope these are OK. :)
>>
>> Thanks,
>> Rob
>
>> From 82f9fa27ec080bbe42279558269fc90d646bdc9a Mon Sep 17 00:00:00 2001
>> From: Robert Swain <rob at opendot.cl>
>> Date: Fri, 13 Jun 2008 11:38:34 +0100
>> Subject: [PATCH] Add code necessary to switch to a generic channel element struct
> [...]
>> +typedef struct {
>> +    // CPE specific
>> +    int common_window;     ///< Set if channels share a common 'ics_struct' in bitstream
>> +    ms_struct ms;
>> +    // Shared
>> +    sce_struct ch[2];
>> +    // CCE specific
>> +    coupling_struct coup;
>> +} che_struct;
>
> Ive thought that maybe some of these could be placed at the end and only
> allocated when needed, either way that would be for a different and
> future patch. (it could also be several structs which share the first few
> fields)
>
> otherswise ok
>
>
>> From 025a1d319ad465448edf365a586a77c8186414b9 Mon Sep 17 00:00:00 2001
>> From: Robert Swain <rob at opendot.cl>
>> Date: Fri, 13 Jun 2008 12:29:11 +0100
>> Subject: [PATCH] - Switch to using che_struct in program_config_struct and AACContext
>>  - Alter code accordingly such that it is still as functional as before
>
> ok
>
>
> [...]
>> From 5412f7b493fb38471f05b4723256a7d3d9855e2c Mon Sep 17 00:00:00 2001
>> From: Robert Swain <rob at opendot.cl>
>> Date: Fri, 13 Jun 2008 13:01:11 +0100
>> Subject: [PATCH] Switch mixdown code to use che_struct
>
> ok
>
>
> [...]
>> From cb1466531de8a533f528b7cc1a734b1236a048ee Mon Sep 17 00:00:00 2001
>> From: Robert Swain <rob at opendot.cl>
>> Date: Fri, 13 Jun 2008 13:04:42 +0100
>> Subject: [PATCH] Make remaining code that uses cpe_struct use che_struct instead
>
> ok
>
>
> [...]
>
>
>> From 219106c85ff29d54a127020ae28f39326c10ca6b Mon Sep 17 00:00:00 2001
>> From: Robert Swain <rob at opendot.cl>
>> Date: Fri, 13 Jun 2008 13:06:08 +0100
>> Subject: [PATCH] Remove unused code
>
> ok
>
> [...]
>
>
>> From 99acb20d34f87ad4bc3d1e346f469ccb8ec43452 Mon Sep 17 00:00:00 2001
>> From: Robert Swain <rob at opendot.cl>
>> Date: Fri, 13 Jun 2008 13:25:47 +0100
>> Subject: [PATCH] Make code simplifications enabled by previous commits
>
> ok
>
> [...]
>
>
>> From b1d801dee3dec324c5927892ab0df1399d9c57e5 Mon Sep 17 00:00:00 2001
>> From: Robert Swain <rob at opendot.cl>
>> Date: Fri, 13 Jun 2008 13:35:43 +0100
>> Subject: [PATCH] Cosmetics
>>
>> ---
>>  aac.c |    8 ++++----
>>  1 files changed, 4 insertions(+), 4 deletions(-)
>>
>
>> diff --git a/aac.c b/aac.c
>> index 8a86885..b473058 100644
>> --- a/aac.c
>> +++ b/aac.c
>> @@ -491,9 +491,9 @@ static int output_configure(AACContext *ac, program_config_struct *newpcs) {
>>          channels += !!pcs->che_type[ID_SCE][i] + !!pcs->che_type[ID_CPE][i] * 2 + !!pcs->che_type[ID_LFE][i];
>>
>>          for(j = 0; j < 4; j++) {
>> -            if(pcs->che_type[j][i] && !ac->che[j][i]) {
>> +            if(pcs->che_type[j][i] && !ac->che[j][i])
>>                  ac->che[j][i] = av_mallocz(sizeof(che_struct));
>> -        } else
>> +            else
>
> i prefer
>
> if(){
> }else
>
> because it results in cleaner looking diffs if something is added while we
> dont loose a line to a '}'

OK. All applied. Thank you very much. :)

Rob