From e2a272bf99ecf48fcb050177113f903b3fb323c4 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Fri, 7 Jun 2019 16:55:19 +0200 Subject: [PATCH] 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 | 163 +++++++++++++++++++++++++++++++++------- app/src/fps_counter.h | 36 +++++++-- app/src/input_manager.c | 21 ++++-- app/src/scrcpy.c | 18 ++++- app/src/video_buffer.c | 16 ++-- app/src/video_buffer.h | 5 +- 6 files changed, 207 insertions(+), 52 deletions(-) diff --git a/app/src/fps_counter.c b/app/src/fps_counter.c index 133c7a25..daece470 100644 --- a/app/src/fps_counter.c +++ b/app/src/fps_counter.c @@ -1,60 +1,169 @@ #include "fps_counter.h" +#include #include +#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; - // no need to initialize the other fields, they are meaningful only when - // started is true + counter->mutex = SDL_CreateMutex(); + if (!counter->mutex) { + 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) { - counter->started = true; - counter->slice_start = SDL_GetTicks(); +fps_counter_destroy(struct fps_counter *counter) { + SDL_DestroyCond(counter->state_cond); + 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 -display_fps(struct fps_counter *counter) { - if (counter->nr_skipped) { - LOGI("%d fps (+%d frames skipped)", counter->nr_rendered, - counter->nr_skipped); - } else { - LOGI("%d fps", counter->nr_rendered); +bool +fps_counter_is_started(struct fps_counter *counter) { + return SDL_AtomicGet(&counter->started); +} + +void +fps_counter_interrupt(struct fps_counter *counter) { + if (!counter->thread) { + return; } + + mutex_lock(counter->mutex); + counter->interrupted = true; + mutex_unlock(counter->mutex); + // wake up blocking wait + cond_signal(counter->state_cond); } -static void -check_expired(struct fps_counter *counter) { - uint32_t now = SDL_GetTicks(); - if (now - counter->slice_start >= 1000) { - display_fps(counter); - // add a multiple of one second - uint32_t elapsed_slices = (now - counter->slice_start) / 1000; - counter->slice_start += 1000 * elapsed_slices; - counter->nr_rendered = 0; - counter->nr_skipped = 0; +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)) { + return; + } + + mutex_lock(counter->mutex); + uint32_t now = SDL_GetTicks(); + check_interval_expired(counter, now); ++counter->nr_skipped; + mutex_unlock(counter->mutex); } diff --git a/app/src/fps_counter.h b/app/src/fps_counter.h index fecef806..6b560a35 100644 --- a/app/src/fps_counter.h +++ b/app/src/fps_counter.h @@ -3,23 +3,49 @@ #include #include +#include +#include +#include struct fps_counter { - bool started; - uint32_t slice_start; // initialized by SDL_GetTicks() - int nr_rendered; - int nr_skipped; + SDL_Thread *thread; + SDL_mutex *mutex; + SDL_cond *state_cond; + + // 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); diff --git a/app/src/input_manager.c b/app/src/input_manager.c index 03f299db..fb8ef8f0 100644 --- a/app/src/input_manager.c +++ b/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) { - mutex_lock(vb->mutex); - if (vb->fps_counter.started) { +switch_fps_counter_state(struct fps_counter *fps_counter) { + // the started state can only be written from the current thread, so there + // 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 { - LOGI("FPS counter started"); - fps_counter_start(&vb->fps_counter); + if (fps_counter_start(fps_counter)) { + LOGI("FPS counter started"); + } 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: diff --git a/app/src/scrcpy.c b/app/src/scrcpy.c index 38ad1ceb..761edb69 100644 --- a/app/src/scrcpy.c +++ b/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" diff --git a/app/src/video_buffer.c b/app/src/video_buffer.c index f26af0b6..2b5f1c2f 100644 --- a/app/src/video_buffer.c +++ b/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 { - if (vb->fps_counter.started && !vb->rendering_frame_consumed) { - fps_counter_add_skipped_frame(&vb->fps_counter); - } + } else if (!vb->rendering_frame_consumed) { + 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); diff --git a/app/src/video_buffer.h b/app/src/video_buffer.h index 9f328172..26a6fa1f 100644 --- a/app/src/video_buffer.h +++ b/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);