[FFmpeg-devel] [PATCH] avcodec/utils: use a default lock manager that uses a pthread mutex

[Michael Niedermayer]

On Fri, Oct 18, 2013 at 09:56:02PM +0200, Michael Niedermayer wrote:
> On Fri, Oct 18, 2013 at 09:42:55PM +0200, wm4 wrote:
> > On Fri, 18 Oct 2013 21:04:47 +0200
> > Michael Niedermayer <michaelni at gmx.at> wrote:
> > 
> > > That makes avformat & avcodec thread safe without the need to explicitly
> > > register a lock manager.
> > > 
> > > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > > ---
> > >  libavcodec/utils.c |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 49 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > > index 3832b81..735c8e0 100644
> > > --- a/libavcodec/utils.c
> > > +++ b/libavcodec/utils.c
> > > @@ -56,9 +56,57 @@
> > >  # include <iconv.h>
> > >  #endif
> > >  
> > > +#if HAVE_PTHREADS
> > > +#include <pthread.h>
> > > +static int default_lockmgr_cb(void **arg, enum AVLockOp op)
> > > +{
> > > +    void * volatile * mutex = arg;
> > > +    int err;
> > > +
> > > +    switch (op) {
> > > +    case AV_LOCK_CREATE:
> > > +        return 0;
> > > +    case AV_LOCK_OBTAIN:
> > > +        if (!*mutex) {
> > > +            pthread_mutex_t *tmp = av_malloc(sizeof(pthread_mutex_t));
> > > +            if (!tmp)
> > > +                return AVERROR(ENOMEM);
> > > +            if ((err = pthread_mutex_init(tmp, NULL))) {
> > > +                av_free(tmp);
> > > +                return AVERROR(err);
> > > +            }
> > > +            if (avpriv_atomic_ptr_cas(mutex, NULL, tmp)) {
> > > +                pthread_mutex_destroy(tmp);
> > > +                av_free(tmp);
> > > +            }
> > > +        }
> > > +
> > > +        if ((err = pthread_mutex_lock(*mutex)))
> > > +            return AVERROR(err);
> > > +
> > > +        return 0;
> > > +    case AV_LOCK_RELEASE:
> > > +        if ((err = pthread_mutex_unlock(*mutex)))
> > > +            return AVERROR(err);
> > > +
> > > +        return 0;
> > > +    case AV_LOCK_DESTROY:
> > > +        if (*mutex)
> > > +            pthread_mutex_destroy(*mutex);
> > > +        av_free(*mutex);
> > > +        avpriv_atomic_ptr_cas(mutex, *mutex, NULL);
> > > +        return 0;
> > > +    }
> > > +    return 1;
> > > +}
> > > +static int (*lockmgr_cb)(void **mutex, enum AVLockOp op) = default_lockmgr_cb;
> > > +#else
> > > +static int (*lockmgr_cb)(void **mutex, enum AVLockOp op) = NULL;
> > > +#endif
> > > +
> > > +
> > >  volatile int ff_avcodec_locked;
> > >  static int volatile entangled_thread_counter = 0;
> > > -static int (*lockmgr_cb)(void **mutex, enum AVLockOp op);
> > >  static void *codec_mutex;
> > >  static void *avformat_mutex;
> > >  
> > 
> > Looks much better, but doesn't seem to use the internal pthread
> > emulation if required (on win32, os/2). So it would be "safe" on Unix
> > only.
> 
> Yes, these should be added by developers who use and can test these
> systems

for reference (untested)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 735c8e0..a8a5db8 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -58,6 +58,13 @@

 #if HAVE_PTHREADS
 #include <pthread.h>
+#elif HAVE_W32THREADS
+#include "compat/w32pthreads.h"
+#elif HAVE_OS2THREADS
+#include "compat/os2threads.h"
+#endif
+
+#if HAVE_PTHREADS || HAVE_W32THREADS || HAVE_OS2THREADS
 static int default_lockmgr_cb(void **arg, enum AVLockOp op)
 {
     void * volatile * mutex = arg;


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- 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/20131018/bbfa1efd/attachment.asc>