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

[Robert Swain]

2008/11/4 Alex Converse <alex.converse at gmail.com>:
> On Tue, Oct 21, 2008 at 1:18 PM, Robert Swain <robert.swain at gmail.com>wrote:
>
>> 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. :)
>>
>
> So after much thought, and an ignored inquiry to mp4-tech <
> http://lists.mpegif.org/pipermail/mp4-tech/2008-October/008431.html>, I
> think I've come up with the proper limits for dpcm_noise_nrg
>
> Basically from 14496-4:
> "hcod_sf[ ]: Shall only be encoded with the values listed in the
> scalefactor Huffman table. Shall be encoded such that the decoded
> scalefactors sf[g][sfb] are within the range of zero to 255, both
> inclusive."
>
> but the next line is:
>
> "dpcm_noise_nrg: No restrictions apply."
>
> In 14496-3 dpcm_noise_nrg is decoded with hcod_sf[] so things get a little
> ambiguous. It's clear from
> http://lists.mpegif.org/pipermail/mp4-tech/2004-February/003276.html that
> dpcm_noise_nrg can go negative but I believe that's only before It's
> composed with global gain. I think once the final PNS "Scalefactors" are
> calculated we need to be in the 0-255 limit. (As an aside the conformace
> streams only use values 39-75, which is why we weren't seeing any funny
> behavior from them).
>
> Anyway, I think the attached patch should fix PNS ff_aac_pow2sf_tab sizing
> issues.

Applied. Thank you for your research endeavours and I'm sorry this has
taken so long. I still need to close the roundup ticket but I'm
pushing my time at the moment so I'll do it later unless someone else
get to it first.

Best regards,
Rob