[FFmpeg-devel] [PATCH] Avoid creating unecessary dependencies on thread libraries.

[Gregory J Wolfe]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
> Behalf Of wm4
> Sent: Monday, November 21, 2016 10:05 AM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] Avoid creating unecessary
> dependencies on thread libraries.
> 
> On Mon, 21 Nov 2016 09:58:33 -0500
> "Gregory J. Wolfe" <gregory.wolfe at kodakalaris.com> wrote:
> 
> > (1) Multi-threading support requires knowing the number of CPUs
> available.
> > When building with MinGW on a Windows system, both gcc and
> Windows run
> > time functions are available to get this information.  However, when
> Windows
> > threading has been selected, the Windows function should be used,
> not the
> > gcc function.  This avoids creating an unnecessary dependency on
> the gcc
> > thread library.
> >
> > (2) When ALL threading support is disabled, the build should not
> create a
> > dependency on ANY thread library.
> >
> > Signed-off-by: Gregory J. Wolfe <gregory.wolfe at kodakalaris.com>
> > ---
> >  libavutil/cpu.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavutil/cpu.c b/libavutil/cpu.c
> > index f5785fc..3843778 100644
> > --- a/libavutil/cpu.c
> > +++ b/libavutil/cpu.c
> > @@ -258,10 +258,15 @@ int av_cpu_count(void)
> >      static volatile int printed;
> >
> >      int nb_cpus = 1;
> > +#if HAVE_THREADS
> >  #if HAVE_WINRT
> >      SYSTEM_INFO sysinfo;
> >  #endif
> > -#if HAVE_SCHED_GETAFFINITY && defined(CPU_COUNT)
> > +    // if HAVE_W32THREADS and HAVE_GETPROCESSAFFINITYMASK,
> we will use
> > +    // Windows GetProcessAffinityMask() instead of gcc library
> function
> > +    // sched_getaffinity().  This avoids creating a dependency on the
> gcc
> > +    // thread library that we don't need/want.
> > +#if HAVE_SCHED_GETAFFINITY && defined(CPU_COUNT) &&
> !(HAVE_W32THREADS && HAVE_GETPROCESSAFFINITYMASK)
> >      cpu_set_t cpuset;
> >
> >      CPU_ZERO(&cpuset);
> > @@ -286,6 +291,7 @@ int av_cpu_count(void)
> >      GetNativeSystemInfo(&sysinfo);
> >      nb_cpus = sysinfo.dwNumberOfProcessors;
> >  #endif
> > +#endif
> >
> >      if (!printed) {
> >          av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n",
> nb_cpus);
> 
> Wouldn't it be better to move the HAVE_GETPROCESSAFFINITYMASK
> case
> above HAVE_SCHED_GETAFFINITY, and avoid making the ifdeffery
> mess
> worse? GetProcessAffinityMask is available on every supported
> Windows
> and could as well be replaced by "ifdef _WIN32" instead.
> 

Yeah, I had the same thought.  For the first go around I opted for the
solution that moved around the least code, not knowing for sure if
I might cause some undesirable side effects by reordering the existing
code.  I can easily redo the patch if we're in agreement on this point.

> Also, technically you might have to stop the configure script to stop
> linking to libpthread. I'm not sure what exactly stops it from doing
> that in your case.
 
The result I observed was that the resulting avutil.dll did NOT have
a dependency on the gcc run time.  My interpretation of this is that
if you link against a library, but don't reference any symbols in it, then
it's as if you never linked against it in the first place.

Greg W.