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

Robert Swain robert.swain
Wed Jul 9 23:54:33 CEST 2008


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.

Regards,
Rob




More information about the ffmpeg-devel mailing list