[x265] Fwd: [PATCH] Threadpool: correctly detect 'mac' os

[Praveen Tiwari]

---------- Forwarded message ----------
From: Pradeep Ramachandran <pradeep at multicorewareinc.com>
Date: Fri, Feb 3, 2017 at 10:48 AM
Subject: Re: [x265] [PATCH] Threadpool: correctly detect 'mac' os
To: Development for x265 <x265-devel at videolan.org>



On Thu, Feb 2, 2017 at 7:51 PM, <praveen at multicorewareinc.com> wrote:

> # HG changeset patch
> # User Praveen Tiwari <praveen at multicorewareinc.com>
> # Date 1486045258 -19800
> #      Thu Feb 02 19:50:58 2017 +0530
> # Node ID 573841e2efb5a68e9d4951bf287c7a64dfa72823
> # Parent  fa52b516f5ff122c4486ed840c5bf1de3c06e974
> Threadpool: correctly detect 'mac' os
>
> diff -r fa52b516f5ff -r 573841e2efb5 source/common/threadpool.cpp
> --- a/source/common/threadpool.cpp      Mon Jan 30 10:35:00 2017 +0530
> +++ b/source/common/threadpool.cpp      Thu Feb 02 19:50:58 2017 +0530
> @@ -57,7 +57,7 @@
>
>  #endif
>
> -#if MACOS
> +#if __APPLE__ && __MACH__
>

It would be better to edit CMakelists.txt to add_definition(-DMACOS) when
the right pre-processor flags that are auto detected by the compiler are
enabled. Keeps the code inside x265 cleaner.

​​Renaming a system defined macro did not make more sense to me rather it
confusing. In case of bug, you always doubt the code under it rather than
checking a predefined macro in Cmake files. As in this case, the code was
counting the right number for cores but system detection by macro
​
MACOS was wrong. System defined macro are easily know to wide range of
users.  ​

>From this document (https://developer.apple.com/l
ibrary/content/documentation/Porting/Conceptual/PortingUnix/
compiling/compiling.html#//apple_ref/doc/uid/TP40002850-SW13) it looks like
__APPLE__ is the only standard one; we don't use any MacOS system calls
directly to make __MACH__ mandatory. Is there some other source that says
that __MACH__ is also required?

​​In fact we are using system call "sysctl(nm, 2, &count, &len, NULL, 0)" ​how
we are going to detect it ? __UNIX__ is not define in os x.​


 #include <sys/param.h>
>  #include <sys/sysctl.h>
>  #endif
> @@ -617,7 +617,7 @@
>      return sysconf(_SC_NPROCESSORS_CONF);
>  #elif __unix__
>      return sysconf(_SC_NPROCESSORS_ONLN);
> -#elif MACOS
> +#elif __APPLE__ && __MACH__
>      int nm[2];
>      size_t len = 4;
>      uint32_t count;
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel
>


_______________________________________________
x265-devel mailing list
x265-devel at videolan.org
https://mailman.videolan.org/listinfo/x265-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20170203/8773804c/attachment.html>