Improve framerate counting

The FPS counter was called only on new frames, so it could not print
values regularly, especially when there are very few FPS (when the
device surface does not change).

To the extreme, it was never able to display 0 fps.

Add a separate thread to print framerate every second.

app/src/fps_counter.c

@@ -1,60 +1,169 @@
 #include "fps_counter.h"
 
+#include <SDL2/SDL_assert.h>
 #include <SDL2/SDL_timer.h>
 
+#include "lock_util.h"
 #include "log.h"
 
-void
+#define FPS_COUNTER_INTERVAL_MS 1000
+
+bool
 fps_counter_init(struct fps_counter *counter) {
-    counter->started = false;
+    counter->mutex = SDL_CreateMutex();
-    // no need to initialize the other fields, they are meaningful only when
+    if (!counter->mutex) {
-    // started is true
+        return false;
+    }
+
+    counter->state_cond = SDL_CreateCond();
+    if (!counter->state_cond) {
+        SDL_DestroyMutex(counter->mutex);
+        return false;
+    }
+
+    counter->thread = NULL;
+    SDL_AtomicSet(&counter->started, 0);
+    // no need to initialize the other fields, they are unused until started
+
+    return true;
 }
 
 void
-fps_counter_start(struct fps_counter *counter) {
+fps_counter_destroy(struct fps_counter *counter) {
-    counter->started = true;
+    SDL_DestroyCond(counter->state_cond);
-    counter->slice_start = SDL_GetTicks();
+    SDL_DestroyMutex(counter->mutex);
+}
+
+// must be called with mutex locked
+static void
+display_fps(struct fps_counter *counter) {
+    unsigned rendered_per_second =
+        counter->nr_rendered * 1000 / FPS_COUNTER_INTERVAL_MS;
+    if (counter->nr_skipped) {
+        LOGI("%u fps (+%u frames skipped)", rendered_per_second,
+                                            counter->nr_skipped);
+    } else {
+        LOGI("%u fps", rendered_per_second);
+    }
+}
+
+// must be called with mutex locked
+static void
+check_interval_expired(struct fps_counter *counter, uint32_t now) {
+    if (now < counter->next_timestamp) {
+        return;
+    }
+
+    display_fps(counter);
     counter->nr_rendered = 0;
     counter->nr_skipped = 0;
+    // add a multiple of the interval
+    uint32_t elapsed_slices =
+        (now - counter->next_timestamp) / FPS_COUNTER_INTERVAL_MS + 1;
+    counter->next_timestamp += FPS_COUNTER_INTERVAL_MS * elapsed_slices;
+}
+
+static int
+run_fps_counter(void *data) {
+    struct fps_counter *counter = data;
+
+    mutex_lock(counter->mutex);
+    while (!counter->interrupted) {
+        while (!counter->interrupted && !SDL_AtomicGet(&counter->started)) {
+            cond_wait(counter->state_cond, counter->mutex);
+        }
+        while (!counter->interrupted && SDL_AtomicGet(&counter->started)) {
+            uint32_t now = SDL_GetTicks();
+            check_interval_expired(counter, now);
+
+            SDL_assert(counter->next_timestamp > now);
+            uint32_t remaining = counter->next_timestamp - now;
+
+            // ignore the reason (timeout or signaled), we just loop anyway
+            cond_wait_timeout(counter->state_cond, counter->mutex, remaining);
+        }
+    }
+    mutex_unlock(counter->mutex);
+    return 0;
+}
+
+bool
+fps_counter_start(struct fps_counter *counter) {
+    mutex_lock(counter->mutex);
+    counter->next_timestamp = SDL_GetTicks() + FPS_COUNTER_INTERVAL_MS;
+    counter->nr_rendered = 0;
+    counter->nr_skipped = 0;
+    mutex_unlock(counter->mutex);
+
+    SDL_AtomicSet(&counter->started, 1);
+    cond_signal(counter->state_cond);
+
+    // counter->thread is always accessed from the same thread, no need to lock
+    if (!counter->thread) {
+        counter->thread =
+            SDL_CreateThread(run_fps_counter, "fps counter", counter);
+        if (!counter->thread) {
+            LOGE("Could not start FPS counter thread");
+            return false;
+        }
+    }
+
+    return true;
 }
 
 void
 fps_counter_stop(struct fps_counter *counter) {
-    counter->started = false;
+    SDL_AtomicSet(&counter->started, 0);
+    cond_signal(counter->state_cond);
 }
 
-static void
+bool
-display_fps(struct fps_counter *counter) {
+fps_counter_is_started(struct fps_counter *counter) {
-    if (counter->nr_skipped) {
+    return SDL_AtomicGet(&counter->started);
-        LOGI("%d fps (+%d frames skipped)", counter->nr_rendered,
-                                            counter->nr_skipped);
-    } else {
-        LOGI("%d fps", counter->nr_rendered);
-    }
 }
 
-static void
+void
-check_expired(struct fps_counter *counter) {
+fps_counter_interrupt(struct fps_counter *counter) {
-    uint32_t now = SDL_GetTicks();
+    if (!counter->thread) {
-    if (now - counter->slice_start >= 1000) {
+        return;
-        display_fps(counter);
+    }
-        // add a multiple of one second
+
-        uint32_t elapsed_slices = (now - counter->slice_start) / 1000;
+    mutex_lock(counter->mutex);
-        counter->slice_start += 1000 * elapsed_slices;
+    counter->interrupted = true;
-        counter->nr_rendered = 0;
+    mutex_unlock(counter->mutex);
-        counter->nr_skipped = 0;
+    // wake up blocking wait
+    cond_signal(counter->state_cond);
+}
+
+void
+fps_counter_join(struct fps_counter *counter) {
+    if (counter->thread) {
+        SDL_WaitThread(counter->thread, NULL);
     }
 }
 
 void
 fps_counter_add_rendered_frame(struct fps_counter *counter) {
-    check_expired(counter);
+    if (!SDL_AtomicGet(&counter->started)) {
+        return;
+    }
+
+    mutex_lock(counter->mutex);
+    uint32_t now = SDL_GetTicks();
+    check_interval_expired(counter, now);
     ++counter->nr_rendered;
+    mutex_unlock(counter->mutex);
 }
 
 void
 fps_counter_add_skipped_frame(struct fps_counter *counter) {
-    check_expired(counter);
+    if (!SDL_AtomicGet(&counter->started)) {
-    ++counter->nr_skipped;
+        return;
+    }
+
+    mutex_lock(counter->mutex);
+    uint32_t now = SDL_GetTicks();
+    check_interval_expired(counter, now);
+    ++counter->nr_skipped;
+    mutex_unlock(counter->mutex);
 }

app/src/fps_counter.h

@@ -3,23 +3,49 @@
 
 #include <stdbool.h>
 #include <stdint.h>
+#include <SDL2/SDL_atomic.h>
+#include <SDL2/SDL_mutex.h>
+#include <SDL2/SDL_thread.h>
 
 struct fps_counter {
-    bool started;
+    SDL_Thread *thread;
-    uint32_t slice_start; // initialized by SDL_GetTicks()
+    SDL_mutex *mutex;
-    int nr_rendered;
+    SDL_cond *state_cond;
-    int nr_skipped;
+
+    // atomic so that we can check without locking the mutex
+    // if the FPS counter is disabled, we don't want to lock unnecessarily
+    SDL_atomic_t started;
+
+    // the following fields are protected by the mutex
+    bool interrupted;
+    unsigned nr_rendered;
+    unsigned nr_skipped;
+    uint32_t next_timestamp;
 };
 
-void
+bool
 fps_counter_init(struct fps_counter *counter);
 
 void
+fps_counter_destroy(struct fps_counter *counter);
+
+bool
 fps_counter_start(struct fps_counter *counter);
 
 void
 fps_counter_stop(struct fps_counter *counter);
 
+bool
+fps_counter_is_started(struct fps_counter *counter);
+
+// request to stop the thread (on quit)
+// must be called before fps_counter_join()
+void
+fps_counter_interrupt(struct fps_counter *counter);
+
+void
+fps_counter_join(struct fps_counter *counter);
+
 void
 fps_counter_add_rendered_frame(struct fps_counter *counter);
 

app/src/input_manager.c

@@ -172,16 +172,19 @@ set_screen_power_mode(struct controller *controller,
 }
 
 static void
-switch_fps_counter_state(struct video_buffer *vb) {
+switch_fps_counter_state(struct fps_counter *fps_counter) {
-    mutex_lock(vb->mutex);
+    // the started state can only be written from the current thread, so there
-    if (vb->fps_counter.started) {
+    // is no ToCToU issue
+    if (fps_counter_is_started(fps_counter)) {
+        fps_counter_stop(fps_counter);
         LOGI("FPS counter stopped");
-        fps_counter_stop(&vb->fps_counter);
     } else {
+        if (fps_counter_start(fps_counter)) {
             LOGI("FPS counter started");
-        fps_counter_start(&vb->fps_counter);
+        } else {
+            LOGE("FPS counter starting failed");
+        }
     }
-    mutex_unlock(vb->mutex);
 }
 
 static void
@@ -339,7 +342,9 @@ input_manager_process_key(struct input_manager *input_manager,
                 return;
             case SDLK_i:
                 if (ctrl && !meta && !shift && !repeat && down) {
-                    switch_fps_counter_state(input_manager->video_buffer);
+                    struct fps_counter *fps_counter =
+                        input_manager->video_buffer->fps_counter;
+                    switch_fps_counter_state(fps_counter);
                 }
                 return;
             case SDLK_n:

app/src/scrcpy.c

@@ -29,6 +29,7 @@
 
 static struct server server = SERVER_INITIALIZER;
 static struct screen screen = SCREEN_INITIALIZER;
+static struct fps_counter fps_counter;
 static struct video_buffer video_buffer;
 static struct stream stream;
 static struct decoder decoder;
@@ -293,6 +294,7 @@ scrcpy(const struct scrcpy_options *options) {
 
     bool ret = false;
 
+    bool fps_counter_initialized = false;
     bool video_buffer_initialized = false;
     bool file_handler_initialized = false;
     bool recorder_initialized = false;
@@ -320,7 +322,13 @@ scrcpy(const struct scrcpy_options *options) {
 
     struct decoder *dec = NULL;
     if (options->display) {
-        if (!video_buffer_init(&video_buffer, options->render_expired_frames)) {
+        if (!fps_counter_init(&fps_counter)) {
+            goto end;
+        }
+        fps_counter_initialized = true;
+
+        if (!video_buffer_init(&video_buffer, &fps_counter,
+                               options->render_expired_frames)) {
             goto end;
         }
         video_buffer_initialized = true;
@@ -414,6 +422,9 @@ end:
     if (file_handler_initialized) {
         file_handler_stop(&file_handler);
     }
+    if (fps_counter_initialized) {
+        fps_counter_interrupt(&fps_counter);
+    }
 
     // shutdown the sockets and kill the server
     server_stop(&server);
@@ -443,6 +454,11 @@ end:
         video_buffer_destroy(&video_buffer);
     }
 
+    if (fps_counter_initialized) {
+        fps_counter_join(&fps_counter);
+        fps_counter_destroy(&fps_counter);
+    }
+
     if (options->show_touches) {
         if (!show_touches_waited) {
             // wait the process which enabled "show touches"

app/src/video_buffer.c

@@ -10,7 +10,10 @@
 #include "log.h"
 
 bool
-video_buffer_init(struct video_buffer *vb, bool render_expired_frames) {
+video_buffer_init(struct video_buffer *vb, struct fps_counter *fps_counter,
+                  bool render_expired_frames) {
+    vb->fps_counter = fps_counter;
+
     if (!(vb->decoding_frame = av_frame_alloc())) {
         goto error_0;
     }
@@ -37,7 +40,6 @@ video_buffer_init(struct video_buffer *vb, bool render_expired_frames) {
     // there is initially no rendering frame, so consider it has already been
     // consumed
     vb->rendering_frame_consumed = true;
-    fps_counter_init(&vb->fps_counter);
 
     return true;
 
@@ -75,10 +77,8 @@ video_buffer_offer_decoded_frame(struct video_buffer *vb,
         while (!vb->rendering_frame_consumed && !vb->interrupted) {
             cond_wait(vb->rendering_frame_consumed_cond, vb->mutex);
         }
-    } else {
+    } else if (!vb->rendering_frame_consumed) {
-        if (vb->fps_counter.started && !vb->rendering_frame_consumed) {
+        fps_counter_add_skipped_frame(vb->fps_counter);
-            fps_counter_add_skipped_frame(&vb->fps_counter);
-        }
     }
 
     video_buffer_swap_frames(vb);
@@ -93,9 +93,7 @@ const AVFrame *
 video_buffer_consume_rendered_frame(struct video_buffer *vb) {
     SDL_assert(!vb->rendering_frame_consumed);
     vb->rendering_frame_consumed = true;
-    if (vb->fps_counter.started) {
+    fps_counter_add_rendered_frame(vb->fps_counter);
-        fps_counter_add_rendered_frame(&vb->fps_counter);
-    }
     if (vb->render_expired_frames) {
         // unblock video_buffer_offer_decoded_frame()
         cond_signal(vb->rendering_frame_consumed_cond);

app/src/video_buffer.h

@@ -17,11 +17,12 @@ struct video_buffer {
     bool interrupted;
     SDL_cond *rendering_frame_consumed_cond;
     bool rendering_frame_consumed;
-    struct fps_counter fps_counter;
+    struct fps_counter *fps_counter;
 };
 
 bool
-video_buffer_init(struct video_buffer *vb, bool render_expired_frames);
+video_buffer_init(struct video_buffer *vb, struct fps_counter *fps_counter,
+                  bool render_expired_frames);
 
 void
 video_buffer_destroy(struct video_buffer *vb);