[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:
>>> >> > [...]
>>> >> >> >> @@ -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. :)

See attached. Updated to current code.

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