[FFmpeg-devel] [PATCHv2] avutil/libm: correct isnan, isinf compat hacks

[Michael Niedermayer]

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.
> 
> http://fate.ffmpeg.org/report.cgi?time=20151124040137&slot=x86_32-msvc11-windows-native
> (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
--- a/libavutil/libm.h
+++ b/libavutil/libm.h
@@ -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
 #if !HAVE_ISINF
 #undef isinf
 /* 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)
         : avpriv_isinf(x))
 #endif /* HAVE_ISINF */

+#define HAVE_ISNAN 0
 #if !HAVE_ISNAN
 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...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151125/45ebe4f2/attachment.sig>