[FFmpeg-devel] [PATCH] lavu/libm: add copysign hack

[Ronald S. Bultje]

Hi,

On Fri, Dec 18, 2015 at 3:47 PM, Ganesh Ajjanagadde <gajjanagadde at gmail.com>
wrote:

> On Fri, Dec 18, 2015 at 12:41 PM, Ronald S. Bultje <rsbultje at gmail.com>
> wrote:
> > Hi,
> >
> > On Fri, Dec 18, 2015 at 2:18 PM, Ganesh Ajjanagadde <
> gajjanagadde at gmail.com>
> > wrote:
> >>
> >> For systems with broken libms.
> >> Tested with NAN, -NAN, INFINITY, -INFINITY, +/-x for regular double x
> and
> >> combinations of these.
> >>
> >> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
> >> ---
> >>  configure        | 2 +-
> >>  libavutil/libm.h | 9 +++++++++
> >>  2 files changed, 10 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/configure b/configure
> >> index 123d1df..7917386 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -2852,7 +2852,7 @@ cropdetect_filter_deps="gpl"
> >>  delogo_filter_deps="gpl"
> >>  deshake_filter_select="pixelutils"
> >>  drawtext_filter_deps="libfreetype"
> >> -dynaudnorm_filter_deps="copysign erf"
> >> +dynaudnorm_filter_deps="erf"
> >>  ebur128_filter_deps="gpl"
> >>  eq_filter_deps="gpl"
> >>  fftfilt_filter_deps="avcodec"
> >> diff --git a/libavutil/libm.h b/libavutil/libm.h
> >> index 6d8bd68..637de19 100644
> >> --- a/libavutil/libm.h
> >> +++ b/libavutil/libm.h
> >> @@ -62,6 +62,15 @@ static av_always_inline float cbrtf(float x)
> >>  }
> >>  #endif
> >>
> >> +#if !HAVE_COPYSIGN
> >> +static av_always_inline double copysign(double x, double y)
> >> +{
> >> +    uint64_t vx = av_double2int(x);
> >> +    uint64_t vy = av_double2int(y);
> >> +    return av_int2double((vx & 0x7fffffffffffffff) | (vy &
> >> 0x8000000000000000));
> >> +}
> >> +#endif
> >
> >
> > Don't these need type suffixes (e.g. UINT64_C(val)) on some systems?
>
> I believe not, see
> http://en.cppreference.com/w/cpp/language/integer_literal


That document confirms that it is indeed legal for a compiler to read the
given literal on a 32bit or windows 64bit system as a 32bit max value, and
your literals don't fit in 32bit. Indeed, we have indeed historically seen
that UINT64_C is primarily required to silence warnings and fix problems on
earlier releases of msvc, which are still supported (although not
necessarily recommended).

and a long
> discussion at libav-devel
> https://lists.libav.org/pipermail/libav-devel/2015-October/072866.html
> and related messages.


I don't see anything in that email that suggests that we should not use
UINT64_C?

Ronald