From 629c296207ea09cdf9820eefdb8d620fe1c7f0dd Mon Sep 17 00:00:00 2001
From: Romain Vimont <rvimont@genymobile.com>
Date: Thu, 8 Feb 2018 19:23:24 +0100
Subject: [PATCH] Move frame swapping logic to frame.c

Expose frames_offer_decoded_frame() and frames_consume_rendered_frame()
so that callers are not exposed to frame swapping (between the decoding
and rendering frames) details.
---
 app/src/decoder.c | 21 ++++-----------------
 app/src/frames.c  | 44 +++++++++++++++++++++++++++++++++++++++++++-
 app/src/frames.h  | 11 ++++++++++-
 app/src/scrcpy.c  |  9 +--------
 4 files changed, 58 insertions(+), 27 deletions(-)

diff --git a/app/src/decoder.c b/app/src/decoder.c
index 33a3b097..561d2dfe 100644
--- a/app/src/decoder.c
+++ b/app/src/decoder.c
@@ -21,24 +21,11 @@ static int read_packet(void *opaque, uint8_t *buf, int buf_size) {
 
 // set the decoded frame as ready for rendering, and notify
 static void push_frame(struct decoder *decoder) {
-    struct frames *frames = decoder->frames;
-    mutex_lock(frames->mutex);
-#ifndef SKIP_FRAMES
-    // if SKIP_FRAMES is disabled, then the decoder must wait for the current
-    // frame to be consumed
-    while (!frames->rendering_frame_consumed) {
-        cond_wait(frames->rendering_frame_consumed_cond, frames->mutex);
+    SDL_bool previous_frame_consumed = frames_offer_decoded_frame(decoder->frames);
+    if (!previous_frame_consumed) {
+        // the previous EVENT_NEW_FRAME will consume this frame
+        return;
     }
-#else
-    if (!frames->rendering_frame_consumed) {
-        SDL_LogInfo(SDL_LOG_CATEGORY_RENDER, "Skip frame");
-    }
-#endif
-
-    frames_swap(frames);
-    frames->rendering_frame_consumed = SDL_FALSE;
-    mutex_unlock(frames->mutex);
-
     static SDL_Event new_frame_event = {
         .type = EVENT_NEW_FRAME,
     };
diff --git a/app/src/frames.c b/app/src/frames.c
index cb8d7e79..7025a830 100644
--- a/app/src/frames.c
+++ b/app/src/frames.c
@@ -1,9 +1,14 @@
 #include "frames.h"
 
+#include <SDL2/SDL_assert.h>
+#include <SDL2/SDL_log.h>
 #include <SDL2/SDL_mutex.h>
 #include <libavutil/avutil.h>
 #include <libavformat/avformat.h>
 
+#include "config.h"
+#include "lockutil.h"
+
 SDL_bool frames_init(struct frames *frames) {
     if (!(frames->decoding_frame = av_frame_alloc())) {
         goto error_0;
@@ -24,6 +29,8 @@ SDL_bool frames_init(struct frames *frames) {
     }
 #endif
 
+    // there is initially no rendering frame, so consider it has already been
+    // consumed
     frames->rendering_frame_consumed = SDL_TRUE;
 
     return SDL_TRUE;
@@ -45,8 +52,43 @@ void frames_destroy(struct frames *frames) {
     av_frame_free(&frames->decoding_frame);
 }
 
-void frames_swap(struct frames *frames) {
+static void frames_swap(struct frames *frames) {
     AVFrame *tmp = frames->decoding_frame;
     frames->decoding_frame = frames->rendering_frame;
     frames->rendering_frame = tmp;
 }
+
+SDL_bool frames_offer_decoded_frame(struct frames *frames) {
+    mutex_lock(frames->mutex);
+    SDL_bool previous_frame_consumed;
+#ifndef SKIP_FRAMES
+    // if SKIP_FRAMES is disabled, then the decoder must wait for the current
+    // frame to be consumed
+    while (!frames->rendering_frame_consumed) {
+        cond_wait(frames->rendering_frame_consumed_cond, frames->mutex);
+    }
+    // by definition, we are not skipping the frames
+    previous_frame_consumed = SDL_TRUE;
+#else
+    previous_frame_consumed = frames->rendering_frame_consumed;
+    if (!previous_frame_consumed) {
+        SDL_LogDebug(SDL_LOG_CATEGORY_RENDER, "Skip frame");
+    }
+#endif
+
+    frames_swap(frames);
+    frames->rendering_frame_consumed = SDL_FALSE;
+    mutex_unlock(frames->mutex);
+    return previous_frame_consumed;
+}
+
+const AVFrame *frames_consume_rendered_frame(struct frames *frames) {
+    SDL_assert(!frames->rendering_frame_consumed);
+    frames->rendering_frame_consumed = SDL_TRUE;
+#ifndef SKIP_FRAMES
+    // if SKIP_FRAMES is disabled, then notify the decoder the current frame is
+    // consumed, so that it may push a new one
+    cond_signal(frames->rendering_frame_consumed_cond);
+#endif
+    return frames->rendering_frame;
+}
diff --git a/app/src/frames.h b/app/src/frames.h
index ba4d5fe7..f96675b2 100644
--- a/app/src/frames.h
+++ b/app/src/frames.h
@@ -22,6 +22,15 @@ struct frames {
 SDL_bool frames_init(struct frames *frames);
 void frames_destroy(struct frames *frames);
 
-void frames_swap(struct frames *frames);
+// set the decoder frame as ready for rendering
+// this function locks frames->mutex during its execution
+// returns true if the previous frame had been consumed
+SDL_bool frames_offer_decoded_frame(struct frames *frames);
+
+// mark the rendering frame as consumed and return it
+// MUST be called with frames->mutex locked!!!
+// the caller is expected to render the returned frame to some texture before
+// unlocking frames->mutex
+const AVFrame *frames_consume_rendered_frame(struct frames *frames);
 
 #endif
diff --git a/app/src/scrcpy.c b/app/src/scrcpy.c
index 8616c82f..51974f3e 100644
--- a/app/src/scrcpy.c
+++ b/app/src/scrcpy.c
@@ -48,14 +48,7 @@ static void count_frame(void) {
 
 static SDL_bool handle_new_frame(void) {
     mutex_lock(frames.mutex);
-    AVFrame *frame = frames.rendering_frame;
-    frames.rendering_frame_consumed = SDL_TRUE;
-#ifndef SKIP_FRAMES
-    // if SKIP_FRAMES is disabled, then notify the decoder the current frame is
-    // consumed, so that it may push a new one
-    cond_signal(frames.rendering_frame_consumed_cond);
-#endif
-
+    const AVFrame *frame = frames_consume_rendered_frame(&frames);
     if (!screen_update(&screen, frame)){
         mutex_unlock(frames.mutex);
         return SDL_FALSE;