[FFmpeg-devel] Patch: Enable OpenCL with Win32 threads

[Michael Niedermayer]

On Thu, Apr 17, 2014 at 01:46:36PM +1000, Matt Oliver wrote:
> >
> > Could this just be integrated in the pthread_mutex_lock win32 wrapper
> > code to emulate PTHREAD_MUTEX_INITIALIZER?
> 
> 
> It probably shouldn't as it adds extra checks that are not necessary if you
> did the appropriate _init calls anyway (also I did it wrong ;) ). Win32
> already does an internal check on the -1 waiters value which is what I used
> in the first patch that added a static initialisation for win32.
> 
> 
> > this is not thread safe at all
> >
> > thread #1  avpriv_atomic_cas
> > thread #2                    avpriv_atomic_cas (if skiped) LOCK_OPENCL use
> > of uninitialized mutex
> >
> > please see the existing code, that sets up global mutexes
> > in the lock manager for example
> >
> 
> This is one of those things were I knew that and knew better but for some
> reason did it anyway (good reason not to submit patches late at night). It
> needed a second atomic and wait to work properly. Anyway attached is a
> patch that initialises in the same way the existing lock manager does which
> should be more acceptable and consistent with existing code. Its a little
> more complicated than static initialisation but if the first static patch
> with the win32 static initializer isnt suitable then this seems the most
> consistent way to support all native thread implementations.

>  opencl.c |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++-------------
>  opencl.h |    1 +
>  2 files changed, 51 insertions(+), 13 deletions(-)
> 1b908cff3181f07226e68c0ed2fa647517dcfae7  opencl-add-support-for-non-pthread-locking.patch
> From 7644090cf14982a700859b6ae23724f3a1800ace Mon Sep 17 00:00:00 2001
> From: Matt Oliver <protogonoi at gmail.com>
> Date: Thu, 17 Apr 2014 13:21:48 +1000
> Subject: [PATCH] opencl: add support for non-pthread locking
> 
> ---
>  libavutil/opencl.c | 63 +++++++++++++++++++++++++++++++++++++++++++-----------
>  libavutil/opencl.h |  1 +
>  2 files changed, 51 insertions(+), 13 deletions(-)
> 
> diff --git a/libavutil/opencl.c b/libavutil/opencl.c
> index 142c6b0..5a3b85f 100644
> --- a/libavutil/opencl.c
> +++ b/libavutil/opencl.c
> @@ -27,15 +27,21 @@
>  #include "avassert.h"
>  #include "opt.h"
>  
> +#if HAVE_THREADS
>  #if HAVE_PTHREADS
> -
>  #include <pthread.h>
> -static pthread_mutex_t atomic_opencl_lock = PTHREAD_MUTEX_INITIALIZER;
> -
> -#define LOCK_OPENCL pthread_mutex_lock(&atomic_opencl_lock)
> -#define UNLOCK_OPENCL pthread_mutex_unlock(&atomic_opencl_lock)
> +#elif HAVE_W32THREADS
> +#include "compat/w32pthreads.h"
> +#elif HAVE_OS2THREADS
> +#include "compat/os2threads.h"
> +#endif
> +#include "atomic.h"
>  
> -#elif !HAVE_THREADS
> +#define MUTEX_OPENCL pthread_mutex_t *atomic_opencl_lock;
> +#define LOCK_OPENCL pthread_mutex_lock(opencl_ctx.atomic_opencl_lock)
> +#define UNLOCK_OPENCL pthread_mutex_unlock(opencl_ctx.atomic_opencl_lock)
> +#else
> +#define MUTEX_OPENCL
>  #define LOCK_OPENCL
>  #define UNLOCK_OPENCL
>  #endif
> @@ -74,6 +80,7 @@ typedef struct {
>      int kernel_code_count;
>      KernelCode kernel_code[MAX_KERNEL_CODE_NUM];
>      AVOpenCLDeviceList device_list;
> +    MUTEX_OPENCL
>  } OpenclContext;
>  
>  #define OFFSET(x) offsetof(OpenclContext, x)
> @@ -321,14 +328,44 @@ void av_opencl_free_device_list(AVOpenCLDeviceList **device_list)
>      av_freep(device_list);
>  }
>  
> -int av_opencl_set_option(const char *key, const char *val)
> +inline int init_opencl_ctx(void)
>  {
> -    int ret = 0;
> -    LOCK_OPENCL;
> +#if HAVE_THREADS

> +    if (!opencl_ctx.atomic_opencl_lock) {

Thread #1 if(1), Thread #2 if(1)

> +        pthread_mutex_t *tmp = av_malloc(sizeof(pthread_mutex_t));
> +        int err;
> +        if (!tmp)
> +            return AVERROR(ENOMEM);
> +        if ((err = pthread_mutex_init(tmp, NULL))) {
> +            av_free(tmp);
> +            return AVERROR(err);
> +        }

> +        if (!avpriv_atomic_ptr_cas(opencl_ctx.atomic_opencl_lock, NULL, tmp)) {

Thread #1 if(1), Thread #2 if(0)
Thread #1 stops here, by choice of the scheduler

> +            LOCK_OPENCL;
> +            av_opt_set_defaults(&opencl_ctx);
> +            opencl_ctx.opt_init_flag = 1;
> +            UNLOCK_OPENCL;
> +        }
> +        else {
> +            pthread_mutex_destroy(tmp);
> +            av_free(tmp);
> +        }

Thread #2 continues on without av_opt_set_defaults() having been
executed


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140418/03fb4ad8/attachment.asc>