[FFmpeg-devel] [PATCH 1/2] avutil/common: add av_clipd64

[Ganesh Ajjanagadde]

The rationale for this function is reflected in the documentation for
it, and is copied here:

Clip a double value into the long long amin-amax range.
This function is needed because conversion of floating point to integers when
it does not fit in the integer's representation does not necessarily saturate
correctly (usually converted to a cvttsd2si on x86) which saturates numbers
> INT64_MAX to INT64_MIN. The standard marks such conversions as undefined
behavior, allowing this sort of mathematically bogus conversions. This provides
a safe alternative that is slower obviously but assures safety and better
mathematical behavior.
@param a value to clip
@param amin minimum value of the clip range
@param amax maximum value of the clip range
@return clipped value

Note that a priori if one can guarantee from the calling side that the
double is in range, it is safe to simply do an explicit/implicit cast,
and that will be far faster. However, otherwise, this function should be
used.

------------------------------------------------------------------------------
As a general remark, Clang/GCC has a -Wfloat-conversion so that at least
implicit conversions can be found. This helped me in auditing the
codebase. In order to reduce noise while testing, an explicit cast on the return
was placed. I can remove it if people prefer, though I like the cast as
it makes the intent of possible narrowing explicit.

Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
---
 libavutil/common.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/libavutil/common.h b/libavutil/common.h
index 6f0f582..e778dd2 100644
--- a/libavutil/common.h
+++ b/libavutil/common.h
@@ -298,6 +298,34 @@ static av_always_inline av_const double av_clipd_c(double a, double amin, double
     else               return a;
 }
 
+/**
+ * Clip a double value into the long long amin-amax range.
+ * This function is needed because conversion of floating point to integers when
+ * it does not fit in the integer's representation does not necessarily saturate
+ * correctly (usually converted to a cvttsd2si on x86) which saturates numbers
+ * > INT64_MAX to INT64_MIN. The standard marks such conversions as undefined
+ * behavior, allowing this sort of mathematically bogus conversions. This provides
+ * a safe alternative that is slower obviously but assures safety and better
+ * mathematical behavior.
+ * @param a value to clip
+ * @param amin minimum value of the clip range
+ * @param amax maximum value of the clip range
+ * @return clipped value
+ */
+static av_always_inline av_const int64_t av_clipd64_c(double a, int64_t amin, int64_t amax)
+{
+#if defined(HAVE_AV_CONFIG_H) && defined(ASSERT_LEVEL) && ASSERT_LEVEL >= 2
+    if (amin > amax) abort();
+#endif
+    // INT64_MAX+1,INT64_MIN are exactly representable as IEEE doubles
+    if (a >= 9223372036854775808.0)
+        return INT64_MAX;
+    if (a <= INT64_MIN)
+        return INT64_MIN;
+    // Finally safe to call av_clipd_c
+    return (int64_t)av_clipd_c(a, amin, amax);
+}
+
 /** Compute ceil(log2(x)).
  * @param x value used to compute ceil(log2(x))
  * @return computed ceiling of log2(x)
@@ -511,6 +539,9 @@ static av_always_inline av_const int av_popcount64_c(uint64_t x)
 #ifndef av_clipd
 #   define av_clipd         av_clipd_c
 #endif
+#ifndef av_clipd64
+#   define av_clipd64       av_clipd64_c
+#endif
 #ifndef av_popcount
 #   define av_popcount      av_popcount_c
 #endif
-- 
2.6.2