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

Ganesh Ajjanagadde gajjanag at mit.edu
Tue Nov 17 22:55:39 CET 2015


On Mon, Nov 16, 2015 at 7:50 AM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> On Mon, Nov 16, 2015 at 6:55 AM, Michael Niedermayer
> <michael at 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 <gajjanagadde at 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 at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>


More information about the ffmpeg-devel mailing list