[FFmpeg-devel] [PATCHv2] avutil/libm: correct isnan, isinf compat hacks

[Ganesh Ajjanagadde]

isnan and isinf are actually macros as per the standard. In particular,
the existing implementation has incorrect signature. Furthermore, this
results in undefined behavior for e.g double values outside float range
as per the standard.

This patch corrects the undefined behavior for all usage within FFmpeg.

Note that long double is not handled as it is not used in FFmpeg.
Furthermore, even if at some point long double gets used, it is likely
not needed to modify the macro in practice for usage in FFmpeg. See
below for analysis.

Getting long double to work strictly per the spec is significantly harder
since a long double may be an IEEE 128 bit quad (very rare), 80 bit
extended precision value (on GCC/Clang), or simply double (on recent Microsoft).
On the other hand, any potential future usage of long double is likely
for precision (when a platform offers extra precision) and not for range, since
the range anyway varies and is not as portable as IEEE 754 single/double
precision. In such cases, the implicit cast to a double is well defined
and isinf and isnan should work as intended.

Reviewed-by: Michael Niedermayer <michael at niedermayer.cc>
Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
---
 libavutil/libm.h | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/libavutil/libm.h b/libavutil/libm.h
index 6c17b28..221c286 100644
--- a/libavutil/libm.h
+++ b/libavutil/libm.h
@@ -83,23 +83,53 @@ static av_always_inline float cbrtf(float x)
 #endif /* HAVE_EXP2F */
 
 #if !HAVE_ISINF
-static av_always_inline av_const int isinf(float x)
+#undef isinf
+/* Note: these do not follow the BSD/Apple/GNU convention of returning -1 for
+-Inf, +1 for Inf, 0 otherwise, but merely follow the POSIX/ISO mandated spec of
+returning a non-zero value for +/-Inf, 0 otherwise. */
+static av_always_inline av_const int avpriv_isinff(float x)
 {
     uint32_t v = av_float2int(x);
     if ((v & 0x7f800000) != 0x7f800000)
         return 0;
     return !(v & 0x007fffff);
 }
+
+static av_always_inline av_const int avpriv_isinf(double x)
+{
+    uint64_t v = av_double2int(x);
+    if ((v & 0x7ff0000000000000) != 0x7ff0000000000000)
+        return 0;
+    return !(v & 0x000fffffffffffff);
+}
+
+#define isinf(x)                  \
+    (sizeof(x) == sizeof(float)   \
+        ? avpriv_isinff(x)        \
+        : avpriv_isinf(x))
 #endif /* HAVE_ISINF */
 
 #if !HAVE_ISNAN
-static av_always_inline av_const int isnan(float x)
+static av_always_inline av_const int avpriv_isnanf(float x)
 {
     uint32_t v = av_float2int(x);
     if ((v & 0x7f800000) != 0x7f800000)
         return 0;
     return v & 0x007fffff;
 }
+
+static av_always_inline av_const int avpriv_isnan(double x)
+{
+    uint64_t v = av_double2int(x);
+    if ((v & 0x7ff0000000000000) != 0x7ff0000000000000)
+        return 0;
+    return v & 0x000fffffffffffff;
+}
+
+#define isnan(x)                  \
+    (sizeof(x) == sizeof(float)   \
+        ? avpriv_isnanf(x)        \
+        : avpriv_isnan(x))
 #endif /* HAVE_ISNAN */
 
 #if !HAVE_LDEXPF
-- 
2.6.2