[FFmpeg-devel] [PATCH] Read |state| under the protection of |mutex| or |progress_mutex|.

Wan-Teh Chang wtc at google.com
Fri Feb 26 03:18:38 CET 2016


Although not documented, |state| is guarded by either |mutex| or
|progress_mutex|. In several places |state| is read without mutex
protection, often as a double-checked locking style performance
optimization. Fix the data races by reading |state| under mutex
protection.
---
 libavcodec/pthread_frame.c | 44 +++++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index b77dd1e..a768b1c 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -323,12 +323,10 @@ static int submit_packet(PerThreadContext *p, AVPacket *avpkt)
 
     if (prev_thread) {
         int err;
-        if (prev_thread->state == STATE_SETTING_UP) {
-            pthread_mutex_lock(&prev_thread->progress_mutex);
-            while (prev_thread->state == STATE_SETTING_UP)
-                pthread_cond_wait(&prev_thread->progress_cond, &prev_thread->progress_mutex);
-            pthread_mutex_unlock(&prev_thread->progress_mutex);
-        }
+        pthread_mutex_lock(&prev_thread->progress_mutex);
+        while (prev_thread->state == STATE_SETTING_UP)
+            pthread_cond_wait(&prev_thread->progress_cond, &prev_thread->progress_mutex);
+        pthread_mutex_unlock(&prev_thread->progress_mutex);
 
         err = update_context_from_thread(p->avctx, prev_thread->avctx, 0);
         if (err) {
@@ -353,9 +351,9 @@ static int submit_packet(PerThreadContext *p, AVPacket *avpkt)
     if (!p->avctx->thread_safe_callbacks && (
          p->avctx->get_format != avcodec_default_get_format ||
          p->avctx->get_buffer2 != avcodec_default_get_buffer2)) {
+        pthread_mutex_lock(&p->progress_mutex);
         while (p->state != STATE_SETUP_FINISHED && p->state != STATE_INPUT_READY) {
             int call_done = 1;
-            pthread_mutex_lock(&p->progress_mutex);
             while (p->state == STATE_SETTING_UP)
                 pthread_cond_wait(&p->progress_cond, &p->progress_mutex);
 
@@ -374,8 +372,8 @@ static int submit_packet(PerThreadContext *p, AVPacket *avpkt)
                 p->state  = STATE_SETTING_UP;
                 pthread_cond_signal(&p->progress_cond);
             }
-            pthread_mutex_unlock(&p->progress_mutex);
         }
+        pthread_mutex_unlock(&p->progress_mutex);
     }
 
     fctx->prev_thread = p;
@@ -426,12 +424,10 @@ int ff_thread_decode_frame(AVCodecContext *avctx,
     do {
         p = &fctx->threads[finished++];
 
-        if (p->state != STATE_INPUT_READY) {
-            pthread_mutex_lock(&p->progress_mutex);
-            while (p->state != STATE_INPUT_READY)
-                pthread_cond_wait(&p->output_cond, &p->progress_mutex);
-            pthread_mutex_unlock(&p->progress_mutex);
-        }
+        pthread_mutex_lock(&p->progress_mutex);
+        while (p->state != STATE_INPUT_READY)
+            pthread_cond_wait(&p->output_cond, &p->progress_mutex);
+        pthread_mutex_unlock(&p->progress_mutex);
 
         av_frame_move_ref(picture, p->frame);
         *got_picture_ptr = p->got_frame;
@@ -510,13 +506,13 @@ void ff_thread_finish_setup(AVCodecContext *avctx) {
 
     if (!(avctx->active_thread_type&FF_THREAD_FRAME)) return;
 
-    if(p->state == STATE_SETUP_FINISHED){
+    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");
+    } else {
+        p->state = STATE_SETUP_FINISHED;
+        pthread_cond_broadcast(&p->progress_cond);
     }
-
-    pthread_mutex_lock(&p->progress_mutex);
-    p->state = STATE_SETUP_FINISHED;
-    pthread_cond_broadcast(&p->progress_cond);
     pthread_mutex_unlock(&p->progress_mutex);
 }
 
@@ -528,12 +524,10 @@ static void park_frame_worker_threads(FrameThreadContext *fctx, int thread_count
     for (i = 0; i < thread_count; i++) {
         PerThreadContext *p = &fctx->threads[i];
 
-        if (p->state != STATE_INPUT_READY) {
-            pthread_mutex_lock(&p->progress_mutex);
-            while (p->state != STATE_INPUT_READY)
-                pthread_cond_wait(&p->output_cond, &p->progress_mutex);
-            pthread_mutex_unlock(&p->progress_mutex);
-        }
+        pthread_mutex_lock(&p->progress_mutex);
+        while (p->state != STATE_INPUT_READY)
+            pthread_cond_wait(&p->output_cond, &p->progress_mutex);
+        pthread_mutex_unlock(&p->progress_mutex);
         p->got_frame = 0;
     }
 }
-- 
2.7.0.rc3.207.g0ac5344



More information about the ffmpeg-devel mailing list