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

Måns Rullgård mans
Thu Dec 4 10:36:26 CET 2008


Diego Biurrun <diego at biurrun.de> writes:

> On Thu, Dec 04, 2008 at 01:03:35AM -0500, Alex Converse wrote:
>> On Wed, Dec 3, 2008 at 10:13 PM, M?ns Rullg?rd <mans at mansr.com> wrote:
>> > "Alex Converse" <alex.converse at gmail.com> writes:
>> >
>> >> 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.
>> >>
>> >> --- 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.
>> 
>> So what's the preferred behavior here? Should it just be an untested
>> configure flag that's on by default? Should all the non-754 code
>> remain at all?
>
> I see little point in keeping this code and littering the code with
> #ifdef, but others will have to make the final decision...

It's possible that the floating point code is faster on some machine.
It can also be useful to keep it as reference.

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




More information about the ffmpeg-devel mailing list