[FFmpeg-devel] [PATCH] AAC: type puns for 16 bit floating point rounding

[Robert Swain]

2009/2/23 Michael Niedermayer <michaelni at gmx.at>:
> On Mon, Feb 23, 2009 at 01:34:44PM +0000, Robert Swain wrote:
>> 2009/2/23 Michael Niedermayer <michaelni at gmx.at>:
>> > On Sun, Feb 22, 2009 at 05:51:58PM -0500, Alex Converse wrote:
>> >> On Mon, Feb 16, 2009 at 7:22 PM, Alex Converse <alex.converse at gmail.com> wrote:
>> >> > On Thu, Feb 12, 2009 at 9:29 AM, Robert Swain <robert.swain at gmail.com> wrote:
>> >> >> 2008/12/21 Robert Swain <robert.swain at gmail.com>:
>> >> >>> 2008/12/13 Guillaume POIRIER <poirierg at gmail.com>:
>> >> >>>> On Sat, Dec 13, 2008 at 3:54 PM, M?ns Rullg?rd <mans at mansr.com> wrote:
>> >> >>>>> Can you upload the file somewhere? ?I'd rather not install mp4box just
>> >> >>>>> for this.
>> >> >>>>
>> >> >>>> Sure, here it is: http://natsuki.mplayerhq.hu/~ml-admin/type_puns.mp4
>> >> >>>
>> >> >>> M?ns?
>> >> >>
>> >> >> This thread seems to be mostly dead. Maybe Alex can do something to
>> >> >> make the speed testing that needs to be done easier so we can get
>> >> >> these optimisations merged into trunk, if indeed they do make decoding
>> >> >> faster.
>> >> >>
>> >> >
>> >> > /MPlayer/incoming/aac-main/aac-main-long.adts,txt
>> >> >
>> >> > My system reports (Linux/amd64):
>> >> > before:
>> >> > real ? ?0m7.602s
>> >> > user ? ?0m7.568s
>> >> > sys ? ? 0m0.032s
>> >> >
>> >> > after:
>> >> > real ? ?0m1.182s
>> >> > user ? ?0m1.148s
>> >> > sys ? ? 0m0.040s
>> >> >
>> >> > I think we are looking for someone on ARM to test
>> >>
>> >> Let's try this one more time:
>> >>
>> >> We are looking for people on exotic architectures (non-x86) to
>> >> benchmark decoding a specific file on their systems before and after
>> >> the attached patch.
>> >>
>> >> The file: /MPlayer/incoming/aac-main/aac-main-long.adts,txt
>> >> Mirror: http://filer.case.edu/ajc30/pub/ffmpeg/
>> >>
>> >> Thanks,
>> >> Alex Converse
>> >
>> > [...]
>> >> @@ -871,24 +875,45 @@ static int decode_spectrum_and_dequant(AACContext * ac, float coef[1024], GetBit
>> >> ?}
>> >>
>> >> ?static av_always_inline float flt16_round(float pf) {
>> >> +#ifdef HAVE_IEEE754_PUN
>> >> + ? ?union float754 tmp;
>> >> + ? ?tmp.f = pf;
>> >> + ? ?tmp.i = (tmp.i + 0x00008000U) & 0xFFFF0000U;
>> >> + ? ?return tmp.f;
>> >> +#else
>> >> ? ? ?int exp;
>> >> ? ? ?pf = frexpf(pf, &exp);
>> >> ? ? ?pf = ldexpf(roundf(ldexpf(pf, 8)), exp-8);
>> >> ? ? ?return pf;
>> >> +#endif
>> >> ?}
>> >
>> > what is the point of the silly redundancy?
>> > I would replace the old code by the new, not keep both with #ifdefs
>> > thats not good for readability.
>> > Of course thats unless you do find a system where the frexpf()/ldexpf is
>> > faster but i doubt such system exists
>>
>> Are you saying that we mandate IEEE754 compliant float arithmetic for
>> using FFmpeg so the #ifdefs are not necessary? If so, if Alex (or
>> someone) removes these, can the patch be applied?
>
> IEEE754 compliant float arithmetic?
> no
>
> IEEE754 compliant 32bit float format?
> well, is there a system on which ffmpeg could run that lacks support
> for 32bit IEEE floats but supports another float format?

I don't know. Does anyone know? Have they tested it? I would guess
not. I suggest we leave this until after the release unless it fixes
something and breaks nothing, then commit replacing the existing code.
If people complain about it, we always have the svn history.

> anyway i must admit i was tired and had not considered this ...
> though i would suspect that a pure integer implementation would beat
> frexpf/ldexpf

This only matters if an arch is found with 32-bit float format
non-IEEE754 compliant.

Regards,
Rob