[FFmpeg-devel] [PATCH] configure+libm.h: add fmin/fmax/fminf/fmaxf emulation

[Muhammad Faiz]

On Tue, Nov 3, 2015 at 9:21 AM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> On Tue, Nov 3, 2015 at 5:10 PM, Muhammad Faiz <mfcc64 at gmail.com> wrote:
>> On Tue, Nov 3, 2015 at 5:42 AM, Clément Bœsch <u at pkh.me> wrote:
>>> On Sat, Oct 31, 2015 at 12:33:54AM +0100, Michael Niedermayer wrote:
>>>> On Sat, Oct 31, 2015 at 12:14:44AM +0100, Hendrik Leppkes wrote:
>>>> > On Fri, Oct 30, 2015 at 11:35 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>> > > From: Michael Niedermayer <michael at niedermayer.cc>
>>>> > >
>>>> > > This should fix the build failure of avf_showcqt.c
>>>> > >
>>>> > > An alternative solution would be to add a check for fmin/fmax to fate-source and
>>>> > > then to replace them by FFMIN/FFMAX, i can do that if preferred?
>>>> > >
>>>> > > Untested due to lack of a affected platform
>>>> > >
>>>> > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>>> > > ---
>>>> > >  configure        |    8 ++++++++
>>>> > >  libavutil/libm.h |   28 ++++++++++++++++++++++++++++
>>>> > >  2 files changed, 36 insertions(+)
>>>> > >
>>>> > > diff --git a/configure b/configure
>>>> > > index 95790f4..e6f5d2c 100755
>>>> > > --- a/configure
>>>> > > +++ b/configure
>>>> > > @@ -1770,6 +1770,10 @@ MATH_FUNCS="
>>>> > >      exp2
>>>> > >      exp2f
>>>> > >      expf
>>>> > > +    fmax
>>>> > > +    fmaxf
>>>> > > +    fmin
>>>> > > +    fminf
>>>> > >      isinf
>>>> > >      isnan
>>>> > >      ldexpf
>>>> > > @@ -5304,6 +5308,10 @@ check_lib math.h sin -lm && LIBM="-lm"
>>>> > >  disabled crystalhd || check_lib libcrystalhd/libcrystalhd_if.h DtsCrystalHDVersion -lcrystalhd || disable crystalhd
>>>> > >
>>>> > >  atan2f_args=2
>>>> > > +fmax_args=2
>>>> > > +fmaxf_args=2
>>>> > > +fmin_args=2
>>>> > > +fminf_args=2
>>>> > >  copysign_args=2
>>>> > >  ldexpf_args=2
>>>> > >  powf_args=2
>>>> > > diff --git a/libavutil/libm.h b/libavutil/libm.h
>>>> > > index 6c17b28..ba837a2 100644
>>>> > > --- a/libavutil/libm.h
>>>> > > +++ b/libavutil/libm.h
>>>> > > @@ -43,6 +43,34 @@
>>>> > >  #define atan2f(y, x) ((float)atan2(y, x))
>>>> > >  #endif
>>>> > >
>>>> > > +#if !HAVE_FMAX
>>>> > > +#undef fmax
>>>> > > +static av_always_inline double fmax(double x, double y)
>>>> > > +{
>>>> > > +    if (x < y) return y;
>>>> > > +    else       return x;
>>>> > > +}
>>>> > > +#endif
>>>> > > +
>>>> > > +#if !HAVE_FMIN
>>>> > > +#undef fmin
>>>> > > +static av_always_inline double fmin(double x, double y)
>>>> > > +{
>>>> > > +    if (x < y) return x;
>>>> > > +    else       return y;
>>>> > > +}
>>>> > > +#endif
>>>> >
>>>> > Wasn't there a concern that these emulations don't behave identical to
>>>> > the C library versions in regards to NaN handling?
>>>> > I faintly remember something in the previous discussion.
>>>>
>>>> yes, i forgot about that :(
>>>> i replaced them by FFMIN / FFMAX
>>>>
>>>
>>> Sorry to raise this again but i htink it must handle NaN, or we need to
>>> use a av_ prefix to define our own semantic.
>>>
>> It seems good to do both
>> use fmax*/fmin* to support NaN handling
>> and add av_* (probably av_max*/av_min*)  to make it more consistent
>> with av_clip*/etc
>
> If a function with side-effects is used inside FFMIN/FFMAX, thats a
> simple bug and should be fixed.
> We don't need to introduce new functions for that. av_clip* exists
> because they are quite a bit more complex than min/max, and even have
> architecture specific optimized versions in some cases.
>
> - Hendrik

If a function without side-effect is used inside FFMIN/FFMAX, the code
is correct,
but the function may be called twice and it will degrade performance.

Thank's