[FFmpeg-devel] [PATCH 00/15] replace pow(10, x) by exp10(x) across FFmpeg

[Ganesh Ajjanagadde]

On Wed, Dec 23, 2015 at 4:39 PM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Wed, Dec 23, 2015 at 03:52:50PM -0800, Ganesh Ajjanagadde wrote:
>> On Wed, Dec 23, 2015 at 3:38 PM, Michael Niedermayer
>> <michael at niedermayer.cc> wrote:
>> > On Wed, Dec 23, 2015 at 10:47:20AM -0800, Ganesh Ajjanagadde wrote:
>> >> exp10(x) is superior to pow(10,x).
>> >> Note that in some cases, this may affect integers derived from pow calls. When
>> >> spotted, this remark has been added to the relevant commit message.
>> >>
>> >> Note that if such a thing is troublesome, one can do one of two things:
>> >> 1. leave the pow(10,x) call as is in such places.
>> >> 2. replace the lavu/libm fallback by the correct pow(10,x) as opposed to the
>> >> sloppy exp2(M_LOG2_10 * x).
>> >>
>> >> Patches tested with FATE on x86-64, GNU/Linux.
>> >
>> > some of these fail on arm
>> >
>> > i suspect thats because exp10 is only available with GNU_SOURCE
>> > and we dont define that nor do we want to define that
>> > -DGNU_SOURCE often leaks in from pkg_config files though
>>
>> ah, thanks for this. Out of curiousity, why don't we want to always
>> define it? My FFmpeg runs fine, built with GNU_SOURCE.
>
> it would hide any use of GNU features and FFmpeg should work
> on non GNU platforms too.
> Its best if any non standard (c99/posix) features trigger immedeate
> errors for developers instead of having to depend on the roundtrip
> over bug reports to find and remove such uses
>
>
>>
>> > my arm setup doesnt include many external libs so GNU_SOURCE doesnt
>> > get enabled
>>
>> It is only with GNU_SOURCE as per man page; I assumed that was handled
>> via configure and the like and I thought I mentioned this somewhere
>> during the discussion.
>
>
>
>
>>
>> >
>> > i didnt think deeply abot it but would it make sense to always enable
>> > the fallback even if exp10 is available? that is assuming the fallback
>> > is faster everywhere
>>
>> The fallback is faster everywhere. The trouble is I run into linker
>> issues when it is available natively.
>
> ifdef ... undef ...
> #define exp10(x) ...
> might work
> not sure this has other issues or not

Have not checked yet, see below for a different idea.

>
>
>> Better solution may be via an ifdef GNU_SOURCE in addition to HAVE_EXP10.
>> Can you please check if that works?
>
> I dont think a check based on GNU_SOURCE can work entirely reliably
> HAVE_EXP10 is set and the function should be available during link time
> but without GNU_SOURCE it is not available from the headers
> configure doesnt check the headers and the check is likely before it
> is known if GNU_SOURCE is set
> a libc could provide exp10 without GNU_SOURCE and a libc could lack
> exp10 even though GNU_SOURCE is set

Practically speaking, there is no libc (or more precisely libm)
providing exp10 apart from GNU.

What I meant was the following (I casually labelled it as ifdef, I
meant some preprocessor stuff):
#if !HAVE_EXP10 || !(defined(_GNU_SOURCE))
...
#endif

I have reproduced your issues via a minimal configure line, and the
above idea fixes those. Do you see any issues? Of course, there is the
theoretical one of a libc (more precisely libm) having exp10 without
GNU_SOURCE, but in that case it should just use the fallback.

>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Everything should be made as simple as possible, but not simpler.
> -- Albert Einstein
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>