[FFmpeg-devel] [PATCH] avcodec thread safety fix

[Michael Niedermayer]

On Sat, May 30, 2009 at 09:49:02AM +0200, Andreas ?man wrote:
> Art Clarke wrote:
> >> Well, my path does
> >>
> >> if(cb(&codec_mutex, AV_LOCK_CREATE))
> >>     return -1;
> >>
> >> In the callback register function. I believe that should be enough?
> > 
> > Sorry, I missed that.  You were already doing what I suggested.  You
> > should create the mutex AFTER you set ff_lockmgr_cb to avoid a race.
> > 
> > e.g. imagine the callback created a mutex but also started a new
> > thread that immediately tried to open a codec before the first
> > callback returned.
> > 
> >> Perhaps it could AV_LOCK_DESTROY all global mutexes if you invoke
> >> ff_lockmgr_register with NULL. Or have av_lockmgr_unregister()?
> > 
> > That would be useful.  I prefer av_lockmgr_register(null) since it
> > operates more like the other callbacks then.  Please call unlock
> > BEFORE you unregister the callback and set the new callback (i.e.
> > always make sure when you're setting or destroying a lock with a lock
> > manager, that it's the manager actually set on ff_lockmgr_cb when
> > running).
> 
> Yep, this makes sense, hope I got it right. Another patch attached.
> 
> >> This sounds reasonable to me. It would also make it possible to avoid
> >> locking for codecs that don't need it at all.
> > 
> > As Michael pointed out, this is, um, complicated.  I'd only suggest
> > going down this path if this was a real scaling problem for a real
> > world use case of libav (an aside, I'd be very surprised if contention
> > for the global codec open  lock was the biggest scaling problem in
> > someone running a multi-threaded decoder/encoder -- and I'm talking
> > from the perspective of someone who already does exactly that).   I
> > mentioned it as an idea, but I'll supplement by saying "it's an idea
> > awaiting the problem" :)
> 
> Yeah, i'm using FFmpeg from lots of threads myself, and frankly. I've
> never bumped into any problems which my external locks around
> avcodec_open and close.
> 
[...}
> @@ -483,6 +491,11 @@
>      ret=0;
>  end:
>      entangled_thread_counter--;
> +
> +    /* Release any user-supplied mutex. */
> +    if (ff_lockmgr_cb) {
> +      (*ff_lockmgr_cb)(&codec_mutex, AV_LOCK_RELEASE);
> +    }
>      return ret;
>  }
>  

indent ...

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

Thouse who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090530/813f5fd7/attachment.pgp>