On Tue, Nov 24, 2015 at 8:48 PM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote: > On Tue, Nov 24, 2015 at 6:53 PM, Michael Niedermayer <michae...@gmx.at> wrote: >> On Tue, Nov 24, 2015 at 09:17:18AM +0100, Hendrik Leppkes wrote: >>> On Sat, Nov 21, 2015 at 2:53 PM, Ganesh Ajjanagadde <gajja...@mit.edu> >>> wrote: >>> > On Wed, Nov 18, 2015 at 3:04 PM, Ganesh Ajjanagadde <gajja...@mit.edu> >>> > wrote: >>> >> On Wed, Nov 18, 2015 at 2:58 PM, Michael Niedermayer <michae...@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 <mich...@niedermayer.cc> >>> >>>> Signed-off-by: Ganesh Ajjanagadde <gajjanaga...@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) >> { > > problem is with the isnan alone, reproduced and under investigation.
Analysis complete and it is due to implementation defined behavior of the cast of the uint64_t result into int. Worked around with a hack: basically add a && 1 at the return to guarantee returning of 0 or 1. Tested with FATE on Michael's lines and pushed. Apologies all for the breakage. > >> >> >> [...] >> -- >> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB >> >> I am the wisest man alive, for I know one thing, and that is that I know >> nothing. -- Socrates >> >> _______________________________________________ >> 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