[FFmpeg-devel] [PATCH] lavu/opencl: ticket #2422 opencl using dynamic mutex

[Wei Gao]

2013/6/29 James Almer <jamrial at gmail.com>

> On 28/06/13 7:17 AM, Wei Gao wrote:
> > Hi
> >
> > The attachment is about the opencl using dynamic mutex, I have tested on
> my
> > comoputer in pthread, I can't compile a win32 version, can anyone help to
> > test?
> >
> > Thanks
> > Best regards
>
> > diff --git a/libavutil/opencl.c b/libavutil/opencl.c
> > index 5887b50..e5de2a7 100644
> > --- a/libavutil/opencl.c
> > +++ b/libavutil/opencl.c
> > @@ -25,18 +25,24 @@
> >  #include "log.h"
> >  #include "avassert.h"
> >  #include "opt.h"
> > +#include "atomic.h"
> >
> >  #if HAVE_PTHREADS
> > -
> >  #include <pthread.h>
> > -static pthread_mutex_t atomic_opencl_lock = PTHREAD_MUTEX_INITIALIZER;
> > +#elif HAVE_W32THREADS
> > +#include "compat/w32pthreads.h"
> > +#elif HAVE_OS2THREADS
> > +#include "compat/os2threads.h"
> > +#endif
>
> I don't think this is necessary as it was established that OpenCL is not
> available on OS2.
>
>
> >
> > -#define LOCK_OPENCL pthread_mutex_lock(&atomic_opencl_lock)
> > -#define UNLOCK_OPENCL pthread_mutex_unlock(&atomic_opencl_lock)
> >
> > -#elif !HAVE_THREADS
> > +#if !HAVE_THREADS
> >  #define LOCK_OPENCL
> >  #define UNLOCK_OPENCL
> > +#else
> > +static pthread_mutex_t atomic_opencl_lock = NULL;
>
> Needs to be a pointer.
> The idea is that it will point to the first mutex that gets allocated and
> initialized in
> init_opencl_mutex() below.
>
> I have tried the pointer, but it can't run correct. I will test later

>
> > +#define LOCK_OPENCL pthread_mutex_lock(&atomic_opencl_lock)
> > +#define UNLOCK_OPENCL pthread_mutex_unlock(&atomic_opencl_lock)
> >  #endif
>
> Thus no need for & here.
>
> If I use  static pthread_mutex_t atomic_opencl_lock, it should be like
this

>
> > @@ -62,6 +68,7 @@ typedef struct {
> >      int platform_idx;
> >      int device_idx;
> >      char *build_options;
> > +    pthread_mutex_t *opencl_mutex;
>
> This is not necessary. atomic_opencl_lock is enough.
>
> I use it to release the memory which should be release at uninit

>
> > +static int init_opencl_mutex(void)
> > +{
> > +#if HAVE_THREADS
> > +    pthread_mutex_t *opencl_mutex;
> > +    opencl_mutex = av_mallocz(sizeof(pthread_mutex_t));
> > +    if (!opencl_mutex) {
> > +        return AVERROR(ENOMEM);
> > +    }
> > +    pthread_mutex_init(opencl_mutex, NULL);
> > +    if (avpriv_atomic_ptr_cas((void * volatile *)&atomic_opencl_lock,
> NULL, opencl_mutex)) {
>
> This line is ok, assuming atomic_opencl_lock is a pointer.
>
> If I use  static pthread_mutex_t *atomic_opencl_lock = NULL;, should it
be avpriv_atomic_ptr_cas((void * volatile *)&atomic_opencl_lock, NULL,
&opencl_mutex), right?

>
> > +    if (opencl_mutex)
> > +        opencl_ctx.opencl_mutex = opencl_mutex;
>
> No need for this as i mentioned earlier.
>
>
> OK


> > +    return 0;
> > +#else
> > +    return 0;
> > +#endif
>
> You can simplify this into
> #endif
>     return 0;
>
>
> OK

> > +static void uninit_opencl_mutex(void)
> > +{
> > +#if HAVE_THREADS
> > +    if (atomic_opencl_lock) {
> > +        pthread_mutex_destroy(&atomic_opencl_lock);
>
> No need for & here either.
>
>
> > +        av_freep(&opencl_ctx.opencl_mutex);
> > +    }
> > +#endif
>
> av_freep(&atomic_opencl_lock);
>
>
> > @@ -588,6 +630,10 @@ static int compile_kernel_file(OpenclContext
> *opencl_ctx)
> >  int av_opencl_init(AVOpenCLExternalEnv *ext_opencl_env)
> >  {
> >      int ret = 0;
> > +    //check whether the mutex has been inited
>
> Initialized.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>