[FFmpeg-devel] [PATCHv2] avutil/libm: correct isnan, isinf compat hacks
michaelni at gmx.at
Wed Nov 25 00:53:20 CET 2015
On Tue, Nov 24, 2015 at 09:17:18AM +0100, Hendrik Leppkes wrote:
> On Sat, Nov 21, 2015 at 2:53 PM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> > On Wed, Nov 18, 2015 at 3:04 PM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> >> On Wed, Nov 18, 2015 at 2:58 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >>> On Tue, Nov 17, 2015 at 04:54:10PM -0500, Ganesh Ajjanagadde wrote:
> >>>> isnan and isinf are actually macros as per the standard. In particular,
> >>>> the existing implementation has incorrect signature. Furthermore, this
> >>>> results in undefined behavior for e.g double values outside float range
> >>>> as per the standard.
> >>>> This patch corrects the undefined behavior for all usage within FFmpeg.
> >>>> Note that long double is not handled as it is not used in FFmpeg.
> >>>> Furthermore, even if at some point long double gets used, it is likely
> >>>> not needed to modify the macro in practice for usage in FFmpeg. See
> >>>> below for analysis.
> >>>> Getting long double to work strictly per the spec is significantly harder
> >>>> since a long double may be an IEEE 128 bit quad (very rare), 80 bit
> >>>> extended precision value (on GCC/Clang), or simply double (on recent Microsoft).
> >>>> On the other hand, any potential future usage of long double is likely
> >>>> for precision (when a platform offers extra precision) and not for range, since
> >>>> the range anyway varies and is not as portable as IEEE 754 single/double
> >>>> precision. In such cases, the implicit cast to a double is well defined
> >>>> and isinf and isnan should work as intended.
> >>>> Reviewed-by: Michael Niedermayer <michael at niedermayer.cc>
> >>>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
> >>>> ---
> >>>> libavutil/libm.h | 34 ++++++++++++++++++++++++++++++++--
> >>>> 1 file changed, 32 insertions(+), 2 deletions(-)
> >>> probably ok
> >>> maybe wait a day or 2 before pushing so people can test it on more
> >>> obscure platforms
> >>> thx
> >> ok, will wait for 2 days for the hypot hack as well. Thanks.
> > pushed, thanks
> nan/inf behavior on VS2012 seems to have broken with this (probably
> nan, but as the patch touches both..)
> Reverting this particular patch resolves the failures.
> (ebur128 was broken before)
heres a patch to reproduce these failures on linux
diff --git a/libavutil/libm.h b/libavutil/libm.h
index 9e5ec5d..1669970 100644
@@ -82,6 +82,7 @@ static av_always_inline float cbrtf(float x)
#define exp2f(x) ((float)exp2(x))
#endif /* HAVE_EXP2F */
+#define HAVE_ISINF 0
/* Note: these do not follow the BSD/Apple/GNU convention of returning -1 for
@@ -109,6 +110,7 @@ static av_always_inline av_const int avpriv_isinf(double x)
#endif /* HAVE_ISINF */
+#define HAVE_ISNAN 0
static av_always_inline av_const int avpriv_isnanf(float x)
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 181 bytes
Desc: Digital signature
More information about the ffmpeg-devel