From 28bce48d479be87ae72295fba0670d10d7eab697 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Sun, 4 Jul 2021 12:30:49 +0200 Subject: [PATCH] Relax v4l2_sink lock constraints To fix a data race, commit 5caeab5f6d12fe0fc69e708a829cfdbec3a401b4 called video_buffer_push() and video_buffer_consume() under the v4l2_sink lock. Instead, use the previous_skipped indication (initialized with video buffer locked) to lock only for protecting the has_frame flag. This enables the possibility for the video_buffer to notify new frames via callbacks without lock inversion issues. --- app/src/v4l2_sink.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/app/src/v4l2_sink.c b/app/src/v4l2_sink.c index 5ee9c8eb..7904b450 100644 --- a/app/src/v4l2_sink.c +++ b/app/src/v4l2_sink.c @@ -121,11 +121,11 @@ run_v4l2_sink(void *data) { break; } - video_buffer_consume(&vs->vb, vs->frame); vs->has_frame = false; - sc_mutex_unlock(&vs->mutex); + video_buffer_consume(&vs->vb, vs->frame); + bool ok = encode_and_write_frame(vs, vs->frame); av_frame_unref(vs->frame); if (!ok) { @@ -303,17 +303,18 @@ sc_v4l2_sink_close(struct sc_v4l2_sink *vs) { static bool sc_v4l2_sink_push(struct sc_v4l2_sink *vs, const AVFrame *frame) { - sc_mutex_lock(&vs->mutex); - - bool ok = video_buffer_push(&vs->vb, frame, NULL); + bool previous_skipped; + bool ok = video_buffer_push(&vs->vb, frame, &previous_skipped); if (!ok) { return false; } - vs->has_frame = true; - sc_cond_signal(&vs->cond); - - sc_mutex_unlock(&vs->mutex); + if (!previous_skipped) { + sc_mutex_lock(&vs->mutex); + vs->has_frame = true; + sc_cond_signal(&vs->cond); + sc_mutex_unlock(&vs->mutex); + } return true; }