[FFmpeg-devel] [PATCH] avutil: Move av_rint64_clip_* to internal.h

[Michael Niedermayer]

On Sat, Nov 14, 2015 at 11:01:55PM -0300, James Almer wrote:
> On 11/14/2015 10:48 PM, Michael Niedermayer wrote:
> > From: Michael Niedermayer <michael at niedermayer.cc>
> > 
> > This should avoid build failures on VS2012
> > Feel free to changes this to a different solution
> > 
> > Signed-off-by: Michael Niedermayer <michael at 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.
> 
> > +{
> > +    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;
> > +}
> > +
> > +#ifndef av_rint64_clip
> > +#   define av_rint64_clip   av_rint64_clip_c
> > +#endif
> 
> No need for this chunk.
> 
> > +
> > +/**
> >   * A wrapper for open() setting O_CLOEXEC.
> >   */
> >  av_warn_unused_result
> > 
> 
> 
> In any case should be ok.

changes made, applied

thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151115/94b68366/attachment.sig>