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

[Ganesh Ajjanagadde]

On Sat, Dec 19, 2015 at 6:26 AM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
> 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.

How, the standard clearly says that the literal will be in the type
int < unsigned int < long int < unsigned long int < long long int <
unsigned long long int, wherever it first fits, and this is also clear
from the link I sent.

long long is at least 64 bits. I can't speak about broken
compilers/environments that lack long long.

> 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?

Nothing wrong with it, but my whole point there was that UINT64_C
(advocated by Luca) was unnecessary - wm4's patch did not use it, Luca
claimed that was wrong, and I argued that it was fine.

>
> Ronald