[FFmpeg-devel] [PATCH 1/3] lavc/pthread_frame: protect read state access in setup finish function

[Ronald S. Bultje]

Hi,

On Fri, Jan 13, 2017 at 9:39 AM, wm4 <nfxjfg at googlemail.com> wrote:

> On Fri, 13 Jan 2017 09:07:48 -0500
> "Ronald S. Bultje" <rsbultje at gmail.com> wrote:
>
> > Hi,
> >
> > On Jan 13, 2017 6:40 AM, "Clément Bœsch" <u at pkh.me> wrote:
> >
> > From: Clément Bœsch <cboesch at gopro.com>
> >
> > ---
> >  libavcodec/pthread_frame.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> > index 7ef5e9f6be..cb6d76284e 100644
> > --- a/libavcodec/pthread_frame.c
> > +++ b/libavcodec/pthread_frame.c
> > @@ -509,11 +509,11 @@ void ff_thread_finish_setup(AVCodecContext
> *avctx) {
> >
> >      if (!(avctx->active_thread_type&FF_THREAD_FRAME)) return;
> >
> > +    pthread_mutex_lock(&p->progress_mutex);
> >      if(p->state == STATE_SETUP_FINISHED){
> >          av_log(avctx, AV_LOG_WARNING, "Multiple ff_thread_finish_setup()
> > calls\n");
> >      }
> >
> > -    pthread_mutex_lock(&p->progress_mutex);
> >
> >
> > This has a problem in that we're potentially calling an I/O function that
> > potentially directly enters application space with a lock held.
>
> I think the user should be taking care of this (or just don't spam
> av_log in a performance critical function). The log call here seems
> more like an internal warning though, and I've never seen it happening.
>

But it's easy to resolve right?

lock();
int state = p->state;
..
unlock();
if (state == ..)
  av_log(..);

Ronald