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

[Ganesh Ajjanagadde]

On Sat, Dec 19, 2015 at 1:33 PM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> 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.

and since when has MS actually cared about POSIX? This statement does
not change the fact that it is just an accident that we have a
solution here.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel