[FFmpeg-devel] [PATCH] lavu/libm: add isfinite fallback

Ganesh Ajjanagadde gajjanagadde at gmail.com
Thu Jan 14 16:54:10 CET 2016


On Wed, Jan 13, 2016 at 9:09 PM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
> Hi,
>
> On Wed, Jan 13, 2016 at 8:09 PM, Ganesh Ajjanagadde <gajjanagadde at gmail.com>
> wrote:
>>
>> On Wed, Jan 13, 2016 at 8:07 PM, Ronald S. Bultje <rsbultje at gmail.com>
>> wrote:
>> > Hi,
>> >
>> > On Wed, Jan 13, 2016 at 7:48 PM, Ganesh Ajjanagadde
>> > <gajjanagadde at gmail.com>
>> > wrote:
>> >>
>> >> On Wed, Jan 13, 2016 at 7:39 PM, Ronald S. Bultje <rsbultje at gmail.com>
>> >> wrote:
>> >> > Hi,
>> >> >
>> >> > On Wed, Jan 13, 2016 at 7:10 PM, Ganesh Ajjanagadde
>> >> > <gajjanagadde at gmail.com>
>> >> > wrote:
>> >> >>
>> >> >> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> >> >> ---
>> >> >>  configure        | 1 +
>> >> >>  libavutil/libm.h | 4 ++++
>> >> >>  2 files changed, 5 insertions(+)
>> >> >>
>> >> >> diff --git a/configure b/configure
>> >> >> index 415d939..a3aaa25 100755
>> >> >> --- a/configure
>> >> >> +++ b/configure
>> >> >> @@ -1821,6 +1821,7 @@ MATH_FUNCS="
>> >> >>      exp2f
>> >> >>      expf
>> >> >>      hypot
>> >> >> +    isfinite
>> >> >>      isinf
>> >> >>      isnan
>> >> >>      ldexpf
>> >> >> diff --git a/libavutil/libm.h b/libavutil/libm.h
>> >> >> index bc44dca..f01e5c6 100644
>> >> >> --- a/libavutil/libm.h
>> >> >> +++ b/libavutil/libm.h
>> >> >> @@ -343,6 +343,10 @@ static av_always_inline av_const int
>> >> >> avpriv_isnan(double x)
>> >> >>          : avpriv_isnan(x))
>> >> >>  #endif /* HAVE_ISNAN */
>> >> >>
>> >> >> +#if !HAVE_ISFINITE
>> >> >> +#define isfinite(x) (!(isinf(x) || isnan(x)))
>> >> >> +#endif /* HAVE_ISFINITE */
>> >> >
>> >> >
>> >> > This will break if you use isfinite(*ptr++) or something obscure like
>> >> > that.
>> >>
>> >> isfinite is a macro, not a function (man isfinite). Your point is
>> >> still valid though, really it should do a sizeof first (dbl/float),
>> >> then call a avpriv_isfinitef, avpriv_isfinite. I was being lazy.
>> >
>> >
>> > Does type matter much? Afaik, inf/nan will be maintained between
>> > float/double/long double.
>>
>> It is important, see a commit I made earlier: 14ea4151d7c. In
>> particular, the main issue is that double to float conversion for dbl
>> outside float range is undefined behavior.
>
>
> Please revert.

Nack. Also, don't know what you want reverted, 14ea415, or this one.
Regardless, all are strong nacks from me.

>
> I don't care much that double to float conversion is undefined; just make it
> a double (or long double).

Double to float is not always undefined; it is undefined for numbers
outside the float range.
I care about it, and I suspect others do as well.

long double fails on ppc; this is well known:
https://ffmpeg.org/pipermail/ffmpeg-devel/2015-November/183975.html.
Can be fixed in configure, but it is a separate issue. It will need
someone with ppc to submit a patch.

Making it call avpriv_isfinite always is likely ok, but that will
entail a float to double cast that is useless. There is no gain; the
macro is very slightly simplified, avpriv_isfinitef can be removed but
at runtime cost. Furthermore, it will make the code asymmetric, unless
all are changed for what purpose?

Also, this approach is the one used in GNU libm at least, suspect even
BSD libm's as well (just open /usr/include/math.h).

>
> Ronald


More information about the ffmpeg-devel mailing list