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

[Hendrik Leppkes]

On Sat, Dec 19, 2015 at 10:28 PM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> On Sat, Dec 19, 2015 at 12:33 PM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
>> Hi,
>>
>> On Sat, Dec 19, 2015 at 2:25 PM, Ganesh Ajjanagadde <gajjanag at mit.edu>
>> wrote:
>>
>>> On Sat, Dec 19, 2015 at 11:05 AM, Hendrik Leppkes <h.leppkes at gmail.com>
>>> wrote:
>>> > On Sat, Dec 19, 2015 at 6:54 PM, Ganesh Ajjanagadde
>>> > <gajjanagadde at gmail.com> wrote:
>>> >> On Sat, Dec 19, 2015 at 8:06 AM, Ganesh Ajjanagadde
>>> >> <gajjanagadde at gmail.com> wrote:
>>> >>> 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.
>>> >>
>>> >> This turns out to be the heart of the matter; as usual, Microsoft's
>>> >> toolchain is fundamentally broken:
>>> >>
>>> https://stackoverflow.com/questions/25626022/how-should-a-64-bit-integer-literal-be-represented-in-c
>>> .
>>> >>
>>> >
>>> > Its behavior would appear to be valid C90 on Windows, since long int
>>> > is 32-bit on that platform (which C90 calls for using, not long long)
>>>
>>> Yes, I meant broken wrt C99 (as seen in the link), which most of our
>>> platforms have and use. Basically, I meant that their C99 support is
>>> half-baked and broken (of course not news).
>>
>>
>> But FFmpeg isn't C99, see
>> https://ffmpeg.org/developer.html#C-language-features
>>
>> "FFmpeg is programmed in the ISO C90 language with a few additional
>> features from ISO C99 [..]"
>
> A last comment to show the ridiculousness of it all: UINT64_C is
> defined only in stdint.h, a C99 feature. Thus, if you want a "strict
> C90" solution to this, there is none. In particular, using UINT64_C is
> not "valid C90" on any platform.
>
> It is just a happy accident that MSVC has UINT64_C.
>

stdint.h was POSIX long before it became C99.