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

Michael Niedermayer michaelni at gmx.at
Mon Mar 26 00:10:58 CEST 2012


On Thu, Mar 22, 2012 at 09:17:37PM +0100, Reimar Döffinger wrote:
> On Thu, Mar 22, 2012 at 12:44:07PM -0700, Aaron Colwell wrote:
> > On Thu, Mar 22, 2012 at 11:33 AM, Reimar Döffinger <Reimar.Doeffinger at gmx.de
> > > wrote:
> > 
> > > On Thu, Mar 22, 2012 at 10:54:45AM -0700, Aaron Colwell wrote:
> > > > 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.
> > 
> > 
> > How about this updated patch for addressing this?
> 
> Looks fine to me but I'd favour a second opinion at least.

LGTM to me as well

patch applied


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

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- 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/20120326/88ea08b0/attachment.asc>


More information about the ffmpeg-devel mailing list