[FFmpeg-devel] [PATCH] lavu/libm: change macros to functions

[Ganesh Ajjanagadde]

On Thu, Dec 24, 2015 at 11:05 AM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> Am 24.12.2015 20:01 schrieb "Ganesh Ajjanagadde" <gajjanag at mit.edu>:
>>
>> On Thu, Dec 24, 2015 at 10:52 AM, Hendrik Leppkes <h.leppkes at gmail.com>
> wrote:
>> > Am 24.12.2015 19:34 schrieb "Ganesh Ajjanagadde" <gajjanagadde at gmail.com
>>:
>> >>
>> >> In the standard library, these are functions. We should match it; there
>> >> is no reason for these to be macros.
>> >>
>> >> While at it, add some trivial comments for readability and correct an
>> >> incorrect (at standard double precision) M_LN2 constant used in the
> exp2
>> >> fallback.
>> >>
>> >> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> >> ---
>> [...]
>> >>
>> >>  #if !HAVE_SINF
>> >> -#undef sinf
>> >> -#define sinf(x) ((float)sin(x))
>> >> -#endif
>> >> +static av_always_inline float sinf(float x)
>> >> +{
>> >> +    return sin(x);
>> >> +}
>> >> +#endif /* HAVE_SINF */
>> >>
>> >>  #if !HAVE_RINT
>> >>  static inline double rint(double x)
>> >> --
>> >> 2.6.4
>> >>
>> >
>> > Could this cause linkage issues if presence of such a function is
>> > mis-detected? Previously this worked fine. There was at least one such
> case
>> > where log2 IIRC was explicitly disabled in configure because its in the
>> > libc library but forgotten in the headers in one specific libc.
>>
>> I believe it does, for instance if I force the fallback via e.g a
>> #define HAVE_RINT 0 on my machine, it fails to link.
>> However, this is really the job of configure: configure's task is to
>> ensure the lack of misdetection. Using a macro is an ad-hoc solution
>> to a more fundamental issue.
>> On this note, see the thread regarding exp10, exp10f: while sorting
>> them out, I want to do it as cleanly as possible.
>> If there are outstanding issues regarding misdetection, they need to
>> be fixed before this can go in.
>>
>>
>
> But consider the case I touched on above - log2 is in the library but not
> in the header - hence log2 cannot be used, proper detection would turn it
> off.
>
> But if this particular case then causes linkage problems, how would we ever
> fix this in configure?
>
> Sounds like increasing the pain, not necessarily reducing it.

I guess so. However, I think the log2 example you give is the
exception, and not the rule. The thing is: at the moment the header is
a mess: some of them are macros, some are functions, and there is
essentially no rationale in the commit messages or as comments
regarding when a macro is used.

Sounds like this will need to be done slowly, or not at all. Maybe I
will just accept defeat here, and just try on getting exp10, exp10f
through. Will push the trivial aspects of the patch though (incorrect
log(2) constant, comments).

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel