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

[Robert Swain]

2008/7/9 Robert Swain <robert.swain at gmail.com>:
> 2008/7/8 Michael Niedermayer <michaelni at gmx.at>:
>> On Tue, Jul 08, 2008 at 01:28:07PM +0100, Robert Swain wrote:
>>> 2008/7/8 Michael Niedermayer <michaelni at gmx.at>:
>>> > On Tue, Jul 08, 2008 at 06:31:36AM +0100, Robert Swain wrote:
>>> >> 2008/7/8 Michael Niedermayer <michaelni at gmx.at>:
>>> >> > On Tue, Jul 08, 2008 at 01:25:52AM +0100, Robert Swain wrote:
>
>>> > [...]
>>> >> >> >>      const IndividualChannelStream * ics = &cpe->ch[1].ics;
>>> >> >> >>      SingleChannelElement * sce1 = &cpe->ch[1];
>>> >> >> >> @@ -1382,6 +1441,9 @@
>>> >> >> >>
>>> >> >> >>  /**
>>> >> >> >>   * Decode a channel_pair_element; reference: table 4.4.
>>> >> >> >> + *
>>> >> >> >> + * @param   id  identifies the instance of a syntax element
>>> >> >> >> + * @return  Returns error status.
>>> >> >> >>   */
>>> >> >> >>  static int decode_cpe(AACContext * ac, GetBitContext * gb, int id) {
>>> >> >> >>      int i;
>>> >> >> >
>>> >> >> > iam as smart as without the comment (i could RTFS but that defeats the puprose
>>> >> >> > of the doxy)
>>> >> >>
>>> >> >> Maybe you are but I thought clarification of 'id' was needed.
>>> >> >
>>> >> > yes, what i meant is that i still do not know what id is :)
>>> >>
>>> >> There are multiple syntax elements within a frame (they are the ID_*)
>>> >> and they can occur more than once within a frame. This variable
>>> >> identifies the instance of a syntax element within a frame. Does that
>>> >> make sense? Would adding "within a frame" to the end help the
>>> >> situation at all?
>>> >
>>> > no, because this isnt true
>>> > id=5 is not neccarrily the 5th element of its kind within a frame.
>>> > and the calling code of decode_cpe() calls the id parameter tag
>>> > and another different variable id.
>>>
>>> In the spec it's called element_instance_tag hence 'tag' when calling
>>> decode_cpe(). The variable is incorrectly named 'id' within
>>> decode_cpe(). I could rename both 'element_instance_tag' if you think
>>> this is an acceptable name. If not, what do you suggest as an
>>> alternative?
>>
>> hmm, i need to think more about this, but at least the code should be
>> consistent, calling one id and then another is not good.
>
> The code consistently uses 'tag' now.
>
> RE: "id=5 is not neccarrily the 5th element of its kind within a frame"
>
> Indeed it isn't, but "identifies the instance of a syntax element"
> doesn't state that id=n implies id=1..n-1 exist also.
>
> "syntax element instance identifier"? :)
>
> I can't think of a better way to write this concisely without adding a
> note stating that id=5 doesn't imply that there are at least 5
> occurrences of this syntax element within this frame.

In case it helps:

element_instance_tag

Unique instance tag for syntactic elements other than fill_element().
All syntactic elements containing instance tags may occur more than
once, but, except for data_stream_element()'s, must have a unique
element_instance_tag in each raw_data_block(). This tag is also used
to reference audio syntactic elements in single_channel_element()'s,
channel_pair_element()'s, lfe_channel_element()'s,
data_channel_element()'s, and coupling_channel_element()'s inside a
program_config_element(), and provides the possibility of up to 16
independent program_config_element()'s.

Rob