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

[Robert Swain]

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:
>> >> > [...]
>> >> >> >> @@ -1008,6 +1020,10 @@
>> >> >> >>
>> >> >> >>  /**
>> >> >> >>   * Decode section_data payload; reference: table 4.46.
>> >> >> >> + *
>> >> >> >> + * @param   cb          array of the codebook used for a window group's scalefactor band
>> >> >> >> + * @param   cb_run_end  array of the last scalefactor band of a codebook run for a window group's scalefactor band
>> >> >> >> + * @return  Returns error status.
>> >> >> >>   */
>> >> >> >>  static int decode_section_data(AACContext * ac, GetBitContext * gb, IndividualChannelStream * ics, enum Codebook cb[][64], int cb_run_end[][64]) {
>> >> >> >>      int g;
>> >> >> >
>> >> >> > this one is only confusing, it begins at the use of the term "code book"
>> >> >> > Could you explain how these variables fit any definition of code book?
>> >> >>
>> >> >> Hmm, I think cb[][] should be renamed cb_type[][]. Otherwise I see no
>> >> >> problem as they relate to Huffman codebooks. Any further confusion?
>> >> >
>> >> > yes, i still dont see what this variable has in common with a code book.
>> >> > or code book type.
>> >> > IIRC it is the coding mode used to code each coeff/band. A little like the
>> >> > 4MV vs. 1MV intra vs inter macroblock types.
>> >> > just that here its things like NOISE / INTENSITY / ...
>> >>
>> >> I see what you mean, as they use the same Huffman table to be decoded,
>> >> they aren't indicating different Huffman codebooks but rather
>> >> different methods of coding the different variable types.
>> >>
>> >> I guess you'll need to complain to ISO that the variable and constant
>> >> names have the wrong semantics as they're called codebooks throughout
>> >> the spec and the *_HCB are used too.
>> >
>> > Iam complaining to you because your implementation uses the word codebook
>> > as well
>> > i could complain to ISO but that would be futile.
>> >
>> > Just because ISO uses bad terminology doesnt mean we should copy it.
>>
>> Fair enough. Do you have any suggestions for a more appropriate name?
>> scalefactor_or_intensity_stereo_position_coding_method[][]
>
> band_type ?

What do you think of the attached patch? I'll re-indent to vertically
align and so on after this or something similar is committed of
course. :)

Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20080709-1336-cb_to_band_type.diff
Type: text/x-diff
Size: 16715 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080709/515ec118/attachment.diff>