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

2016-11-28 Thread Gregory J Wolfe 
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf Of Carl Eugen Hoyos
> Sent: Monday, November 21, 2016 8:12 PM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] Avoid creating unecessary
> dependencies on thread libraries.
> 
> 2016-11-21 19:52 GMT+01:00 Gregory J. Wolfe
> <gregory.wo...@kodakalaris.com>:
> 
> > (2) When ALL threading support is disabled, the build should not
> create a
> > dependency on ANY thread library.
> 
> If this done through "#if HAVE_THREADS", it would be preferable to
> split the patches imo.
> 
> Carl Eugen

Back from Thanksgiving holiday.  Will split patch into two and resubmit.

Greg W.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-11-21 Thread Carl Eugen Hoyos 
2016-11-21 19:52 GMT+01:00 Gregory J. Wolfe :

> (2) When ALL threading support is disabled, the build should not create a
> dependency on ANY thread library.

If this done through "#if HAVE_THREADS", it would be preferable to
split the patches imo.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-11-21 Thread Gregory J. Wolfe 
(1) Multi-threading support requires knowing the number of CPUs available.
When building with MinGW on a Windows system, both Windows and gcc run
time functions are available to get this information.  If available,
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 
---
 libavutil/cpu.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/libavutil/cpu.c b/libavutil/cpu.c
index f5785fc..a55921a 100644
--- a/libavutil/cpu.c
+++ b/libavutil/cpu.c
@@ -258,20 +258,25 @@ 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_GETPROCESSAFFINITYMASK, we will use Windows
+// GetProcessAffinityMask() over gcc library function
+// sched_getaffinity().  This avoids creating a dependency
+// on the gcc thread library that we don't need/want.
+#if HAVE_GETPROCESSAFFINITYMASK
+DWORD_PTR proc_aff, sys_aff;
+if (GetProcessAffinityMask(GetCurrentProcess(), _aff, _aff))
+nb_cpus = av_popcount64(proc_aff);
+#elif HAVE_SCHED_GETAFFINITY && defined(CPU_COUNT)
 cpu_set_t cpuset;
 
 CPU_ZERO();
 
 if (!sched_getaffinity(0, sizeof(cpuset), ))
 nb_cpus = CPU_COUNT();
-#elif HAVE_GETPROCESSAFFINITYMASK
-DWORD_PTR proc_aff, sys_aff;
-if (GetProcessAffinityMask(GetCurrentProcess(), _aff, _aff))
-nb_cpus = av_popcount64(proc_aff);
 #elif HAVE_SYSCTL && defined(HW_NCPU)
 int mib[2] = { CTL_HW, HW_NCPU };
 size_t len = sizeof(nb_cpus);
@@ -286,6 +291,7 @@ int av_cpu_count(void)
 GetNativeSystemInfo();
 nb_cpus = sysinfo.dwNumberOfProcessors;
 #endif
+#endif
 
 if (!printed) {
 av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n", nb_cpus);
-- 
2.5.1.windows.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-11-21 Thread Hendrik Leppkes 
On Mon, Nov 21, 2016 at 5:23 PM, Gregory J Wolfe
<gregory.wo...@kodakalaris.com> wrote:
>> -Original Message-
>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
>> Behalf Of wm4
>> Sent: Monday, November 21, 2016 10:05 AM
>> To: ffmpeg-devel@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.wo...@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.wo...@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();
>> > @@ -286,6 +291,7 @@ int av_cpu_count(void)
>> >  GetNativeSystemInfo();
>> >  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.
>

Moving the code to prefer the win32 functions if available is
definitely preferable to further complicating the ifdefs to not only
check for features but also doing negative checks on other features.
The preferred option should just be on top - which in this case is the
OS-native function over the pthread functions, which are always some
sort of emulation layer ontop of the win32 functions anyway.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-11-21 Thread wm4 
On Mon, 21 Nov 2016 16:23:03 +
Gregory J Wolfe <gregory.wo...@kodakalaris.com> wrote:

> > -Original Message-
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > Behalf Of wm4
> > Sent: Monday, November 21, 2016 10:05 AM
> > To: ffmpeg-devel@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.wo...@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.wo...@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();
> > > @@ -286,6 +291,7 @@ int av_cpu_count(void)
> > >  GetNativeSystemInfo();
> > >  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.

Depends if there are any cargo cult people who are against it.

> > 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.

This could still be caused by certain link flag (forgot which one). In
general, linking against a dynamic library without using any of its
functions, could still have side effects, such as registering plugins
in the DLL init functions and such.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-11-21 Thread Gregory J Wolfe 
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf Of wm4
> Sent: Monday, November 21, 2016 10:05 AM
> To: ffmpeg-devel@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.wo...@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.wo...@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();
> > @@ -286,6 +291,7 @@ int av_cpu_count(void)
> >  GetNativeSystemInfo();
> >  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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-11-21 Thread wm4 
On Mon, 21 Nov 2016 09:58:33 -0500
"Gregory J. Wolfe"  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 
> ---
>  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();
> @@ -286,6 +291,7 @@ int av_cpu_count(void)
>  GetNativeSystemInfo();
>  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.

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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2016-11-21 Thread Gregory J. Wolfe 
(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 
---
 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();
@@ -286,6 +291,7 @@ int av_cpu_count(void)
 GetNativeSystemInfo();
 nb_cpus = sysinfo.dwNumberOfProcessors;
 #endif
+#endif
 
 if (!printed) {
 av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n", nb_cpus);
-- 
2.5.1.windows.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel