On Sun, Nov 15, 2015 at 10:24:52AM -0500, Ganesh Ajjanagadde wrote: > On Sun, Nov 15, 2015 at 10:23 AM, wm4 <nfx...@gmail.com> wrote: > > On Sun, 15 Nov 2015 10:20:41 -0500 > > Ganesh Ajjanagadde <gajja...@mit.edu> wrote: > > > >> On Sat, Nov 14, 2015 at 9:01 PM, James Almer <jamr...@gmail.com> wrote: > >> > On 11/14/2015 10:48 PM, Michael Niedermayer wrote: > >> >> From: Michael Niedermayer <mich...@niedermayer.cc> > >> >> > >> >> This should avoid build failures on VS2012 > >> >> Feel free to changes this to a different solution > >> >> > >> >> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> > >> >> --- > >> >> libavutil/common.h | 39 --------------------------------------- > >> >> libavutil/internal.h | 40 ++++++++++++++++++++++++++++++++++++++++ > >> >> 2 files changed, 40 insertions(+), 39 deletions(-) > >> >> > >> >> diff --git a/libavutil/common.h b/libavutil/common.h > >> >> index 813fb37..6f0f582 100644 > >> >> --- a/libavutil/common.h > >> >> +++ b/libavutil/common.h > >> >> @@ -298,42 +298,6 @@ static av_always_inline av_const double > >> >> av_clipd_c(double a, double amin, double > >> >> else return a; > >> >> } > >> >> > >> >> -/** > >> >> - * Clip and convert 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_rint64_clip_c(double a, > >> >> int64_t amin, int64_t amax) > >> >> -{ > >> >> - int64_t res; > >> >> -#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 > >> >> - // do range checks first > >> >> - if (a >= 9223372036854775808.0) > >> >> - return amax; > >> >> - if (a <= -9223372036854775808.0) > >> >> - return amin; > >> >> - > >> >> - // safe to call llrint and clip accordingly > >> >> - res = llrint(a); > >> >> - if (res > amax) > >> >> - return amax; > >> >> - if (res < amin) > >> >> - return amin; > >> >> - return res; > >> >> -} > >> >> - > >> >> /** Compute ceil(log2(x)). > >> >> * @param x value used to compute ceil(log2(x)) > >> >> * @return computed ceiling of log2(x) > >> >> @@ -547,9 +511,6 @@ 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_rint64_clip > >> >> -# define av_rint64_clip av_rint64_clip_c > >> >> -#endif > >> >> #ifndef av_popcount > >> >> # define av_popcount av_popcount_c > >> >> #endif > >> >> diff --git a/libavutil/internal.h b/libavutil/internal.h > >> >> index 5c2cd99..cb0c8cd 100644 > >> >> --- a/libavutil/internal.h > >> >> +++ b/libavutil/internal.h > >> >> @@ -257,6 +257,46 @@ void avpriv_request_sample(void *avc, > >> >> #endif > >> >> > >> >> /** > >> >> + * Clip and convert 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_rint64_clip_c(double a, > >> >> int64_t amin, int64_t amax) > >> > > >> > IMO rename it to avpriv_rint64_clip() or even ff_rint64_clip() since > >> > it's inlined > >> > and not public/exported. > >> > >> Just noticed an issue: Ronald mentioned to me that ffserver and other > >> such programs should not use internal API. This therefore needs to be > >> exported somehow. > > > > If only ffserver needs it, implement it there? > > > > Or even better, just delete ffserver. > > I have repeated this many times in the past, but ffserver was given as > a mere illustration. cmdutils.c also needs it, and there are likely > more such instances that I have not checked.
i think both cmdutils and ffserver should check their arguments and not clip them this can not be done after *rint64_clip() in all cases because if the whole int64 range is valid then there is not enough information left. If the check is done before on the doubles then normal llrint() should work for these cases. ff_rint64_clip() might still be usefull elsewhere but i think all user supplied values should be checked and not clipped [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No great genius has ever existed without some touch of madness. -- Aristotle
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel