[2/2] compat/w32pthreads: use the condition variable API directly when targeting newer versions of Windows

Message ID 1412820307-3800-1-git-send-email-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer Oct. 9, 2014, 2:05 a.m.
Wrap the function calls in a similar fashion to how it's being done
with the critical section API.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 compat/w32pthreads.h | 63 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 26 deletions(-)

Comments

Martin Storsjö Oct. 9, 2014, 8:03 a.m. | #1
On Wed, 8 Oct 2014, James Almer wrote:

> Wrap the function calls in a similar fashion to how it's being done
> with the critical section API.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> compat/w32pthreads.h | 63 ++++++++++++++++++++++++++++++----------------------
> 1 file changed, 37 insertions(+), 26 deletions(-)
>
> diff --git a/compat/w32pthreads.h b/compat/w32pthreads.h
> index b905a95..e586ecb 100644
> --- a/compat/w32pthreads.h
> +++ b/compat/w32pthreads.h
> @@ -64,32 +64,6 @@ typedef struct pthread_cond_t {
> } pthread_cond_t;
> #endif
>
> -/* function pointers to conditional variable API on windows 6.0+ kernels */
> -#if _WIN32_WINNT < 0x0600
> -static void (WINAPI *cond_broadcast)(pthread_cond_t *cond);
> -static void (WINAPI *cond_init)(pthread_cond_t *cond);
> -static void (WINAPI *cond_signal)(pthread_cond_t *cond);
> -static BOOL (WINAPI *cond_wait)(pthread_cond_t *cond, pthread_mutex_t *mutex,
> -                                DWORD milliseconds);
> -#else
> -#define cond_init      InitializeConditionVariable
> -#define cond_broadcast WakeAllConditionVariable
> -#define cond_signal    WakeConditionVariable
> -#define cond_wait      SleepConditionVariableCS
> -
> -#define CreateEvent(a, reset, init, name)                   \
> -    CreateEventEx(a, name,                                  \
> -                  (reset ? CREATE_EVENT_MANUAL_RESET : 0) | \
> -                  (init ? CREATE_EVENT_INITIAL_SET : 0),    \
> -                  EVENT_ALL_ACCESS)
> -// CreateSemaphoreExA seems to be desktop-only, but as long as we don't
> -// use named semaphores, it doesn't matter if we use the W version.
> -#define CreateSemaphore(a, b, c, d) \
> -    CreateSemaphoreExW(a, b, c, d, 0, SEMAPHORE_ALL_ACCESS)
> -#define InitializeCriticalSection(x) InitializeCriticalSectionEx(x, 0, 0)
> -#define WaitForSingleObject(a, b) WaitForSingleObjectEx(a, b, FALSE)
> -#endif
> -

Where did the 
CreateEvent/CreateSemaphore/InitializeCriticalSection/WaitForSingleObject 
definitions go here? When targeting desktop windows they don't matter 
(since the old functions still exist), but when targeting WinRT/WinPhone, 
the old functions are no longer available.

// Martin
James Almer Oct. 9, 2014, 3:21 p.m. | #2
On 09/10/14 5:03 AM, Martin Storsjö wrote:
> On Wed, 8 Oct 2014, James Almer wrote:
> 
>> Wrap the function calls in a similar fashion to how it's being done
>> with the critical section API.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> compat/w32pthreads.h | 63 ++++++++++++++++++++++++++++++----------------------
>> 1 file changed, 37 insertions(+), 26 deletions(-)
>>
>> diff --git a/compat/w32pthreads.h b/compat/w32pthreads.h
>> index b905a95..e586ecb 100644
>> --- a/compat/w32pthreads.h
>> +++ b/compat/w32pthreads.h
>> @@ -64,32 +64,6 @@ typedef struct pthread_cond_t {
>> } pthread_cond_t;
>> #endif
>>
>> -/* function pointers to conditional variable API on windows 6.0+ kernels */
>> -#if _WIN32_WINNT < 0x0600
>> -static void (WINAPI *cond_broadcast)(pthread_cond_t *cond);
>> -static void (WINAPI *cond_init)(pthread_cond_t *cond);
>> -static void (WINAPI *cond_signal)(pthread_cond_t *cond);
>> -static BOOL (WINAPI *cond_wait)(pthread_cond_t *cond, pthread_mutex_t *mutex,
>> -                                DWORD milliseconds);
>> -#else
>> -#define cond_init      InitializeConditionVariable
>> -#define cond_broadcast WakeAllConditionVariable
>> -#define cond_signal    WakeConditionVariable
>> -#define cond_wait      SleepConditionVariableCS
>> -
>> -#define CreateEvent(a, reset, init, name)                   \
>> -    CreateEventEx(a, name,                                  \
>> -                  (reset ? CREATE_EVENT_MANUAL_RESET : 0) | \
>> -                  (init ? CREATE_EVENT_INITIAL_SET : 0),    \
>> -                  EVENT_ALL_ACCESS)
>> -// CreateSemaphoreExA seems to be desktop-only, but as long as we don't
>> -// use named semaphores, it doesn't matter if we use the W version.
>> -#define CreateSemaphore(a, b, c, d) \
>> -    CreateSemaphoreExW(a, b, c, d, 0, SEMAPHORE_ALL_ACCESS)
>> -#define InitializeCriticalSection(x) InitializeCriticalSectionEx(x, 0, 0)
>> -#define WaitForSingleObject(a, b) WaitForSingleObjectEx(a, b, FALSE)
>> -#endif
>> -
> 
> Where did the CreateEvent/CreateSemaphore/InitializeCriticalSection/WaitForSingleObject definitions go here? When targeting desktop windows they don't matter (since the old functions still exist), but when targeting WinRT/WinPhone, the old functions are no longer available.

You're right about InitializeCriticalSection and WaitForSingleObject (I somehow missed those), but 
the redefinition of CreateEvent and CreateSemaphore are not needed anymore since they will now be 
used only for the non-native version of the condition variable API, which is only compiled when 
_WIN32_WINT < 0x0600.

I'll send a patch to put the former two back in place.
Martin Storsjö Oct. 9, 2014, 4:22 p.m. | #3
On Thu, 9 Oct 2014, James Almer wrote:

> On 09/10/14 5:03 AM, Martin Storsjö wrote:
>> On Wed, 8 Oct 2014, James Almer wrote:
>>
>>> Wrap the function calls in a similar fashion to how it's being done
>>> with the critical section API.
>>>
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>> compat/w32pthreads.h | 63 ++++++++++++++++++++++++++++++----------------------
>>> 1 file changed, 37 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/compat/w32pthreads.h b/compat/w32pthreads.h
>>> index b905a95..e586ecb 100644
>>> --- a/compat/w32pthreads.h
>>> +++ b/compat/w32pthreads.h
>>> @@ -64,32 +64,6 @@ typedef struct pthread_cond_t {
>>> } pthread_cond_t;
>>> #endif
>>>
>>> -/* function pointers to conditional variable API on windows 6.0+ kernels */
>>> -#if _WIN32_WINNT < 0x0600
>>> -static void (WINAPI *cond_broadcast)(pthread_cond_t *cond);
>>> -static void (WINAPI *cond_init)(pthread_cond_t *cond);
>>> -static void (WINAPI *cond_signal)(pthread_cond_t *cond);
>>> -static BOOL (WINAPI *cond_wait)(pthread_cond_t *cond, pthread_mutex_t *mutex,
>>> -                                DWORD milliseconds);
>>> -#else
>>> -#define cond_init      InitializeConditionVariable
>>> -#define cond_broadcast WakeAllConditionVariable
>>> -#define cond_signal    WakeConditionVariable
>>> -#define cond_wait      SleepConditionVariableCS
>>> -
>>> -#define CreateEvent(a, reset, init, name)                   \
>>> -    CreateEventEx(a, name,                                  \
>>> -                  (reset ? CREATE_EVENT_MANUAL_RESET : 0) | \
>>> -                  (init ? CREATE_EVENT_INITIAL_SET : 0),    \
>>> -                  EVENT_ALL_ACCESS)
>>> -// CreateSemaphoreExA seems to be desktop-only, but as long as we don't
>>> -// use named semaphores, it doesn't matter if we use the W version.
>>> -#define CreateSemaphore(a, b, c, d) \
>>> -    CreateSemaphoreExW(a, b, c, d, 0, SEMAPHORE_ALL_ACCESS)
>>> -#define InitializeCriticalSection(x) InitializeCriticalSectionEx(x, 0, 0)
>>> -#define WaitForSingleObject(a, b) WaitForSingleObjectEx(a, b, FALSE)
>>> -#endif
>>> -
>>
>> Where did the CreateEvent/CreateSemaphore/InitializeCriticalSection/WaitForSingleObject definitions go here? When targeting desktop windows they don't matter (since the old functions still exist), but when targeting WinRT/WinPhone, the old functions are no longer available.
>
> You're right about InitializeCriticalSection and WaitForSingleObject (I somehow missed those), but
> the redefinition of CreateEvent and CreateSemaphore are not needed anymore since they will now be
> used only for the non-native version of the condition variable API, which is only compiled when
> _WIN32_WINT < 0x0600.

Ah, I see.

// Martin

Patch

diff --git a/compat/w32pthreads.h b/compat/w32pthreads.h
index b905a95..e586ecb 100644
--- a/compat/w32pthreads.h
+++ b/compat/w32pthreads.h
@@ -64,32 +64,6 @@  typedef struct pthread_cond_t {
 } pthread_cond_t;
 #endif
 
-/* function pointers to conditional variable API on windows 6.0+ kernels */
-#if _WIN32_WINNT < 0x0600
-static void (WINAPI *cond_broadcast)(pthread_cond_t *cond);
-static void (WINAPI *cond_init)(pthread_cond_t *cond);
-static void (WINAPI *cond_signal)(pthread_cond_t *cond);
-static BOOL (WINAPI *cond_wait)(pthread_cond_t *cond, pthread_mutex_t *mutex,
-                                DWORD milliseconds);
-#else
-#define cond_init      InitializeConditionVariable
-#define cond_broadcast WakeAllConditionVariable
-#define cond_signal    WakeConditionVariable
-#define cond_wait      SleepConditionVariableCS
-
-#define CreateEvent(a, reset, init, name)                   \
-    CreateEventEx(a, name,                                  \
-                  (reset ? CREATE_EVENT_MANUAL_RESET : 0) | \
-                  (init ? CREATE_EVENT_INITIAL_SET : 0),    \
-                  EVENT_ALL_ACCESS)
-// CreateSemaphoreExA seems to be desktop-only, but as long as we don't
-// use named semaphores, it doesn't matter if we use the W version.
-#define CreateSemaphore(a, b, c, d) \
-    CreateSemaphoreExW(a, b, c, d, 0, SEMAPHORE_ALL_ACCESS)
-#define InitializeCriticalSection(x) InitializeCriticalSectionEx(x, 0, 0)
-#define WaitForSingleObject(a, b) WaitForSingleObjectEx(a, b, FALSE)
-#endif
-
 static av_unused unsigned __stdcall attribute_align_arg win32thread_worker(void *arg)
 {
     pthread_t *h = arg;
@@ -138,6 +112,35 @@  static inline int pthread_mutex_unlock(pthread_mutex_t *m)
     return 0;
 }
 
+#if _WIN32_WINNT >= 0x0600
+static inline void pthread_cond_init(pthread_cond_t *cond, const void *unused_attr)
+{
+    InitializeConditionVariable(cond);
+}
+
+/* native condition variables do not destroy */
+static inline void pthread_cond_destroy(pthread_cond_t *cond)
+{
+    return;
+}
+
+static inline void pthread_cond_broadcast(pthread_cond_t *cond)
+{
+    WakeAllConditionVariable(cond);
+}
+
+static inline int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex)
+{
+    SleepConditionVariableCS(cond, mutex, INFINITE);
+    return 0;
+}
+
+static inline void pthread_cond_signal(pthread_cond_t *cond)
+{
+    WakeConditionVariable(cond);
+}
+
+#else // _WIN32_WINNT < 0x0600
 /* for pre-Windows 6.0 platforms we need to define and use our own condition
  * variable and api */
 typedef struct  win32_cond_t {
@@ -149,6 +152,13 @@  typedef struct  win32_cond_t {
     volatile int is_broadcast;
 } win32_cond_t;
 
+/* function pointers to conditional variable API on windows 6.0+ kernels */
+static void (WINAPI *cond_broadcast)(pthread_cond_t *cond);
+static void (WINAPI *cond_init)(pthread_cond_t *cond);
+static void (WINAPI *cond_signal)(pthread_cond_t *cond);
+static BOOL (WINAPI *cond_wait)(pthread_cond_t *cond, pthread_mutex_t *mutex,
+                                DWORD milliseconds);
+
 static av_unused void pthread_cond_init(pthread_cond_t *cond, const void *unused_attr)
 {
     win32_cond_t *win32_cond = NULL;
@@ -276,6 +286,7 @@  static av_unused void pthread_cond_signal(pthread_cond_t *cond)
 
     pthread_mutex_unlock(&win32_cond->mtx_broadcast);
 }
+#endif
 
 static av_unused void w32thread_init(void)
 {