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

[Marc Hoffman]

On Feb 22, 2007, at 6:24 AM, M?ns Rullg?rd wrote:

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

Ok so we have 2 issues.

1. configure, issue how to pick off if a system supports llrint  
library call.  I choose to use configure have with the check to  
assert we get a HAVE_LLRINT.

2. work around of the semantics of llrint for systems that don't have  
it.  I made a suggestion of a type of approach obviously you don't  
like which makes sense as to why.  Let me see now we truncate the  
result of llrint anyways to a long on output so something in the  
realm of what I have suggested should work.  And for systems that  
don't have llrint it is pretty much irrelevant anyways because the  
code just will not compile as is.

Would it be more acceptable to include the semantics for this  
function into ffmpeg if HAVE_LLRINT is not defined?

http://www.koders.com/c/fidD08BC2E4D564C3478A14FC47B0D8347A991C7396.aspx