[Ffmpeg-devel] llrint is referenced in mpegaudiodec.c

[Måns Rullgård]

Marc Hoffman said:
>
> On Feb 21, 2007, at 7:18 PM, M?ns Rullg?rd wrote:
>
>> Marc Hoffman <mmh at pleasantst.com> writes:
>>
>>> The function llrint is referenced in mpegaudiodec.c, this should be
>>> wrapped in the same manner as lrint because some systems don't have
>>> this feature.
>>>
>>> mpegaudiodec.c  llrint -> lrint
>>>
>>> Can I get some clarity about how the group would like to solve this
>>> issue.  For Blackfin we don't have this llrint which I believe in
>>> this particular example could be defined as lrint anyways.
>>
>> I've said it before, and I'll say it again: llrint() can NOT be
>> replaced by lrint() there.  Search the archives for discussions about
>> why.
>>
>
> Sorry to force a painful visit to the past where you had made a
> similar observation to me only a couple of months prior.
>
> _IIRC_ some values didnt fit in an 32bit int and caused a floating
> point exception, adding a check for >MAX should work too but its
> more complex ...
>
> This is my inteneded use of the config define included in the patch
> is this right track/acceptable?
>
> #ifndef HAVE_LLRINT
> #define MAX_SINT 0x7fffffff
> #define MIN_SINT 0x80000000

#include <limits.h> => INT_MAX, INT_MIN

> #define llrint(x) lrint(((x)>MAX_SINT?(MAX_SINT):(x)<MIN_SINT? MIN_SINT:(x)))

This is very wrong.  You are clipping to the signed 32-bit range, even though
some values are in the range INT_MAX < x <= UINT_MAX.

> patch needed for the configuration management stuff independent to
> solution.

Needlessly complicated.  The lrint test is done that way to detect some known
buggy implementations.  We have no reason to believe that llrint would have
the same bugs.

-- 
M?ns Rullg?rd
mans at mansr.com