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

[Michael Niedermayer]

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?

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

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

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is 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/20090223/ff7aaa0e/attachment.pgp>