From 8595862005ebfb8a02081bea593884b308c19b49 Mon Sep 17 00:00:00 2001
From: Romain Vimont <rom@rom1v.com>
Date: Sun, 3 Mar 2019 00:26:48 +0100
Subject: [PATCH] Use explicit output parameter for skipped frame

The function video_buffer_offer_decoded_frame() returned a bool to
indicate whether the previous frame had been consumed.

This was confusing, because we could expect the returned bool report
whether the action succeeded.

Make the semantic explicit by using an output parameter.

Also revert the flag (report if the frame has been skipped instead of
consumed) to avoid confusion for the first frame (the previous is
neither skipped nor consumed because there is no previous frame).
---
 app/src/decoder.c      | 7 ++++---
 app/src/video_buffer.c | 8 ++++----
 app/src/video_buffer.h | 7 ++++---
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/app/src/decoder.c b/app/src/decoder.c
index 59bd94ce..b206ebe0 100644
--- a/app/src/decoder.c
+++ b/app/src/decoder.c
@@ -20,9 +20,10 @@
 // set the decoded frame as ready for rendering, and notify
 static void
 push_frame(struct decoder *decoder) {
-    bool previous_frame_consumed =
-            video_buffer_offer_decoded_frame(decoder->video_buffer);
-    if (!previous_frame_consumed) {
+    bool previous_frame_skipped;
+    video_buffer_offer_decoded_frame(decoder->video_buffer,
+                                     &previous_frame_skipped);
+    if (previous_frame_skipped) {
         // the previous EVENT_NEW_FRAME will consume this frame
         return;
     }
diff --git a/app/src/video_buffer.c b/app/src/video_buffer.c
index b56ae9f2..8f1d1d9d 100644
--- a/app/src/video_buffer.c
+++ b/app/src/video_buffer.c
@@ -63,8 +63,9 @@ video_buffer_swap_frames(struct video_buffer *vb) {
     vb->rendering_frame = tmp;
 }
 
-bool
-video_buffer_offer_decoded_frame(struct video_buffer *vb) {
+void
+video_buffer_offer_decoded_frame(struct video_buffer *vb,
+                                 bool *previous_frame_skipped) {
     mutex_lock(vb->mutex);
 #ifndef SKIP_FRAMES
     // if SKIP_FRAMES is disabled, then the decoder must wait for the current
@@ -80,11 +81,10 @@ video_buffer_offer_decoded_frame(struct video_buffer *vb) {
 
     video_buffer_swap_frames(vb);
 
-    bool previous_frame_consumed = vb->rendering_frame_consumed;
+    *previous_frame_skipped = !vb->rendering_frame_consumed;
     vb->rendering_frame_consumed = false;
 
     mutex_unlock(vb->mutex);
-    return previous_frame_consumed;
 }
 
 const AVFrame *
diff --git a/app/src/video_buffer.h b/app/src/video_buffer.h
index 03bcaa18..93222236 100644
--- a/app/src/video_buffer.h
+++ b/app/src/video_buffer.h
@@ -30,9 +30,10 @@ video_buffer_destroy(struct video_buffer *vb);
 
 // set the decoded frame as ready for rendering
 // this function locks frames->mutex during its execution
-// returns true if the previous frame had been consumed
-bool
-video_buffer_offer_decoded_frame(struct video_buffer *vb);
+// the output flag is set to report whether the previous frame has been skipped
+void
+video_buffer_offer_decoded_frame(struct video_buffer *vb,
+                                 bool *previous_frame_skipped);
 
 // mark the rendering frame as consumed and return it
 // MUST be called with frames->mutex locked!!!