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

[Robert Swain]

2008/8/18 Michael Niedermayer <michaelni at gmx.at>:
> On Mon, Aug 18, 2008 at 06:42:32PM +0100, Robert Swain wrote:
>> 2008/8/18 Robert Swain <robert.swain at gmail.com>:
>> > 2008/8/18 Michael Niedermayer <michaelni at gmx.at>:
>> >> On Mon, Aug 18, 2008 at 02:03:53PM +0100, Robert Swain wrote:
>> >>> 2008/8/18 Michael Niedermayer <michaelni at gmx.at>:
>> >>> > On Mon, Aug 18, 2008 at 01:47:42PM +0100, Robert Swain wrote:
>> >>> >> 2008/8/18 Robert Swain <robert.swain at gmail.com>:
>> >>> >> > 2008/8/18 Robert Swain <robert.swain at gmail.com>:
>> >>> >> >> 2008/8/18 Robert Swain <robert.swain at gmail.com>:
>> >> [...]
>> >>> > [...]
>> >>> >> +            for (m = 0; m < size; m++, start += inc)
>> >>> >> +                for (i = 1; i <= FFMIN(m, order); i++)
>> >>> >> +                    coef[start] -= coef[start - i] * lpc[i];
>> >>> >>          }
>> >>> >
>> >>> > are you sure this is correct for both values of inc ?
>> >>> > i mean one will have coef[start - i] unfiltered and one filtered as far as
>> >>> > i can see ...
>> >>>
>> >>> You're right and I thought this was strange, but it's the same
>> >>> behaviour as before from what I see and so in that sense it is as
>> >>> correct as it was before. The spec says:
>> >>>
>> >>> tns_ar_filter( spectrum[], size, inc, lpc[], order )
>> >>> {
>> >>>  - Simple all-pole filter of order "order" defined by
>> >>>    y(n) =  x(n) - lpc[1]*y(n-1) - ... - lpc[order]*y(n-order)
>> >>>
>> >>>  - The state variables of the filter are initialized to zero every time
>> >>>
>> >>>  - The output data is written over the input data ("in-place operation")
>> >>>
>> >>>  - An input vector of "size" samples is processed and the index increment
>> >>>    to the next data sample is given by "inc"
>> >>> }
>> >>>
>> >>> I think the third point suggests that it's the intended approach.
>> >>
>> >> but the first suggests it is not
>> >> isnt there some reference implementation or something around?
>> >
>> > 3GPP reference code, tns.c (with cruft removed and indented sanely):
>> >
>> >
>> > for (i=0; i<order; i++)
>> >    state[i] = 0.0F;
>> >
>> > if (inc == -1)
>> >    spec += size - 1;
>> >
>> > for (i=0; i<size; i++) {
>> >    accu = *spec * lpc[0];
>> >
>> >    for (j=0; j<order; j++)
>> >        accu -= lpc[j+1] * state[j];
>> >
>> >    for (j=order-1; j>0; j--)
>> >        state[j] = state[j-1];
>> >
>> >    state[0] = accu;
>> >    *spec = accu;
>> >
>> >    spec += inc;
>> > }
>> >
>> >
>> > 'spec' is basically equivalent to 'coef'. The ref code stores the
>> > states from the beginning of the state array and shifts all the states
>> > along to add a new one to the beginning. The SoC code stores them
>> > starting from the end of the array and works backwards, wrapping back
>> > to the end when the index goes past the beginning of the array. That
>> > is, the ith state[0] == b[(order-1-i)%order].
>> >
>> > Maybe with some index trickery the above can be simplified to
>> > something similar to what I was suggesting for the incorrect code.
>>
>> I don't think much index trickery is needed actually, unless you can
>> come up with something better than:
>>
>> for (m = 0; m < size; m++, start += inc)
>>     for (i = 1; i <= FFMIN(m, order); i++)
>>         coef[start] -= coef[start - i*inc] * lpc[i];
>>
>> The only difference is the *inc in the coef[] index. The reference
>> code considers the y[n-i] relative to the filter direction so if inc
>> is -1 the equation should be coef[start + i]. New patch attached.
>> Still the same PSNR with the zodiac trailer sample. I guess it doesn't
>> use any reverse direction TNS.
>
> patch ok

Applied but with a similar loop for the encode case so that I don't
break anything in SoC for AAC_LTP.

Rob