[FFmpeg-devel] [PATCH] pthread: Avoid crashes/odd behavior caused by spurious wakeups

Ben Jackson ben at ben.com
Fri Sep 14 01:34:25 CEST 2012


On Fri, Sep 14, 2012 at 12:55:53AM +0200, Michael Niedermayer wrote:
> 
> well, the job count can change for each avcodec_thread_execute()
> i see now its not really helping in simplifying things but it feels
> more correct to set it to 0 when there are no jobs to be done

I considered a patch to clean up ThreadContext when it's idle.  Part of
the reason that spurious wakeups in worker() were hard to figure out is
that it had all the stale data from the last job and crashed inside the
decoder.  Cleaning up between executes would move the crash to worker()...

By that point I knew how to fix the problem so I just fixed it so that
(hopefully) the stale info in ThreadContext is irrelevant.  But I'll
submit a patch to clean out ThreadContext in avcodec_thread_park_workers
if you think it's worthwhile.

> > > > +    c->current_execute++;
> > > 
> > > signed integer overflow and undefined behavior
> 
> ^=1 or unsigned or anything equivalent is fine

I'll resubmit with that change later tonight.

> no, the original code is broken and the patch fixes it. I just hoped
> that a simpler condition would work but it seems thats not the case

I think something more state-machine like (similar to the frame threading)
would be easier to understand but it would be a rewrite.

-- 
Ben Jackson AD7GD
<ben at ben.com>
http://www.ben.com/


More information about the ffmpeg-devel mailing list