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

[Måns Rullgård]

"Alex Converse" <alex.converse at gmail.com> writes:

> Hi,
>
> Attached is the remaing patch from the AAC Main series with the
> rounding functions rewritten as suggested to do arithmetic in the
> integer domain rather than the floating point domain.
>
> A configure check has been added as suggested but I'm not happy with
> it. Any input on this is greatly appreciated.
>
> Regards,
> Alex Converse
>
> diff --git a/configure b/configure
> index 605bf3f..035e939 100755
> --- a/configure
> +++ b/configure
> @@ -844,6 +844,7 @@ HAVE_LIST="
>      gethrtime
>      GetProcessTimes
>      getrusage
> +    ieee754_pun
>      imlib2
>      inet_aton
>      inline_asm
> @@ -1800,6 +1801,13 @@ EOF
>  od -A n -t x1 $TMPO | grep -q '42 *49 *47 *45' && enable bigendian
>  
>  # ---
> +# IEEE float test
> +check_cc <<EOF || die "IEEE float test failed"
> +float f[3] = {218387568.0f, 218387568.0f, 218387568.0f };
> +EOF
> +strings $TMPO | grep -E -q 'MPEGMPEGMPEG|GEPMGEPMGEPM' && enable ieee754_pun

That test isn't portable.  However, I think it's safe to assume
IEEE754 floats.  I doubt anyone will ever run FFmpeg on a VAX, and
it's already broken on Cray since we assume two's complement signed
integers.  

>  # check availability of some header files
>  
>  if check_func dlopen; then
> diff --git a/libavcodec/aac.c b/libavcodec/aac.c
> index 863646e..8063b5b 100644
> --- a/libavcodec/aac.c
> +++ b/libavcodec/aac.c
> @@ -91,6 +91,10 @@
>  #include <math.h>
>  #include <string.h>
>  
> +#ifdef ENABLE_IEEE754_PUN

The ENABLE_* symbols are always defined.  Use HAVE_ or CONFIG_ with
#ifdef.

> +union float754 { float f; uint32_t i; };
> +#endif
> +
>  static VLC vlc_scalefactors;
>  static VLC vlc_spectral[11];
>  
> @@ -838,24 +842,45 @@ static int decode_spectrum_and_dequant(AACContext * ac, float coef[1024], GetBit
>  }
>  
>  static av_always_inline float flt16_round(float pf) {
> +#ifdef ENABLE_IEEE754_PUN
> +    union float754 tmp;
> +    tmp.f = pf;
> +    tmp.i = (tmp.i + 0x00008000U) & 0xFFFF0000U;
> +    return tmp.f;
> +#else

Are this things safe under strict aliasing rules?

-- 
M?ns Rullg?rd
mans at mansr.com