[FFmpeg-devel] [PATCH] AAC decoder round 8

[Robert Swain]

2008/8/17 Michael Niedermayer <michaelni at gmx.at>:
> On Sun, Aug 17, 2008 at 11:12:55PM +0100, Robert Swain wrote:
>> 2008/8/15 Robert Swain <robert.swain at gmail.com>:
>> > 2008/8/15 Michael Niedermayer <michaelni at gmx.at>:
>> >> On Fri, Aug 15, 2008 at 09:04:24AM +0100, Robert Swain wrote:
>>
>> [...]
>>
>> >>> Index: libavcodec/aac.c
>> >>> ===================================================================
>> >>> --- libavcodec/aac.c  (revision 14774)
>> >>> +++ libavcodec/aac.c  (working copy)
>> >> [...]
>> >>> +
>> >>> +                for (i = 0; i < tns->order[w][filt]; i++)
>> >>> +                    tns->coef[w][filt][i] = get_bits(gb, coef_len);
>> >>
>> >> tns->coef is only used to index into tmp2_map thus
>> >> tns->coef could already contain the values from tmp2_map
>> >> this also would make the tmp2_map field unneeded in the struct
>> >
>> > OK, I'll look into this.
>>
>> See attached. I assume this is what you mean.

Applied.

>> [...]
>>
>> >>> Index: libavcodec/aacdectab.h
>> >>> ===================================================================
>> >>> --- libavcodec/aacdectab.h    (revision 14767)
>> >>> +++ libavcodec/aacdectab.h    (working copy)
>> >
>> >>> +
>> >>> +/* @name tns_tmp2_map
>> >>> + * Tables of the tmp2[] arrays of LPC coefficients used for TNS.
>> >>> + * The suffix _M_N[] indicate the values of coef_compress and coef_res
>> >>> + * respectively.
>> >>> + * @{
>> >>> + */
>> >>> +static const float tns_tmp2_map_1_3[TNS_MAX_ORDER] = {
>> >>> +     0.00000000,  0.43388373, -0.64278758, -0.34202015,
>> >>> +     0.97492790,  0.78183150, -0.64278758, -0.34202015,
>> >>> +    -0.43388373, -0.78183150, -0.64278758, -0.34202015,
>> >>> +    -0.78183150, -0.43388373, -0.64278758, -0.34202015,
>> >>> +     0.78183150,  0.97492790, -0.64278758, -0.34202015
>> >>> +};
>> >>> +
>> >>> +static const float tns_tmp2_map_0_3[TNS_MAX_ORDER] = {
>> >>> +     0.00000000,  0.43388373,  0.78183150,  0.97492790,
>> >>> +    -0.98480773, -0.86602539, -0.64278758, -0.34202015,
>> >>> +    -0.43388373, -0.78183150, -0.97492790, -0.97492790,
>> >>> +    -0.98480773, -0.86602539, -0.64278758, -0.34202015,
>> >>> +     0.78183150,  0.97492790,  0.97492790,  0.78183150
>> >>> +};
>> >>> +
>> >>> +static const float tns_tmp2_map_1_4[TNS_MAX_ORDER] = {
>> >>> +     0.00000000,  0.20791170,  0.40673664,  0.58778524,
>> >>> +    -0.67369562, -0.52643216, -0.36124167, -0.18374951,
>> >>> +     0.99452192,  0.95105648,  0.86602539,  0.74314481,
>> >>> +    -0.67369562, -0.52643216, -0.36124167, -0.18374951,
>> >>> +    -0.20791176, -0.40673670, -0.58778530, -0.74314487
>> >>> +};
>> >>> +
>> >>> +static const float tns_tmp2_map_0_4[TNS_MAX_ORDER] = {
>> >>> +     0.00000000,  0.20791170,  0.40673664,  0.58778524,
>> >>> +     0.74314481,  0.86602539,  0.95105654,  0.99452192,
>> >>> +    -0.99573416, -0.96182561, -0.89516330, -0.79801720,
>> >>> +    -0.67369562, -0.52643216, -0.36124167, -0.18374951,
>> >>> +    -0.20791176, -0.40673670, -0.58778530, -0.74314487
>> >>> +};
>> >>
>> >> iam not sure if the code is correct but i think several of these
>> >> elements can never be accessed
>> >
>> > I'll check the code.
>>
>> The current code only accesses elements 0-15 by my reckoning.
>
> actually i think it accesses fewer
>
> lets just look at the table 3rd table, to access this implicates that
> tmp2_idx=2
> which implicates
> coef_res=0
> coef_compress=1
>
> and
> coef_len = coef_res + 3 - coef_compress;
> which makes it 2
> thus only 4 elements of the 3rd table could be accssed
> unless i missed something

I don't think you did. I reached the same conclusion shortly after
sending the e-mail. The code in the spec for this section is
absolutely horrible. Working through their code, I don't obtain the
same values as are in the tns_tmp2_map_M_N tables though I expect I'm
making a mistake as I don't quite understand the intention of some of
the code.

Rob