[FFmpeg-devel] [PATCH] AAC Decoder round 3

[Robert Swain]

2008/7/7 Robert Swain <robert.swain at gmail.com>:
> 2008/7/6 Michael Niedermayer <michaelni at gmx.at>:
>> On Tue, Jul 01, 2008 at 12:37:11PM +0100, Robert Swain wrote:
>> [...]
>>> +static void coupling_tool(AACContext * ac, int independent, int domain) {
>>
>> This as well as other functions could benefit from some documentation
>> what does it do, what is "independent" what "domain" ?
>
> independent == 0 : dependent channel coupling
> independent == 1 : independent channel coupling
>
> The dependence of the channel coupling has implications on the data
> that is shared between the coupled channels and how the data is
> processed.
>
> domain == 0 : apply coupling before TNS decoding
> domain == 1 : apply coupling after TNS decoding
>
> This variable is called cc_domain in the spec.
>
> If you're not happy with the names, please make suggestions based on
> the descriptions. I have added the documentation of these particular
> ones and I'm looking at other non-obvious function names and adding
> doxygen comments.
>
>>> +    int i;
>>> +    for (i = 0; i < MAX_TAGID; i++) {
>>> +        ChannelElement * cc = ac->che[ID_CCE][i];
>>> +        if (cc) {
>>> +            if (cc->coup.ind_sw && independent) {
>>> +                transform_coupling_tool(ac, cc, coupling_independent_trans);
>>> +            } else if (!cc->coup.ind_sw && !independent && (cc->coup.domain == domain)) {
>>> +                transform_coupling_tool(ac, cc, coupling_dependent_trans);
>>> +            }
>>> +        }
>>> +    }
>>> +}
>>> +
>>
>>> +static void transform_sce_tool(AACContext * ac, void (*sce_trans)(AACContext * ac, SingleChannelElement * sce)) {
>>
>> iam still not happy with _tool() functions. Functions should be named
>> so that the names describes what the function does.
>
> They're called tools in the spec. Looking up blah_tool shouldn't be
> too difficult, but I agree it would be better if one didn't have to
> look up the meaning of a function name. I'll see what I can do about
> this tomorrow.

How far does the attached patch go towards aiding understanding of the
functions and non-obvious parameters?

Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20080707-1754-doxygen_comments.diff
Type: text/x-diff
Size: 11042 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080707/4f9984df/attachment.diff>