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

[Ganesh Ajjanagadde]

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.

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