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

[Ganesh Ajjanagadde]

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.

>
> [...]
> --
> 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
>