[FFmpeg-devel] [PATCH 1/3] pthread : Remove one unnecessary lock/unlock pair in worker loop.

[Reimar Döffinger]

On Thu, Mar 22, 2012 at 10:54:45AM -0700, Aaron Colwell wrote:
> On Thu, Mar 22, 2012 at 10:34 AM, Reimar Döffinger <Reimar.Doeffinger at gmx.de
> > wrote:
> 
> > On Thu, Mar 22, 2012 at 09:51:49AM -0700, Aaron Colwell wrote:
> > > Why does ff_thread_finish_setup(avctx) really need to be called outside
> > of
> > > the p->mutex?
> >
> > I doubt it needs to, it just generally is safer to not keep a mutex
> > without a good reason to do so, not the other way round of only
> > releasing a mutex when you absolutely have to.
> >
> 
> In general I would agree, but in this case it isn't particularly clear why
> p->mutex isn't needed only at this specific point. A few lines down the
> same ff_thread_finish_setup() call is made but with p->mutex locked. Any
> idea why these calls are made in different locking contexts?

I would say because for the first avoiding the lock is just a matter of
moving the lock call.
For the second case, it would have required a full unlock+lock cycle.

> From what I can tell p->mutex appears to only be used for allowing packets
> to be submitted when the worker thread is ready for them. It seems like
> this extra unlocked region opens up the possiblity for submit_packet() to
> sneak in state modifications when the worker thread isn't really ready. Am
> I misunderstanding the role of p->mutex?

I see what you mean.
Yes, submit_packet is blocking itself by setting state to SETTING_UP.
However resetting it outside the mutex as done here might indeed allow
it to incorrectly submit one additional packet to the same decode call,
i.e. we would silently lose one input packet.