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

[Robert Swain]

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.

Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20080818-1839-simplify_tns_ar_filter.diff
Type: text/x-diff
Size: 1351 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080818/c43adef8/attachment.diff>