[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