From b049f51f49e60ff81ef1d8054be31a3d2bdd5eff Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Tue, 7 Apr 2026 20:35:01 +0200 Subject: [PATCH] Remove frame buffer internal mutex Let the caller handle locking as needed, allowing them to perform other actions under the same lock. --- app/src/frame_buffer.c | 27 ++++++--------------------- app/src/frame_buffer.h | 8 ++++---- app/src/screen.c | 20 +++++++++++++++++--- app/src/screen.h | 4 +++- app/src/v4l2_sink.c | 12 ++++++------ 5 files changed, 36 insertions(+), 35 deletions(-) diff --git a/app/src/frame_buffer.c b/app/src/frame_buffer.c index 9fd4cf6f..699a3f37 100644 --- a/app/src/frame_buffer.c +++ b/app/src/frame_buffer.c @@ -19,13 +19,6 @@ sc_frame_buffer_init(struct sc_frame_buffer *fb) { return false; } - bool ok = sc_mutex_init(&fb->mutex); - if (!ok) { - av_frame_free(&fb->pending_frame); - av_frame_free(&fb->tmp_frame); - return false; - } - // there is initially no frame, so consider it has already been consumed fb->pending_frame_consumed = true; @@ -34,7 +27,6 @@ sc_frame_buffer_init(struct sc_frame_buffer *fb) { void sc_frame_buffer_destroy(struct sc_frame_buffer *fb) { - sc_mutex_destroy(&fb->mutex); av_frame_free(&fb->pending_frame); av_frame_free(&fb->tmp_frame); } @@ -47,8 +39,12 @@ swap_frames(AVFrame **lhs, AVFrame **rhs) { } bool -sc_frame_buffer_push(struct sc_frame_buffer *fb, const AVFrame *frame, - bool *previous_frame_skipped) { +sc_frame_buffer_has_frame(struct sc_frame_buffer *fb) { + return !fb->pending_frame_consumed; +} + +bool +sc_frame_buffer_push(struct sc_frame_buffer *fb, const AVFrame *frame) { // Use a temporary frame to preserve pending_frame in case of error. // tmp_frame is an empty frame, no need to call av_frame_unref() beforehand. int r = av_frame_ref(fb->tmp_frame, frame); @@ -57,32 +53,21 @@ sc_frame_buffer_push(struct sc_frame_buffer *fb, const AVFrame *frame, return false; } - sc_mutex_lock(&fb->mutex); - // Now that av_frame_ref() succeeded, we can replace the previous // pending_frame swap_frames(&fb->pending_frame, &fb->tmp_frame); av_frame_unref(fb->tmp_frame); - if (previous_frame_skipped) { - *previous_frame_skipped = !fb->pending_frame_consumed; - } fb->pending_frame_consumed = false; - - sc_mutex_unlock(&fb->mutex); - return true; } void sc_frame_buffer_consume(struct sc_frame_buffer *fb, AVFrame *dst) { - sc_mutex_lock(&fb->mutex); assert(!fb->pending_frame_consumed); fb->pending_frame_consumed = true; av_frame_move_ref(dst, fb->pending_frame); // av_frame_move_ref() resets its source frame, so no need to call // av_frame_unref() - - sc_mutex_unlock(&fb->mutex); } diff --git a/app/src/frame_buffer.h b/app/src/frame_buffer.h index e748adfb..f5bd1a24 100644 --- a/app/src/frame_buffer.h +++ b/app/src/frame_buffer.h @@ -24,8 +24,6 @@ struct sc_frame_buffer { AVFrame *pending_frame; AVFrame *tmp_frame; // To preserve the pending frame on error - sc_mutex mutex; - bool pending_frame_consumed; }; @@ -36,8 +34,10 @@ void sc_frame_buffer_destroy(struct sc_frame_buffer *fb); bool -sc_frame_buffer_push(struct sc_frame_buffer *fb, const AVFrame *frame, - bool *skipped); +sc_frame_buffer_has_frame(struct sc_frame_buffer *fb); + +bool +sc_frame_buffer_push(struct sc_frame_buffer *fb, const AVFrame *frame); void sc_frame_buffer_consume(struct sc_frame_buffer *fb, AVFrame *dst); diff --git a/app/src/screen.c b/app/src/screen.c index b1bcdf3a..1a878c9b 100644 --- a/app/src/screen.c +++ b/app/src/screen.c @@ -360,8 +360,10 @@ sc_screen_frame_sink_push(struct sc_frame_sink *sink, const AVFrame *frame) { struct sc_screen *screen = DOWNCAST(sink); assert(screen->video); - bool previous_skipped; - bool ok = sc_frame_buffer_push(&screen->fb, frame, &previous_skipped); + sc_mutex_lock(&screen->mutex); + bool previous_skipped = sc_frame_buffer_has_frame(&screen->fb); + bool ok = sc_frame_buffer_push(&screen->fb, frame); + sc_mutex_unlock(&screen->mutex); if (!ok) { return false; } @@ -403,11 +405,16 @@ sc_screen_init(struct sc_screen *screen, screen->req.fullscreen = params->fullscreen; screen->req.start_fps_counter = params->start_fps_counter; - bool ok = sc_frame_buffer_init(&screen->fb); + bool ok = sc_mutex_init(&screen->mutex); if (!ok) { return false; } + ok = sc_frame_buffer_init(&screen->fb); + if (!ok) { + goto error_destroy_mutex; + } + if (!sc_fps_counter_init(&screen->fps_counter)) { goto error_destroy_frame_buffer; } @@ -605,6 +612,8 @@ error_destroy_fps_counter: sc_fps_counter_destroy(&screen->fps_counter); error_destroy_frame_buffer: sc_frame_buffer_destroy(&screen->fb); +error_destroy_mutex: + sc_mutex_destroy(&screen->mutex); return false; } @@ -684,6 +693,7 @@ sc_screen_destroy(struct sc_screen *screen) { SDL_DestroyWindow(screen->window); sc_fps_counter_destroy(&screen->fps_counter); sc_frame_buffer_destroy(&screen->fb); + sc_mutex_destroy(&screen->mutex); SDL_Event event; int nevents = SDL_PeepEvents(&event, 1, SDL_GETEVENT, @@ -807,12 +817,16 @@ sc_screen_update_frame(struct sc_screen *screen) { } else { av_frame_unref(screen->resume_frame); } + sc_mutex_lock(&screen->mutex); sc_frame_buffer_consume(&screen->fb, screen->resume_frame); + sc_mutex_unlock(&screen->mutex); return true; } av_frame_unref(screen->frame); + sc_mutex_lock(&screen->mutex); sc_frame_buffer_consume(&screen->fb, screen->frame); + sc_mutex_unlock(&screen->mutex); return sc_screen_apply_frame(screen); } diff --git a/app/src/screen.h b/app/src/screen.h index e04abefe..19d3d2ca 100644 --- a/app/src/screen.h +++ b/app/src/screen.h @@ -41,9 +41,11 @@ struct sc_screen { struct sc_texture tex; struct sc_input_manager im; struct sc_mouse_capture mc; // only used in mouse relative mode - struct sc_frame_buffer fb; struct sc_fps_counter fps_counter; + struct sc_mutex mutex; + struct sc_frame_buffer fb; // protected by mutex + // The initial requested window properties struct { int16_t x; diff --git a/app/src/v4l2_sink.c b/app/src/v4l2_sink.c index 92551149..c4f3b783 100644 --- a/app/src/v4l2_sink.c +++ b/app/src/v4l2_sink.c @@ -128,9 +128,8 @@ run_v4l2_sink(void *data) { } vs->has_frame = false; - sc_mutex_unlock(&vs->mutex); - sc_frame_buffer_consume(&vs->fb, vs->frame); + sc_mutex_unlock(&vs->mutex); bool ok = encode_and_write_frame(vs, vs->frame); av_frame_unref(vs->frame); @@ -311,19 +310,20 @@ sc_v4l2_sink_close(struct sc_v4l2_sink *vs) { static bool sc_v4l2_sink_push(struct sc_v4l2_sink *vs, const AVFrame *frame) { - bool previous_skipped; - bool ok = sc_frame_buffer_push(&vs->fb, frame, &previous_skipped); + sc_mutex_lock(&vs->mutex); + bool previous_skipped = sc_frame_buffer_has_frame(&vs->fb); + bool ok = sc_frame_buffer_push(&vs->fb, frame); if (!ok) { + sc_mutex_unlock(&vs->mutex); return false; } if (!previous_skipped) { - sc_mutex_lock(&vs->mutex); vs->has_frame = true; sc_cond_signal(&vs->cond); - sc_mutex_unlock(&vs->mutex); } + sc_mutex_unlock(&vs->mutex); return true; }