[FFmpeg-devel] AAC decoder round 9

Michael Niedermayer michaelni
Thu Aug 21 01:40:26 CEST 2008


On Wed, Aug 20, 2008 at 11:00:11PM +0100, Robert Swain wrote:
> 2008/8/20 Robert Swain <robert.swain at gmail.com>:
> > 2008/8/20 Michael Niedermayer <michaelni at gmx.at>:
> >> On Wed, Aug 20, 2008 at 07:44:24PM +0100, Robert Swain wrote:
> >>> 2008/8/20 Robert Swain <robert.swain at gmail.com>:
> >>> > 2008/8/20 Vitor Sessak <vitor1001 at gmail.com>:
> >>> >> Robert Swain wrote:
> >> [...]
> >>> >>> It seems the various conditions on in[] and f0 cause it to exit with
> >>> >>> an error with the coef[] vectors I've tried and as they're potentially
> >>> >>> valid, that's disconcerting for using this code.
> >>> >>>
> >>> >>> compute_lpc_coefs() accepts a two dimensional lpc array as an
> >>> >>> argument. I'm guessing this so that the coefficients are available for
> >>> >>> various orders ready for testing to choose which is best or something
> >>> >>> like that.
> >>> >>>
> >>> >>> Do you have any advice for how to proceed? I'm going to keep prodding
> >>> >>> eval_lpc_coeffs() in the mean time.
> >>> >>
> >>> >> Can the following patch be modified to work with AAC too?
> >>> >
> >>> > I don't like the patch as it is, but you've helped me quite a bit to
> >>> > figure this out. :)
> >>> >
> >>> > lpc[0] = 1 isn't used so I'd like to remove it and start the useful
> >>> > coefficients from index 0. With this, the above-quoted loop can be
> >>> > rewritten to use part of compute_lpc_coefs():
> >>> >
> >>> > // tns_decode_coef
> >>> > for (m = 0; m < order; m++) {
> >>> >    float tmp;
> >>> >    lpc[m] = tns->coef[w][filt][m];
> >>> >    for (i = 0; i < m/2; i++) {
> >>> >        tmp = lpc[i];
> >>> >        lpc[i]     += lpc[m] * lpc[m-1-i];
> >>> >        lpc[m-1-i] += lpc[m] * tmp;
> >>> >    }
> >>> >    if(m & 1)
> >>> >        lpc[i]     += lpc[m] * lpc[i];
> >>> > }
> >>> >
> >>> > Doing it this way avoids the need for the b[] temporary array and
> >>> > produces identical output I think. You should be able to see clearly
> >>> > that this is the same as the main loops within compute_lpc_coefs().
> >>> > So, now the question is how to refactor this code to satisfy everyone.
> >>> >
> >>> > Michael - would you like a single generic macro like the one Vitor
> >>> > proposes? Or maybe two macros, one for this inner set of loops and one
> >>> > for the extra autocorrelation function return value normalisation to
> >>> > convert to autocorrelation coefficients and checks that are needed for
> >>> > the other implementations? Or something else?
> >>>
> >>> Actually, I think an inline function will be better. I'll have a look
> >>> a go at implementing this after I've eaten.
> >>
> >> thats exactly what i wanted to suggest, all these macros are ugly ...
> >> (the reason behind vitors macros was IIRC a compression loss with float
> >>  vs.double in relation to flac but i didnt had the time to investigate
> >>  what excatly was causing it or if there is a better solution ...)
> >
> > Hmm. Will this cause a problem if I use a double temporary variable? I
> > think I've figured everything else out in my head. Though maybe
> > there's something I've missed, I'll find out shortly. :)
> 
> We could:
> 
> - have two shared versions of this function - one float and one double
> - share a float version between aac and ra288
> - have one function which uses void pointers and uses nasty pointer arithmetic
> - use a macro like Vitor tried
> - try to find the issue in flac causing the decrease in compression
> efficiency when using float rather than double as it is for this
> function at the moment but this could take some time
> 
> Unless we can choose a clear course of action, I'm inclined to push
> that this last hunk be committed so I can work on more worthwhile
> things such as imdct_half() porting and SBR support. 8 lines of code
> in aac.c that could use something shared from elsewhere doesn't seem
> all that important in the grand scheme of things... but that's my
> opinion. :)

ok, commit it, its not reasonable to hold this up due to a non trivial
simplification of 8 lines of code ...

of course iam still rather interrested which variable exactly is responsible
for the compression loss ...

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Thouse who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080821/fe008b1a/attachment.pgp>



More information about the ffmpeg-devel mailing list