Hi, On Sun, Nov 1, 2015 at 1:03 PM, Ganesh Ajjanagadde <gajjanaga...@gmail.com> wrote:
> 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 <gajjanaga...@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); > +} Is this different from llrint? Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel