[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 [..]"

And what such features are is conveniently left vaguely specified, and
practically is dictated by the lowest common denominator, usually
MSVC...

These things really belong on IRC, as they have no bearing wrt the patch.

Whatever. Anyway, this is all off topic. I have my own opinions on
Microsoft's toolchain, they have not changed at all regardless.
Concretely, I am putting in the ridiculous UINT64_C, as I mentioned
way above in the thread, so there is no issue.

On this note, you may want to examine wm4's patch:
https://lists.libav.org/pipermail/libav-devel/2015-October/072852.html.
I know it is correct wrt proper toolchains, and I have zero interest
in whether or not it works correctly on MSVC.

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