[FFmpeg-devel] [Issue 664] [PATCH] Fix AAC PNS Scaling

[Robert Swain]

2008/10/21 Michael Niedermayer <michaelni at gmx.at>:
> On Tue, Oct 21, 2008 at 06:50:02PM +0100, Robert Swain wrote:
>> 2008/10/21 Alex Converse <alex.converse at gmail.com>:
>> > On Tue, Oct 21, 2008 at 1:29 PM, Alex Converse <alex.converse at gmail.com> wrote:
>> >> On Tue, Sep 30, 2008 at 11:25 PM, Alex Converse <alex.converse at gmail.com> wrote:
>> >>> The attached patch should fix AAC PNS scaling [Issue 664]. It will not
>> >>> fix PNS conformance.
>> >>
>> >> Here is my understanding of the current situation:
>> >> 1) PNS is part of the MPEG-4 AAC-LC object type
>> >> 2) PNS is part of the most commonly used family of 14496-3sp4
>> >> profiles, the AAC family of profiles (the AAC profile, the HE-AAC, and
>> >> the HE-AACv2 profile)
>> >> 3) PNS is *broken* in FFMPEG svn [Issue 664], *regardless of per band scaling*
>> >> 4) (For better or worse) PNS requires per band scaling in the latest
>> >> standard (ISO/IEC 14496-3:2005)
>> >> 5) PNS is a low bitrate tool
>> >> (<http://www.hydrogenaudio.org/forums/index.php?s=&showtopic=34652&view=findpost&p=305762>,
>> >> <http://www.hydrogenaudio.org/forums/index.php?s=&showtopic=53340&view=findpost&p=478068>)
>> >> 6) ffaac is enabled by default in FFmpeg
>> >>
>> >> It seems counterproductive to let this broken situation remain while
>> >> arguing the merits of square roots and other normalization schemes.
>> >>
>> >> I propose we:
>> >> A) Fix PNS using one sqrtf() per band (aac-pns-scale-v3.diff)
>> >
>> > Apologies, I attached the reversed diff. The patch attached to this
>> > message is what I actually propose.
>> >
>> >> B) People interesting in a non-inner-most-loop optimal 1/sqrt can duke
>> >> it out afterwards. Alternative normalization schemes (like
>> >> pregenerating noise) can also be explored by interested parties
>> >>
>> >> This will allow users to immediately stop using broken PNS
>> >> accidentally (6,1, 2, 3) while the computational hit of one sqrtf per
>> >> band (A) should be mitigated by the decreased complexity in decoding a
>> >> low bitrate stream (5).
>> >>
>> >> Thoughts?
>>
>> I'm happy with this patch.
>
> then apply it, you are aac maintainer
> or do you think one of my patch application monkeies will do it for you? ;)

No, I'll do it, but I didn't know if you'd object. I think we need to
expand pow2sf_tab a bit anyway and some of the < 255U checks aren't
right. So this is being looked into and then this patch and that will
be applied in close proximity. :)

Rob