Re: [FFmpeg-devel] [PATCH] configure, avutil/libm: add fmax, fmaxf, fmin, fminf support

[Clément Bœsch]

On Thu, Oct 15, 2015 at 08:18:02AM -0400, Ganesh Ajjanagadde wrote:
> It has been demonstrated that using libc provided floating point
> functions is beneficial, in the context of fabs() vs FFABS.
> 
> Unfortunately, MSVC 2012 (and earlier) lack the ISO C99 fmax, fmaxf,
> fmin, fminf functions. This patch adds them, thus making their usage in
> FFmpeg safe.
> 

Do we have any use of fmax/fmin? The functions don't exist to make
FFMAX-like faster, it actually is more complex:

   These functions return the maximum of x and y.

   If one argument is a NaN, the other argument is returned.

   If both arguments are NaN, a NaN is returned.

Which means it's likely slower but will do more. Not that I mind, but in this
case, if we happen to use them, you will want to fix your local implementation
to match this behaviour.

[...]

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure, avutil/libm: add fmax, fmaxf, fmin, fminf support

[Carl Eugen Hoyos]

Ganesh Ajjanagadde  gmail.com> writes:

> It has been demonstrated that using libc provided floating point
> functions is beneficial, in the context of fabs() vs FFABS.

Please provide actual numbers for this patch.

Carl Eugen

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure, avutil/libm: add fmax, fmaxf, fmin, fminf support

[Ganesh Ajjanagadde]

On Thu, Oct 15, 2015 at 8:31 AM, Clément Bœsch  wrote:
> On Thu, Oct 15, 2015 at 08:18:02AM -0400, Ganesh Ajjanagadde wrote:
>> It has been demonstrated that using libc provided floating point
>> functions is beneficial, in the context of fabs() vs FFABS.
>>
>> Unfortunately, MSVC 2012 (and earlier) lack the ISO C99 fmax, fmaxf,
>> fmin, fminf functions. This patch adds them, thus making their usage in
>> FFmpeg safe.
>>
>
> Do we have any use of fmax/fmin? The functions don't exist to make
> FFMAX-like faster, it actually is more complex:
>
>These functions return the maximum of x and y.
>
>If one argument is a NaN, the other argument is returned.
>
>If both arguments are NaN, a NaN is returned.
>
> Which means it's likely slower but will do more. Not that I mind, but in this
> case, if we happen to use them, you will want to fix your local implementation
> to match this behaviour.

2 comments:
1. There is a tradeoff: the gain from possible better optimization of
>=, versus the nan handling.
2. NaN handling is not being done currently (by FFMAX and the like) -
so I don't know and can't comment whether we want it. You are right
that if we change to a fmax, we should make it consistent everywhere
on all platforms . Anyone else has comments on this?

>
> [...]
>
> --
> Clément B.
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] configure, avutil/libm: add fmax, fmaxf, fmin, fminf support

[Ganesh Ajjanagadde]

It has been demonstrated that using libc provided floating point
functions is beneficial, in the context of fabs() vs FFABS.

Unfortunately, MSVC 2012 (and earlier) lack the ISO C99 fmax, fmaxf,
fmin, fminf functions. This patch adds them, thus making their usage in
FFmpeg safe.

Signed-off-by: Ganesh Ajjanagadde 
---
 configure|  9 +
 libavutil/libm.h | 28 
 2 files changed, 37 insertions(+)

diff --git a/configure b/configure
index 52a65a4..571d4f2 100755
--- a/configure
+++ b/configure
@@ -1766,6 +1766,10 @@ MATH_FUNCS="
 exp2
 exp2f
 expf
+fmax
+fmaxf
+fmin
+fminf
 isinf
 isnan
 ldexpf
@@ -5278,6 +5282,11 @@ disabled crystalhd || check_lib 
libcrystalhd/libcrystalhd_if.h DtsCrystalHDVersi
 
 atan2f_args=2
 copysign_args=2
+fmax_args=2
+fmaxf_args=2
+fmin_args=2
+fminf_args=2
+fmin_args=2
 ldexpf_args=2
 powf_args=2
 
diff --git a/libavutil/libm.h b/libavutil/libm.h
index 6c17b28..a4a2cc0 100644
--- a/libavutil/libm.h
+++ b/libavutil/libm.h
@@ -82,6 +82,34 @@ static av_always_inline float cbrtf(float x)
 #define exp2f(x) ((float)exp2(x))
 #endif /* HAVE_EXP2F */
 
+#if !HAVE_FMAX
+static av_always_inline av_const double fmax(double x, double y)
+{
+return (x >= y) ? x : y;
+}
+#endif /* HAVE_FMAX */
+
+#if !HAVE_FMAXF
+static av_always_inline av_const float fmaxf(float x, float y)
+{
+return (x >= y) ? x : y;
+}
+#endif /* HAVE_FMAXF */
+
+#if !HAVE_FMIN
+static av_always_inline av_const double fmin(double x, double y)
+{
+return (x <= y) ? x : y;
+}
+#endif /* HAVE_FMIN */
+
+#if !HAVE_FMINF
+static av_always_inline av_const float fminf(float x, float y)
+{
+return (x <= y) ? x : y;
+}
+#endif /* HAVE_FMINF */
+
 #if !HAVE_ISINF
 static av_always_inline av_const int isinf(float x)
 {
-- 
2.6.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure, avutil/libm: add fmax, fmaxf, fmin, fminf support

[Ganesh Ajjanagadde]

On Thu, Oct 15, 2015 at 9:10 AM, Ganesh Ajjanagadde  wrote:
> On Thu, Oct 15, 2015 at 8:31 AM, Clément Bœsch  wrote:
>> On Thu, Oct 15, 2015 at 08:18:02AM -0400, Ganesh Ajjanagadde wrote:
>>> It has been demonstrated that using libc provided floating point
>>> functions is beneficial, in the context of fabs() vs FFABS.
>>>
>>> Unfortunately, MSVC 2012 (and earlier) lack the ISO C99 fmax, fmaxf,
>>> fmin, fminf functions. This patch adds them, thus making their usage in
>>> FFmpeg safe.
>>>
>>
>> Do we have any use of fmax/fmin? The functions don't exist to make
>> FFMAX-like faster, it actually is more complex:
>>
>>These functions return the maximum of x and y.
>>
>>If one argument is a NaN, the other argument is returned.
>>
>>If both arguments are NaN, a NaN is returned.
>>
>> Which means it's likely slower but will do more. Not that I mind, but in this
>> case, if we happen to use them, you will want to fix your local 
>> implementation
>> to match this behaviour.
>
> 2 comments:
> 1. There is a tradeoff: the gain from possible better optimization of
>>=, versus the nan handling.
> 2. NaN handling is not being done currently (by FFMAX and the like) -
> so I don't know and can't comment whether we want it. You are right
> that if we change to a fmax, we should make it consistent everywhere
> on all platforms . Anyone else has comments on this?

If we don't care about NaN, Inf behavior, we should strongly consider
enabling of (on GCC):
-ffinite-math-only
Allow optimizations for floating-point arithmetic that assume that
arguments and results are not NaNs or +-Infs.

This option should never be turned on by any -O option since it can
result in incorrect output for programs which depend on an exact
implementation of IEEE or ISO rules/specifications.

The default is -fno-finite-math-only.

As stated above, -O3 does not automatically enable it.

Thinking about it, if we actually care about NaN's/Infs etc I will
drop this patch. If we don't, we can enable -ffinite-math-only, then I
am pretty sure a good libc will no-op the isnan checks in fmin, fmax,
etc.

>
>>
>> [...]
>>
>> --
>> Clément B.
>>
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure, avutil/libm: add fmax, fmaxf, fmin, fminf support

[Ganesh Ajjanagadde]

On Thu, Oct 15, 2015 at 10:18 AM, Hendrik Leppkes  wrote:
> On Thu, Oct 15, 2015 at 4:04 PM, Derek Buitenhuis
>  wrote:
>> On 10/15/2015 1:18 PM, Ganesh Ajjanagadde wrote:
>>> It has been demonstrated that using libc provided floating point
>>> functions is beneficial, in the context of fabs() vs FFABS.
>>>
>>> Unfortunately, MSVC 2012 (and earlier) lack the ISO C99 fmax, fmaxf,
>>> fmin, fminf functions. This patch adds them, thus making their usage in
>>> FFmpeg safe.
>>
>> We previously had at least fmin emulation in libm.h, but it
>> caused problems on some systems and was removed.
>>
>> Carl and Michael should remember.
>
> See 4436a8f44dedc83767b3d9da9beb85d1fae2ca30

Thanks for the reference, patch dropped.
-ffinite-math (and some other floating point optimizations) look
interesting and could be useful in libavfilter. I will look at this a
little more carefully.

Of course, we should never enable associative optimizations - that
would be bad from an accuracy standpoint.

> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure, avutil/libm: add fmax, fmaxf, fmin, fminf support

[Ganesh Ajjanagadde]

On Thu, Oct 15, 2015 at 11:12 AM, Ganesh Ajjanagadde  wrote:
> On Thu, Oct 15, 2015 at 10:18 AM, Hendrik Leppkes  wrote:
>> On Thu, Oct 15, 2015 at 4:04 PM, Derek Buitenhuis
>>  wrote:
>>> On 10/15/2015 1:18 PM, Ganesh Ajjanagadde wrote:
 It has been demonstrated that using libc provided floating point
 functions is beneficial, in the context of fabs() vs FFABS.

 Unfortunately, MSVC 2012 (and earlier) lack the ISO C99 fmax, fmaxf,
 fmin, fminf functions. This patch adds them, thus making their usage in
 FFmpeg safe.
>>>
>>> We previously had at least fmin emulation in libm.h, but it
>>> caused problems on some systems and was removed.
>>>
>>> Carl and Michael should remember.
>>
>> See 4436a8f44dedc83767b3d9da9beb85d1fae2ca30
>
> Thanks for the reference, patch dropped.
> -ffinite-math (and some other floating point optimizations) look
> interesting and could be useful in libavfilter. I will look at this a
> little more carefully.
>
> Of course, we should never enable associative optimizations - that
> would be bad from an accuracy standpoint.

Did some digging - I don't think any of these are a good idea. For
instance, avfilter can deal with essentially untrusted audio. I also
assume that there exists an audio format whose samples are inherently
floating point and not the standard 8/16 bit pcm etc. In such a
scenario, we want a filter to do something that makes more
mathematical sense. Any of these optimizations cause one or the other
mathematical issue to happen.

In fact, getting fmin or fmax to work might be useful from a
non-performance standpoint, just to make the result make more sense
when NaN is signalled. For instance, consider the -volumedetect
filter. A single NaN (e.g in the input file, or after another filter
in a filtergraph) could clobber the peak detection. How to get them to
work while not breaking stuff as noticed in commit
4436a8f44dedc83767b3d9da9beb85d1fae2ca30 is a trickier question. Any
ideas?

>
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure, avutil/libm: add fmax, fmaxf, fmin, fminf support

[Carl Eugen Hoyos]

Ganesh Ajjanagadde  mit.edu> writes:

> > Ganesh Ajjanagadde  gmail.com> writes:
> >
> >> It has been demonstrated that using libc provided floating point
> >> functions is beneficial, in the context of fabs() vs FFABS.
> >
> > Please provide actual numbers for this patch.
> 
> For a change, why don't you post numbers for where FFMAX is faster.

I don't claim it is faster.

I am just saying that if commits are made that 
potentially break some uncommon platforms, a 
reproducible explanation is given.

Carl Eugen

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure, avutil/libm: add fmax, fmaxf, fmin, fminf support

[Derek Buitenhuis]

On 10/15/2015 1:18 PM, Ganesh Ajjanagadde wrote:
> It has been demonstrated that using libc provided floating point
> functions is beneficial, in the context of fabs() vs FFABS.
> 
> Unfortunately, MSVC 2012 (and earlier) lack the ISO C99 fmax, fmaxf,
> fmin, fminf functions. This patch adds them, thus making their usage in
> FFmpeg safe.

We previously had at least fmin emulation in libm.h, but it 
caused problems on some systems and was removed.

Carl and Michael should remember.

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure, avutil/libm: add fmax, fmaxf, fmin, fminf support

[Hendrik Leppkes]

On Thu, Oct 15, 2015 at 4:04 PM, Derek Buitenhuis
 wrote:
> On 10/15/2015 1:18 PM, Ganesh Ajjanagadde wrote:
>> It has been demonstrated that using libc provided floating point
>> functions is beneficial, in the context of fabs() vs FFABS.
>>
>> Unfortunately, MSVC 2012 (and earlier) lack the ISO C99 fmax, fmaxf,
>> fmin, fminf functions. This patch adds them, thus making their usage in
>> FFmpeg safe.
>
> We previously had at least fmin emulation in libm.h, but it
> caused problems on some systems and was removed.
>
> Carl and Michael should remember.

See 4436a8f44dedc83767b3d9da9beb85d1fae2ca30
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure, avutil/libm: add fmax, fmaxf, fmin, fminf support

[Clément Bœsch]

On Thu, Oct 15, 2015 at 09:10:00AM -0400, Ganesh Ajjanagadde wrote:
> On Thu, Oct 15, 2015 at 8:31 AM, Clément Bœsch  wrote:
> > On Thu, Oct 15, 2015 at 08:18:02AM -0400, Ganesh Ajjanagadde wrote:
> >> It has been demonstrated that using libc provided floating point
> >> functions is beneficial, in the context of fabs() vs FFABS.
> >>
> >> Unfortunately, MSVC 2012 (and earlier) lack the ISO C99 fmax, fmaxf,
> >> fmin, fminf functions. This patch adds them, thus making their usage in
> >> FFmpeg safe.
> >>
> >
> > Do we have any use of fmax/fmin? The functions don't exist to make
> > FFMAX-like faster, it actually is more complex:
> >
> >These functions return the maximum of x and y.
> >
> >If one argument is a NaN, the other argument is returned.
> >
> >If both arguments are NaN, a NaN is returned.
> >
> > Which means it's likely slower but will do more. Not that I mind, but in 
> > this
> > case, if we happen to use them, you will want to fix your local 
> > implementation
> > to match this behaviour.
> 
> 2 comments:

> 1. There is a tradeoff: the gain from possible better optimization of
> >=, versus the nan handling.

FFMIN() and FFMAX() are translated to a single instruction on x86 with
floats: minsd/maxsd. I doubt you can do better.

> 2. NaN handling is not being done currently (by FFMAX and the like) -
> so I don't know and can't comment whether we want it.

If you name the function the same as the system, yes you want to match the
behaviour exactly. Because otherwise, as soon as someone assumes a certain
standard behaviour, there will be nasty surprises on random systems.

[...]

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure, avutil/libm: add fmax, fmaxf, fmin, fminf support

[Ganesh Ajjanagadde]

On Thu, Oct 15, 2015 at 8:20 AM, Carl Eugen Hoyos  wrote:
> Ganesh Ajjanagadde  gmail.com> writes:
>
>> It has been demonstrated that using libc provided floating point
>> functions is beneficial, in the context of fabs() vs FFABS.
>
> Please provide actual numbers for this patch.

For a change, why don't you post numbers for where FFMAX is faster.

>
> Carl Eugen
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure, avutil/libm: add fmax, fmaxf, fmin, fminf support

[Ronald S. Bultje]

Hi,

On Thu, Oct 15, 2015 at 9:10 AM, Ganesh Ajjanagadde 
wrote:

> On Thu, Oct 15, 2015 at 8:31 AM, Clément Bœsch  wrote:
> > On Thu, Oct 15, 2015 at 08:18:02AM -0400, Ganesh Ajjanagadde wrote:
> >> It has been demonstrated that using libc provided floating point
> >> functions is beneficial, in the context of fabs() vs FFABS.
> >>
> >> Unfortunately, MSVC 2012 (and earlier) lack the ISO C99 fmax, fmaxf,
> >> fmin, fminf functions. This patch adds them, thus making their usage in
> >> FFmpeg safe.
> >>
> >
> > Do we have any use of fmax/fmin? The functions don't exist to make
> > FFMAX-like faster, it actually is more complex:
> >
> >These functions return the maximum of x and y.
> >
> >If one argument is a NaN, the other argument is returned.
> >
> >If both arguments are NaN, a NaN is returned.
> >
> > Which means it's likely slower but will do more. Not that I mind, but in
> this
> > case, if we happen to use them, you will want to fix your local
> implementation
> > to match this behaviour.
>
> 2 comments:
> 1. There is a tradeoff: the gain from possible better optimization of
> >=, versus the nan handling.
> 2. NaN handling is not being done currently (by FFMAX and the like) -
> so I don't know and can't comment whether we want it. You are right
> that if we change to a fmax, we should make it consistent everywhere
> on all platforms . Anyone else has comments on this?


If we provide compat functions for maxf() etc., they should behave
identically to the standard-defined maxf(). Note that this may not be more
complex, bool operations on NaN always return false, you can (ab)use that
to keep checks simple. Write (unit) tests if you're not sure.

Whether we switch from FFMAX to maxf() is a separate question that should
be answered by whether maxf() is actually equally fast / faster or not.
That there's corner case behavioural differences is understood and doesn't
have to be part of this discussion IMO.

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel