On Mon, Nov 16, 2015 at 7:50 AM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote:
> On Mon, Nov 16, 2015 at 6:55 AM, Michael Niedermayer
> <mich...@niedermayer.cc> wrote:
>> On Sat, Nov 14, 2015 at 08:05:59PM -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.
>>> There are some issues with long double, but they are theoretical at the
>>> moment. For instance, the usual culprit for lacking isinf and that uses
>>> this fallback is Microsoft, and on Microsoft, long double = double
>>> anyway. Furthermore, no client of isinf, isnan in the codebase actually
>>> uses long double arguments.
>>>
>>> The above issue is harder because long double may be an IEEE 128 bit quad
>>> (very rare) or a 80 bit extended precision value (on GCC/Clang).
>>>
>>> Signed-off-by: Ganesh Ajjanagadde <gajjanaga...@gmail.com>
>>> ---
>>> libavutil/libm.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 48 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavutil/libm.h b/libavutil/libm.h
>>> index ab5df1b..04d9411 100644
>>> --- a/libavutil/libm.h
>>> +++ b/libavutil/libm.h
>>> @@ -91,23 +91,69 @@ static av_always_inline av_const double hypot(double x,
>>> double y)
>>> #endif /* HAVE_HYPOT */
>>>
>>> #if !HAVE_ISINF
>>> -static av_always_inline av_const int isinf(float x)
>>> +#undef isinf
>>> +/* Note: these do not follow the BSD/Apple/GNU convention of returning -1
>>> for
>>> +-Inf, +1 for Inf, 0 otherwise, but merely follow the POSIX/ISO mandated
>>> spec of
>>> +returning a non-zero value for +/-Inf, 0 otherwise. */
>>> +static av_always_inline av_const int avpriv_isinff(float x)
>>> {
>>> uint32_t v = av_float2int(x);
>>> if ((v & 0x7f800000) != 0x7f800000)
>>> return 0;
>>> return !(v & 0x007fffff);
>>> }
>>> +
>>> +static av_always_inline av_const int avpriv_isinf(double x)
>>> +{
>>> + uint64_t v = av_double2int(x);
>>> + if ((v & 0x7ff0000000000000) != 0x7ff0000000000000)
>>> + return 0;
>>> + return !(v & 0x000fffffffffffff);
>>> +}
>>> +
>>
>>> +static av_always_inline av_const int avpriv_isinfl(long double x)
>>> +{
>>> + // FIXME: just a stub, hard as long double width can vary between
>>> platforms
>>> + // Also currently irrelevant
>>> + return avpriv_isinf(x);
>>> +}
>>
>> i think we should not add any long double code. It could break
>> build and is unused
>
> long double is C89, C99 made it useful by adding support for it in
> stdlib with sinl etc.
> Nevertheless, since we don't use it, agreed. Still feel a comment with
> a TODO is useful.
> Will rework and post later.
After more detailed thought, I don't think a comment is necessary.
Rationale given in updated patch's commit message, just now sent.
>
>>
>> [...]
>> --
>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> Awnsering whenever a program halts or runs forever is
>> On a turing machine, in general impossible (turings halting problem).
>> On any real computer, always possible as a real computer has a finite number
>> of states N, and will either halt in less than N cycles or never halt.
>>
>> _______________________________________________
>> 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
