From 3ffea72fa65e7289a9d46ea37809328fd869d38f Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Sun, 11 Apr 2021 15:01:05 +0200 Subject: [PATCH] Remove option --render-expired-frames This flag forced the decoder to wait for the previous frame to be consumed by the display. It was initially implemented as a compilation flag for testing, not intended to be exposed at runtime. But to remove ifdefs and to allow users to test this flag easily, it had finally been exposed by commit ebccb9f6cc111e8acfbe10d656cac5c1f1b744a0. In practice, it turned out to be useless: it had no practical impact, and it did not solve or mitigate any performance issues causing frame skipping. But that added some complexity to the codebase: it required an additional condition variable, and made video buffer calls possibly blocking, which in turn required code to interrupt it on exit. To prepare support for multiple sinks plugged to the decoder (display and v4l2 for example), the blocking call used for pacing the decoder output becomes unacceptable, so just remove this useless "feature". --- README.md | 12 ------------ app/scrcpy.1 | 4 ---- app/src/cli.c | 9 ++------- app/src/decoder.c | 5 ----- app/src/decoder.h | 3 --- app/src/scrcpy.c | 9 +++------ app/src/scrcpy.h | 2 -- app/src/stream.c | 7 ------- app/src/stream.h | 3 --- app/src/video_buffer.c | 38 +------------------------------------- app/src/video_buffer.h | 9 +-------- app/tests/test_cli.c | 2 -- 12 files changed, 7 insertions(+), 96 deletions(-) diff --git a/README.md b/README.md index 6a987a73a3..5f583b8116 100644 --- a/README.md +++ b/README.md @@ -491,18 +491,6 @@ scrcpy -Sw ``` -#### Render expired frames - -By default, to minimize latency, _scrcpy_ always renders the last decoded frame -available, and drops any previous one. - -To force the rendering of all frames (at a cost of a possible increased -latency), use: - -```bash -scrcpy --render-expired-frames -``` - #### Show touches For presentations, it may be useful to show physical touches (on the physical diff --git a/app/scrcpy.1 b/app/scrcpy.1 index 92b8e1e3ce..be361c720e 100644 --- a/app/scrcpy.1 +++ b/app/scrcpy.1 @@ -155,10 +155,6 @@ Supported names are currently "direct3d", "opengl", "opengles2", "opengles", "me .UR https://wiki.libsdl.org/SDL_HINT_RENDER_DRIVER .UE -.TP -.B \-\-render\-expired\-frames -By default, to minimize latency, scrcpy always renders the last available decoded frame, and drops any previous ones. This flag forces to render all frames, at a cost of a possible increased latency. - .TP .BI "\-\-rotation " value Set the initial display rotation. Possibles values are 0, 1, 2 and 3. Each increment adds a 90 degrees rotation counterclockwise. diff --git a/app/src/cli.c b/app/src/cli.c index 484c48ef6a..b85bc10f0f 100644 --- a/app/src/cli.c +++ b/app/src/cli.c @@ -143,12 +143,6 @@ scrcpy_print_usage(const char *arg0) { " \"opengles2\", \"opengles\", \"metal\" and \"software\".\n" " \n" "\n" - " --render-expired-frames\n" - " By default, to minimize latency, scrcpy always renders the\n" - " last available decoded frame, and drops any previous ones.\n" - " This flag forces to render all frames, at a cost of a\n" - " possible increased latency.\n" - "\n" " --rotation value\n" " Set the initial display rotation.\n" " Possibles values are 0, 1, 2 and 3. Each increment adds a 90\n" @@ -816,7 +810,8 @@ scrcpy_parse_args(struct scrcpy_cli_args *args, int argc, char *argv[]) { opts->stay_awake = true; break; case OPT_RENDER_EXPIRED_FRAMES: - opts->render_expired_frames = true; + LOGW("Option --render-expired-frames has been removed. This " + "flag has been ignored."); break; case OPT_WINDOW_TITLE: opts->window_title = optarg; diff --git a/app/src/decoder.c b/app/src/decoder.c index a13cf75eb8..b71011941b 100644 --- a/app/src/decoder.c +++ b/app/src/decoder.c @@ -69,8 +69,3 @@ decoder_push(struct decoder *decoder, const AVPacket *packet) { #endif return true; } - -void -decoder_interrupt(struct decoder *decoder) { - video_buffer_interrupt(decoder->video_buffer); -} diff --git a/app/src/decoder.h b/app/src/decoder.h index bbd7a9a763..ba06583e1d 100644 --- a/app/src/decoder.h +++ b/app/src/decoder.h @@ -26,7 +26,4 @@ decoder_close(struct decoder *decoder); bool decoder_push(struct decoder *decoder, const AVPacket *packet); -void -decoder_interrupt(struct decoder *decoder); - #endif diff --git a/app/src/scrcpy.c b/app/src/scrcpy.c index e6ae4f4c77..2c0025e1c3 100644 --- a/app/src/scrcpy.c +++ b/app/src/scrcpy.c @@ -334,7 +334,7 @@ scrcpy(const struct scrcpy_options *options) { } fps_counter_initialized = true; - if (!video_buffer_init(&video_buffer, options->render_expired_frames)) { + if (!video_buffer_init(&video_buffer)) { goto end; } video_buffer_initialized = true; @@ -430,11 +430,8 @@ scrcpy(const struct scrcpy_options *options) { LOGD("quit..."); end: - // stop stream and controller so that they don't continue once their socket - // is shutdown - if (stream_started) { - stream_stop(&stream); - } + // The stream is not stopped explicitly, because it will stop by itself on + // end-of-stream if (controller_started) { controller_stop(&controller); } diff --git a/app/src/scrcpy.h b/app/src/scrcpy.h index 8ee70f606b..f91cb6b88f 100644 --- a/app/src/scrcpy.h +++ b/app/src/scrcpy.h @@ -72,7 +72,6 @@ struct scrcpy_options { bool control; bool display; bool turn_screen_off; - bool render_expired_frames; bool prefer_text; bool window_borderless; bool mipmaps; @@ -120,7 +119,6 @@ struct scrcpy_options { .control = true, \ .display = true, \ .turn_screen_off = false, \ - .render_expired_frames = false, \ .prefer_text = false, \ .window_borderless = false, \ .mipmaps = true, \ diff --git a/app/src/stream.c b/app/src/stream.c index ba72f1642e..e0a223be3f 100644 --- a/app/src/stream.c +++ b/app/src/stream.c @@ -285,13 +285,6 @@ stream_start(struct stream *stream) { return true; } -void -stream_stop(struct stream *stream) { - if (stream->decoder) { - decoder_interrupt(stream->decoder); - } -} - void stream_join(struct stream *stream) { sc_thread_join(&stream->thread, NULL); diff --git a/app/src/stream.h b/app/src/stream.h index d4a9bd4afb..421c1bf01e 100644 --- a/app/src/stream.h +++ b/app/src/stream.h @@ -31,9 +31,6 @@ stream_init(struct stream *stream, socket_t socket, bool stream_start(struct stream *stream); -void -stream_stop(struct stream *stream); - void stream_join(struct stream *stream); diff --git a/app/src/video_buffer.c b/app/src/video_buffer.c index 94619840c2..d4954afcf5 100644 --- a/app/src/video_buffer.c +++ b/app/src/video_buffer.c @@ -7,7 +7,7 @@ #include "util/log.h" bool -video_buffer_init(struct video_buffer *vb, bool wait_consumer) { +video_buffer_init(struct video_buffer *vb) { vb->producer_frame = av_frame_alloc(); if (!vb->producer_frame) { goto error_0; @@ -28,18 +28,6 @@ video_buffer_init(struct video_buffer *vb, bool wait_consumer) { goto error_3; } - vb->wait_consumer = wait_consumer; - if (wait_consumer) { - ok = sc_cond_init(&vb->pending_frame_consumed_cond); - if (!ok) { - sc_mutex_destroy(&vb->mutex); - goto error_2; - } - // interrupted is not used if wait_consumer is disabled since offering - // a frame will never block - vb->interrupted = false; - } - // there is initially no frame, so consider it has already been consumed vb->pending_frame_consumed = true; @@ -61,9 +49,6 @@ video_buffer_init(struct video_buffer *vb, bool wait_consumer) { void video_buffer_destroy(struct video_buffer *vb) { - if (vb->wait_consumer) { - sc_cond_destroy(&vb->pending_frame_consumed_cond); - } sc_mutex_destroy(&vb->mutex); av_frame_free(&vb->consumer_frame); av_frame_free(&vb->pending_frame); @@ -93,12 +78,6 @@ video_buffer_producer_offer_frame(struct video_buffer *vb) { assert(vb->cbs); sc_mutex_lock(&vb->mutex); - if (vb->wait_consumer) { - // wait for the current (expired) frame to be consumed - while (!vb->pending_frame_consumed && !vb->interrupted) { - sc_cond_wait(&vb->pending_frame_consumed_cond, &vb->mutex); - } - } av_frame_unref(vb->pending_frame); swap_frames(&vb->producer_frame, &vb->pending_frame); @@ -125,23 +104,8 @@ video_buffer_consumer_take_frame(struct video_buffer *vb) { swap_frames(&vb->consumer_frame, &vb->pending_frame); av_frame_unref(vb->pending_frame); - if (vb->wait_consumer) { - // unblock video_buffer_offer_decoded_frame() - sc_cond_signal(&vb->pending_frame_consumed_cond); - } sc_mutex_unlock(&vb->mutex); // consumer_frame is only written from this thread, no need to lock return vb->consumer_frame; } - -void -video_buffer_interrupt(struct video_buffer *vb) { - if (vb->wait_consumer) { - sc_mutex_lock(&vb->mutex); - vb->interrupted = true; - sc_mutex_unlock(&vb->mutex); - // wake up blocking wait - sc_cond_signal(&vb->pending_frame_consumed_cond); - } -} diff --git a/app/src/video_buffer.h b/app/src/video_buffer.h index 4d11e3ab37..48e57ff422 100644 --- a/app/src/video_buffer.h +++ b/app/src/video_buffer.h @@ -34,10 +34,7 @@ struct video_buffer { AVFrame *consumer_frame; sc_mutex mutex; - bool wait_consumer; // never overwrite a pending frame if it is not consumed - bool interrupted; - sc_cond pending_frame_consumed_cond; bool pending_frame_consumed; const struct video_buffer_callbacks *cbs; @@ -56,7 +53,7 @@ struct video_buffer_callbacks { }; bool -video_buffer_init(struct video_buffer *vb, bool wait_consumer); +video_buffer_init(struct video_buffer *vb); void video_buffer_destroy(struct video_buffer *vb); @@ -75,8 +72,4 @@ video_buffer_producer_offer_frame(struct video_buffer *vb); const AVFrame * video_buffer_consumer_take_frame(struct video_buffer *vb); -// wake up and avoid any blocking call -void -video_buffer_interrupt(struct video_buffer *vb); - #endif diff --git a/app/tests/test_cli.c b/app/tests/test_cli.c index cd222d6363..3fa9b3d781 100644 --- a/app/tests/test_cli.c +++ b/app/tests/test_cli.c @@ -58,7 +58,6 @@ static void test_options(void) { "--push-target", "/sdcard/Movies", "--record", "file", "--record-format", "mkv", - "--render-expired-frames", "--serial", "0123456789abcdef", "--show-touches", "--turn-screen-off", @@ -87,7 +86,6 @@ static void test_options(void) { assert(!strcmp(opts->push_target, "/sdcard/Movies")); assert(!strcmp(opts->record_filename, "file")); assert(opts->record_format == SC_RECORD_FORMAT_MKV); - assert(opts->render_expired_frames); assert(!strcmp(opts->serial, "0123456789abcdef")); assert(opts->show_touches); assert(opts->turn_screen_off);