[FFmpeg-devel] [PATCH] frame_thread_encoder: extend critical code covered by finished_task_mutex.

[Michael Niedermayer]

On Wed, May 24, 2017 at 12:59:58PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On May 24, 2017 12:51 PM, "Clément Bœsch" <u at pkh.me> wrote:
> 
> On Wed, May 24, 2017 at 12:15:05PM -0400, Ronald S. Bultje wrote:
> > Should fix tsan errors in utvideoenc_rgb_left and related tests.
> > ---
> >  libavcodec/frame_thread_encoder.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavcodec/frame_thread_encoder.c b/libavcodec/frame_thread_
> encoder.c
> > index 27ae356..2746964 100644
> > --- a/libavcodec/frame_thread_encoder.c
> > +++ b/libavcodec/frame_thread_encoder.c
> > @@ -273,14 +273,21 @@ int ff_thread_video_encode_frame(AVCodecContext
> *avctx, AVPacket *pkt, const AVF
> >
> >          c->task_index = (c->task_index+1) % BUFFER_SIZE;
> >
> > -        if(!c->finished_tasks[c->finished_task_index].outdata &&
> (c->task_index - c->finished_task_index) % BUFFER_SIZE <=
> avctx->thread_count)
> > +        pthread_mutex_lock(&c->finished_task_mutex);
> > +        if(!c->finished_tasks[c->finished_task_index].outdata &&
> > +           (c->task_index - c->finished_task_index) % BUFFER_SIZE <=
> avctx->thread_count) {
> > +            pthread_mutex_unlock(&c->finished_task_mutex);
> >              return 0;
> > +        }
> > +    } else {
> > +        pthread_mutex_lock(&c->finished_task_mutex);
> >      }
> >
> > -    if(c->task_index == c->finished_task_index)
> > +    if(c->task_index == c->finished_task_index) {
> > +        pthread_mutex_unlock(&c->finished_task_mutex);
> >          return 0;
> > +    }
> >
> > -    pthread_mutex_lock(&c->finished_task_mutex);
> >      while (!c->finished_tasks[c->finished_task_index].outdata) {
> >          pthread_cond_wait(&c->finished_task_cond,
> &c->finished_task_mutex);
> >      }
> 
> 
> LGTM
> 
> alternatively you could do sth like the following (totally untested) to
> reduce the pthread spaghettis:
> 
> 
> Yes, I initially wrote it like that but considered the spaghetti too much
> for the edge case benefit it brings to flushing only.
> 
> But if people prefer that, I can do that too. What do others think?

I like ubitux version

thanks

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170525/2931a2e7/attachment.sig>