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

[Hendrik Leppkes]

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.