diff mbox

[FFmpeg-devel,3/6] pthread_frame: call update_context_from_user() after acquiring lock.

Message ID 1490796744-76454-3-git-send-email-rsbultje@gmail.com
State Accepted
Commit 1269cd5b6f540bef5913bf134d2f461aac50d70b
Headers show

Commit Message

Ronald S. Bultje March 29, 2017, 2:12 p.m. UTC
Otherwise the thread may still be in the middle of decoding a previous
frame, which would effectively trigger a race condition on any field
concurrently read and written.
---
 libavcodec/pthread_frame.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

Comments

wm4 March 29, 2017, 2:38 p.m. UTC | #1
On Wed, 29 Mar 2017 10:12:21 -0400
"Ronald S. Bultje" <rsbultje@gmail.com> wrote:

> Otherwise the thread may still be in the middle of decoding a previous
> frame, which would effectively trigger a race condition on any field
> concurrently read and written.
> ---
>  libavcodec/pthread_frame.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> index 34c36d8..6cc43f6 100644
> --- a/libavcodec/pthread_frame.c
> +++ b/libavcodec/pthread_frame.c
> @@ -379,7 +379,8 @@ static void release_delayed_buffers(PerThreadContext *p)
>      }
>  }
>  
> -static int submit_packet(PerThreadContext *p, AVPacket *avpkt)
> +static int submit_packet(PerThreadContext *p, AVCodecContext *user_avctx,
> +                         AVPacket *avpkt)
>  {
>      FrameThreadContext *fctx = p->parent;
>      PerThreadContext *prev_thread = fctx->prev_thread;
> @@ -391,6 +392,12 @@ static int submit_packet(PerThreadContext *p, AVPacket *avpkt)
>  
>      pthread_mutex_lock(&p->mutex);
>  
> +    ret = update_context_from_user(p->avctx, user_avctx);
> +    if (ret) {
> +        pthread_mutex_unlock(&p->mutex);
> +        return ret;
> +    }
> +
>      release_delayed_buffers(p);
>  
>      if (prev_thread) {
> @@ -479,10 +486,7 @@ int ff_thread_decode_frame(AVCodecContext *avctx,
>       */
>  
>      p = &fctx->threads[fctx->next_decoding];
> -    err = update_context_from_user(p->avctx, avctx);
> -    if (err)
> -        goto finish;
> -    err = submit_packet(p, avpkt);
> +    err = submit_packet(p, avctx, avpkt);
>      if (err)
>          goto finish;
>  

So PerThreadContext.mutex protects the thread's avctx as well? Makes
sense then, I guess.
diff mbox

Patch

diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 34c36d8..6cc43f6 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -379,7 +379,8 @@  static void release_delayed_buffers(PerThreadContext *p)
     }
 }
 
-static int submit_packet(PerThreadContext *p, AVPacket *avpkt)
+static int submit_packet(PerThreadContext *p, AVCodecContext *user_avctx,
+                         AVPacket *avpkt)
 {
     FrameThreadContext *fctx = p->parent;
     PerThreadContext *prev_thread = fctx->prev_thread;
@@ -391,6 +392,12 @@  static int submit_packet(PerThreadContext *p, AVPacket *avpkt)
 
     pthread_mutex_lock(&p->mutex);
 
+    ret = update_context_from_user(p->avctx, user_avctx);
+    if (ret) {
+        pthread_mutex_unlock(&p->mutex);
+        return ret;
+    }
+
     release_delayed_buffers(p);
 
     if (prev_thread) {
@@ -479,10 +486,7 @@  int ff_thread_decode_frame(AVCodecContext *avctx,
      */
 
     p = &fctx->threads[fctx->next_decoding];
-    err = update_context_from_user(p->avctx, avctx);
-    if (err)
-        goto finish;
-    err = submit_packet(p, avpkt);
+    err = submit_packet(p, avctx, avpkt);
     if (err)
         goto finish;