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

[Michael Niedermayer]

On Wed, Apr 16, 2014 at 11:22:59PM +1000, Matt Oliver wrote:
> Heres a different version that doesnt require some win32 black magic. I
> used the existing init flag as an atomic flag to correctly initialise the
> mutex on first use. There was already several checks on the init flag in
> various functions so this doesnt add much apart from changing those to
> atomics. This new patch will work with pthread/win32/os2.
> 
> There are 2 parts to the patch. The first adds an extra atomic function for
> 32b compare and exchange (the existing pointer version obviously wouldnt
> work in 64b). The second part is the changes to opencl itself.

>  atomic.c       |   20 ++++++++++++++++++++
>  atomic.h       |   10 ++++++++++
>  atomic_gcc.h   |    6 ++++++
>  atomic_suncc.h |    6 ++++++
>  atomic_win32.h |    6 ++++++
>  5 files changed, 48 insertions(+)
> ec4403589c8f1f5ff8660d6a81b2662ca797c072  0001-atomic-add-32b-integer-compare-and-swap-function.patch
> From 75d1b8d12d4bffc722e90a14bab10117e44cca70 Mon Sep 17 00:00:00 2001
> From: Matt Oliver <protogonoi at gmail.com>
> Date: Wed, 16 Apr 2014 18:50:35 +1000
> Subject: [PATCH 1/2] atomic: add 32b integer compare and swap function.
> 
> ---
>  libavutil/atomic.c       | 20 ++++++++++++++++++++
>  libavutil/atomic.h       | 10 ++++++++++
>  libavutil/atomic_gcc.h   |  6 ++++++
>  libavutil/atomic_suncc.h |  6 ++++++
>  libavutil/atomic_win32.h |  6 ++++++
>  5 files changed, 48 insertions(+)
> 
> diff --git a/libavutil/atomic.c b/libavutil/atomic.c
> index 1fa6308..7d3eacb 100644
> --- a/libavutil/atomic.c
> +++ b/libavutil/atomic.c
> @@ -59,6 +59,17 @@ int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int inc)
>      return res;
>  }
>  
> +int avpriv_atomic_cas(volatile int *ptr, int oldval, int newval)
[...]
>  void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval)

avpriv_atomic_cas() is a good name for a function that supports any
type, for int thought, int should be in the name like in the existing
functions


[...]
> @@ -321,14 +328,26 @@ 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 void init_opencl_ctx()
>  {
> -    int ret = 0;
> -    LOCK_OPENCL;
> +#if HAVE_THREADS
> +    if (!avpriv_atomic_cas( &opencl_ctx.opt_init_flag, 0, 1 )) {
> +        av_opt_set_defaults(&opencl_ctx);
> +        pthread_mutex_init(&opencl_ctx.atomic_opencl_lock, NULL);
> +    }

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- 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/20140416/63ea1ef3/attachment.asc>