Extract current video_buffer to frame_buffer

The current video buffer only stores one pending frame.

In order to add a new buffering feature, move this part to a separate
"frame buffer". Keep the video_buffer, which currently delegates all its
calls to the frame_buffer.

app/meson.build

@@ -10,6 +10,7 @@ src = [
     'src/event_converter.c',
     'src/file_handler.c',
     'src/fps_counter.c',
+    'src/frame_buffer.c',
     'src/input_manager.c',
     'src/opengl.c',
     'src/receiver.c',

app/src/frame_buffer.c

@@ -0,0 +1,88 @@
+#include "frame_buffer.h"
+
+#include <assert.h>
+#include <libavutil/avutil.h>
+#include <libavformat/avformat.h>
+
+#include "util/log.h"
+
+bool
+sc_frame_buffer_init(struct sc_frame_buffer *fb) {
+    fb->pending_frame = av_frame_alloc();
+    if (!fb->pending_frame) {
+        return false;
+    }
+
+    fb->tmp_frame = av_frame_alloc();
+    if (!fb->tmp_frame) {
+        av_frame_free(&fb->pending_frame);
+        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;
+
+    return true;
+}
+
+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);
+}
+
+static inline void
+swap_frames(AVFrame **lhs, AVFrame **rhs) {
+    AVFrame *tmp = *lhs;
+    *lhs = *rhs;
+    *rhs = tmp;
+}
+
+bool
+sc_frame_buffer_push(struct sc_frame_buffer *fb, const AVFrame *frame,
+                  bool *previous_frame_skipped) {
+    sc_mutex_lock(&fb->mutex);
+
+    // 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);
+    if (r) {
+        LOGE("Could not ref frame: %d", r);
+        return false;
+    }
+
+    // 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);
+}

app/src/frame_buffer.h

@@ -0,0 +1,44 @@
+#ifndef SC_FRAME_BUFFER_H
+#define SC_FRAME_BUFFER_H
+
+#include "common.h"
+
+#include <stdbool.h>
+
+#include "util/thread.h"
+
+// forward declarations
+typedef struct AVFrame AVFrame;
+
+/**
+ * A frame buffer holds 1 pending frame, which is the last frame received from
+ * the producer (typically, the decoder).
+ *
+ * If a pending frame has not been consumed when the producer pushes a new
+ * frame, then it is lost. The intent is to always provide access to the very
+ * last frame to minimize latency.
+ */
+
+struct sc_frame_buffer {
+    AVFrame *pending_frame;
+    AVFrame *tmp_frame; // To preserve the pending frame on error
+
+    sc_mutex mutex;
+
+    bool pending_frame_consumed;
+};
+
+bool
+sc_frame_buffer_init(struct sc_frame_buffer *fb);
+
+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);
+
+void
+sc_frame_buffer_consume(struct sc_frame_buffer *fb, AVFrame *dst);
+
+#endif

app/src/scrcpy.c

@@ -343,19 +343,29 @@ scrcpy(const struct scrcpy_options *options) {
         stream_add_sink(&s->stream, &rec->packet_sink);
     }
 
-    if (options->display) {
-        if (options->control) {
-            if (!controller_init(&s->controller, s->server.control_socket)) {
-                goto end;
-            }
-            controller_initialized = true;
-
-            if (!controller_start(&s->controller)) {
-                goto end;
-            }
-            controller_started = true;
+    if (options->control) {
+        if (!controller_init(&s->controller, s->server.control_socket)) {
+            goto end;
         }
+        controller_initialized = true;
 
+        if (!controller_start(&s->controller)) {
+            goto end;
+        }
+        controller_started = true;
+
+        if (options->turn_screen_off) {
+            struct control_msg msg;
+            msg.type = CONTROL_MSG_TYPE_SET_SCREEN_POWER_MODE;
+            msg.set_screen_power_mode.mode = SCREEN_POWER_MODE_OFF;
+
+            if (!controller_push_msg(&s->controller, &msg)) {
+                LOGW("Could not request 'set screen power mode'");
+            }
+        }
+    }
+
+    if (options->display) {
         const char *window_title =
             options->window_title ? options->window_title : device_name;
 
@@ -379,16 +389,6 @@ scrcpy(const struct scrcpy_options *options) {
         screen_initialized = true;
 
         decoder_add_sink(&s->decoder, &s->screen.frame_sink);
-
-        if (options->turn_screen_off) {
-            struct control_msg msg;
-            msg.type = CONTROL_MSG_TYPE_SET_SCREEN_POWER_MODE;
-            msg.set_screen_power_mode.mode = SCREEN_POWER_MODE_OFF;
-
-            if (!controller_push_msg(&s->controller, &msg)) {
-                LOGW("Could not request 'set screen power mode'");
-            }
-        }
     }
 
 #ifdef HAVE_V4L2

app/src/screen.c

@@ -276,7 +276,7 @@ screen_frame_sink_push(struct sc_frame_sink *sink, const AVFrame *frame) {
     struct screen *screen = DOWNCAST(sink);
 
     bool previous_frame_skipped;
-    bool ok = video_buffer_push(&screen->vb, frame, &previous_frame_skipped);
+    bool ok = sc_video_buffer_push(&screen->vb, frame, &previous_frame_skipped);
     if (!ok) {
         return false;
     }
@@ -304,7 +304,7 @@ screen_init(struct screen *screen, const struct screen_params *params) {
     screen->fullscreen = false;
     screen->maximized = false;
 
-    bool ok = video_buffer_init(&screen->vb);
+    bool ok = sc_video_buffer_init(&screen->vb);
     if (!ok) {
         LOGE("Could not initialize video buffer");
         return false;
@@ -454,7 +454,7 @@ error_destroy_window:
 error_destroy_fps_counter:
     fps_counter_destroy(&screen->fps_counter);
 error_destroy_video_buffer:
-    video_buffer_destroy(&screen->vb);
+    sc_video_buffer_destroy(&screen->vb);
 
     return false;
 }
@@ -489,7 +489,7 @@ screen_destroy(struct screen *screen) {
     SDL_DestroyRenderer(screen->renderer);
     SDL_DestroyWindow(screen->window);
     fps_counter_destroy(&screen->fps_counter);
-    video_buffer_destroy(&screen->vb);
+    sc_video_buffer_destroy(&screen->vb);
 }
 
 static void
@@ -595,7 +595,7 @@ update_texture(struct screen *screen, const AVFrame *frame) {
 static bool
 screen_update_frame(struct screen *screen) {
     av_frame_unref(screen->frame);
-    video_buffer_consume(&screen->vb, screen->frame);
+    sc_video_buffer_consume(&screen->vb, screen->frame);
     AVFrame *frame = screen->frame;
 
     fps_counter_add_rendered_frame(&screen->fps_counter);

app/src/screen.h

@@ -8,6 +8,7 @@
 #include <libavformat/avformat.h>
 
 #include "coords.h"
+#include "fps_counter.h"
 #include "opengl.h"
 #include "trait/frame_sink.h"
 #include "video_buffer.h"
@@ -19,7 +20,7 @@ struct screen {
     bool open; // track the open/close state to assert correct behavior
 #endif
 
-    struct video_buffer vb;
+    struct sc_video_buffer vb;
     struct fps_counter fps_counter;
 
     SDL_Window *window;

app/src/util/thread.c

@@ -31,7 +31,7 @@ sc_mutex_init(sc_mutex *mutex) {
 
     mutex->mutex = sdl_mutex;
 #ifndef NDEBUG
-    mutex->locker = 0;
+    atomic_init(&mutex->locker, 0);
 #endif
     return true;
 }
@@ -52,7 +52,8 @@ sc_mutex_lock(sc_mutex *mutex) {
         abort();
     }
 
-    mutex->locker = sc_thread_get_id();
+    atomic_store_explicit(&mutex->locker, sc_thread_get_id(),
+                          memory_order_relaxed);
 #else
     (void) r;
 #endif
@@ -62,7 +63,7 @@ void
 sc_mutex_unlock(sc_mutex *mutex) {
 #ifndef NDEBUG
     assert(sc_mutex_held(mutex));
-    mutex->locker = 0;
+    atomic_store_explicit(&mutex->locker, 0, memory_order_relaxed);
 #endif
     int r = SDL_UnlockMutex(mutex->mutex);
 #ifndef NDEBUG
@@ -83,7 +84,9 @@ sc_thread_get_id(void) {
 #ifndef NDEBUG
 bool
 sc_mutex_held(struct sc_mutex *mutex) {
-    return mutex->locker == sc_thread_get_id();
+    sc_thread_id locker_id =
+        atomic_load_explicit(&mutex->locker, memory_order_relaxed);
+    return locker_id == sc_thread_get_id();
 }
 #endif
 
@@ -112,7 +115,8 @@ sc_cond_wait(sc_cond *cond, sc_mutex *mutex) {
         abort();
     }
 
-    mutex->locker = sc_thread_get_id();
+    atomic_store_explicit(&mutex->locker, sc_thread_get_id(),
+                          memory_order_relaxed);
 #else
     (void) r;
 #endif
@@ -127,7 +131,8 @@ sc_cond_timedwait(sc_cond *cond, sc_mutex *mutex, uint32_t ms) {
         abort();
     }
 
-    mutex->locker = sc_thread_get_id();
+    atomic_store_explicit(&mutex->locker, sc_thread_get_id(),
+                          memory_order_relaxed);
 #endif
     assert(r == 0 || r == SDL_MUTEX_TIMEDOUT);
     return r == 0;

app/src/util/thread.h

@@ -3,6 +3,7 @@
 
 #include "common.h"
 
+#include <stdatomic.h>
 #include <stdbool.h>
 #include <stdint.h>
 
@@ -12,7 +13,8 @@ typedef struct SDL_mutex SDL_mutex;
 typedef struct SDL_cond SDL_cond;
 
 typedef int sc_thread_fn(void *);
-typedef unsigned int sc_thread_id;
+typedef unsigned sc_thread_id;
+typedef atomic_uint sc_atomic_thread_id;
 
 typedef struct sc_thread {
     SDL_Thread *thread;
@@ -21,7 +23,7 @@ typedef struct sc_thread {
 typedef struct sc_mutex {
     SDL_mutex *mutex;
 #ifndef NDEBUG
-    sc_thread_id locker;
+    sc_atomic_thread_id locker;
 #endif
 } sc_mutex;
 

app/src/v4l2_sink.c

@@ -112,7 +112,7 @@ run_v4l2_sink(void *data) {
     for (;;) {
         sc_mutex_lock(&vs->mutex);
 
-        while (!vs->stopped && vs->vb.pending_frame_consumed) {
+        while (!vs->stopped && !vs->has_frame) {
             sc_cond_wait(&vs->cond, &vs->mutex);
         }
 
@@ -121,9 +121,11 @@ run_v4l2_sink(void *data) {
             break;
         }
 
+        sc_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) {
@@ -139,7 +141,7 @@ run_v4l2_sink(void *data) {
 
 static bool
 sc_v4l2_sink_open(struct sc_v4l2_sink *vs) {
-    bool ok = video_buffer_init(&vs->vb);
+    bool ok = sc_video_buffer_init(&vs->vb);
     if (!ok) {
         return false;
     }
@@ -241,6 +243,10 @@ sc_v4l2_sink_open(struct sc_v4l2_sink *vs) {
         goto error_av_frame_free;
     }
 
+    vs->has_frame = false;
+    vs->header_written = false;
+    vs->stopped = false;
+
     LOGD("Starting v4l2 thread");
     ok = sc_thread_create(&vs->thread, run_v4l2_sink, "v4l2", vs);
     if (!ok) {
@@ -248,9 +254,6 @@ sc_v4l2_sink_open(struct sc_v4l2_sink *vs) {
         goto error_av_packet_free;
     }
 
-    vs->header_written = false;
-    vs->stopped = false;
-
     LOGI("v4l2 sink started to device: %s", vs->device_name);
 
     return true;
@@ -272,7 +275,7 @@ error_cond_destroy:
 error_mutex_destroy:
     sc_mutex_destroy(&vs->mutex);
 error_video_buffer_destroy:
-    video_buffer_destroy(&vs->vb);
+    sc_video_buffer_destroy(&vs->vb);
 
     return false;
 }
@@ -294,19 +297,23 @@ sc_v4l2_sink_close(struct sc_v4l2_sink *vs) {
     avformat_free_context(vs->format_ctx);
     sc_cond_destroy(&vs->cond);
     sc_mutex_destroy(&vs->mutex);
-    video_buffer_destroy(&vs->vb);
+    sc_video_buffer_destroy(&vs->vb);
 }
 
 static bool
 sc_v4l2_sink_push(struct sc_v4l2_sink *vs, const AVFrame *frame) {
-    bool ok = video_buffer_push(&vs->vb, frame, NULL);
+    sc_mutex_lock(&vs->mutex);
+
+    bool ok = sc_video_buffer_push(&vs->vb, frame, NULL);
     if (!ok) {
         return false;
     }
 
-    // signal possible change of vs->vb.pending_frame_consumed
+    vs->has_frame = true;
     sc_cond_signal(&vs->cond);
 
+    sc_mutex_unlock(&vs->mutex);
+
     return true;
 }
 

app/src/v4l2_sink.h

@@ -12,7 +12,7 @@
 struct sc_v4l2_sink {
     struct sc_frame_sink frame_sink; // frame sink trait
 
-    struct video_buffer vb;
+    struct sc_video_buffer vb;
     AVFormatContext *format_ctx;
     AVCodecContext *encoder_ctx;
 
@@ -22,6 +22,7 @@ struct sc_v4l2_sink {
     sc_thread thread;
     sc_mutex mutex;
     sc_cond cond;
+    bool has_frame;
     bool stopped;
     bool header_written;
 

app/src/video_buffer.c

@@ -7,82 +7,22 @@
 #include "util/log.h"
 
 bool
-video_buffer_init(struct video_buffer *vb) {
-    vb->pending_frame = av_frame_alloc();
-    if (!vb->pending_frame) {
-        return false;
-    }
-
-    vb->tmp_frame = av_frame_alloc();
-    if (!vb->tmp_frame) {
-        av_frame_free(&vb->pending_frame);
-        return false;
-    }
-
-    bool ok = sc_mutex_init(&vb->mutex);
-    if (!ok) {
-        av_frame_free(&vb->pending_frame);
-        av_frame_free(&vb->tmp_frame);
-        return false;
-    }
-
-    // there is initially no frame, so consider it has already been consumed
-    vb->pending_frame_consumed = true;
-
-    return true;
+sc_video_buffer_init(struct sc_video_buffer *vb) {
+    return sc_frame_buffer_init(&vb->fb);
 }
 
 void
-video_buffer_destroy(struct video_buffer *vb) {
-    sc_mutex_destroy(&vb->mutex);
-    av_frame_free(&vb->pending_frame);
-    av_frame_free(&vb->tmp_frame);
-}
-
-static inline void
-swap_frames(AVFrame **lhs, AVFrame **rhs) {
-    AVFrame *tmp = *lhs;
-    *lhs = *rhs;
-    *rhs = tmp;
+sc_video_buffer_destroy(struct sc_video_buffer *vb) {
+    sc_frame_buffer_destroy(&vb->fb);
 }
 
 bool
-video_buffer_push(struct video_buffer *vb, const AVFrame *frame,
+sc_video_buffer_push(struct sc_video_buffer *vb, const AVFrame *frame,
                   bool *previous_frame_skipped) {
-    sc_mutex_lock(&vb->mutex);
-
-    // 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(vb->tmp_frame, frame);
-    if (r) {
-        LOGE("Could not ref frame: %d", r);
-        return false;
-    }
-
-    // Now that av_frame_ref() succeeded, we can replace the previous
-    // pending_frame
-    swap_frames(&vb->pending_frame, &vb->tmp_frame);
-    av_frame_unref(vb->tmp_frame);
-
-    if (previous_frame_skipped) {
-        *previous_frame_skipped = !vb->pending_frame_consumed;
-    }
-    vb->pending_frame_consumed = false;
-
-    sc_mutex_unlock(&vb->mutex);
-
-    return true;
+    return sc_frame_buffer_push(&vb->fb, frame, previous_frame_skipped);
 }
 
 void
-video_buffer_consume(struct video_buffer *vb, AVFrame *dst) {
-    sc_mutex_lock(&vb->mutex);
-    assert(!vb->pending_frame_consumed);
-    vb->pending_frame_consumed = true;
-
-    av_frame_move_ref(dst, vb->pending_frame);
-    // av_frame_move_ref() resets its source frame, so no need to call
-    // av_frame_unref()
-
-    sc_mutex_unlock(&vb->mutex);
+sc_video_buffer_consume(struct sc_video_buffer *vb, AVFrame *dst) {
+    sc_frame_buffer_consume(&vb->fb, dst);
 }

app/src/video_buffer.h

@@ -1,50 +1,30 @@
-#ifndef VIDEO_BUFFER_H
-#define VIDEO_BUFFER_H
+#ifndef SC_VIDEO_BUFFER_H
+#define SC_VIDEO_BUFFER_H
 
 #include "common.h"
 
 #include <stdbool.h>
 
-#include "fps_counter.h"
-#include "util/thread.h"
+#include "frame_buffer.h"
 
 // forward declarations
 typedef struct AVFrame AVFrame;
 
-/**
- * A video buffer holds 1 pending frame, which is the last frame received from
- * the producer (typically, the decoder).
- *
- * If a pending frame has not been consumed when the producer pushes a new
- * frame, then it is lost. The intent is to always provide access to the very
- * last frame to minimize latency.
- *
- * The producer and the consumer typically do not live in the same thread.
- * That's the reason why the callback on_frame_available() does not provide the
- * frame as parameter: the consumer might post an event to its own thread to
- * retrieve the pending frame from there, and that frame may have changed since
- * the callback if producer pushed a new one in between.
- */
-
-struct video_buffer {
-    AVFrame *pending_frame;
-    AVFrame *tmp_frame; // To preserve the pending frame on error
-
-    sc_mutex mutex;
-
-    bool pending_frame_consumed;
+struct sc_video_buffer {
+    struct sc_frame_buffer fb;
 };
 
 bool
-video_buffer_init(struct video_buffer *vb);
+sc_video_buffer_init(struct sc_video_buffer *vb);
 
 void
-video_buffer_destroy(struct video_buffer *vb);
+sc_video_buffer_destroy(struct sc_video_buffer *vb);
 
 bool
-video_buffer_push(struct video_buffer *vb, const AVFrame *frame, bool *skipped);
+sc_video_buffer_push(struct sc_video_buffer *vb, const AVFrame *frame,
+                     bool *skipped);
 
 void
-video_buffer_consume(struct video_buffer *vb, AVFrame *dst);
+sc_video_buffer_consume(struct sc_video_buffer *vb, AVFrame *dst);
 
 #endif