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

[wm4]

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.

> Can the av_log call be moved under a local variable for the condition to
> after the lock is released?