On Thu, Dec 24, 2015 at 11:05 AM, Hendrik Leppkes <h.lepp...@gmail.com> wrote: > Am 24.12.2015 20:01 schrieb "Ganesh Ajjanagadde" <gajja...@mit.edu>: >> >> On Thu, Dec 24, 2015 at 10:52 AM, Hendrik Leppkes <h.lepp...@gmail.com> > wrote: >> > Am 24.12.2015 19:34 schrieb "Ganesh Ajjanagadde" <gajjanaga...@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 <gajjanaga...@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@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel