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
ebccb9f6cc.

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".
This commit is contained in:
Romain Vimont 2021-04-11 15:01:05 +02:00
parent 21b590b766
commit 55806e7d31
12 changed files with 7 additions and 96 deletions

View file

@ -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 #### Show touches
For presentations, it may be useful to show physical touches (on the physical For presentations, it may be useful to show physical touches (on the physical

View file

@ -155,10 +155,6 @@ Supported names are currently "direct3d", "opengl", "opengles2", "opengles", "me
.UR https://wiki.libsdl.org/SDL_HINT_RENDER_DRIVER .UR https://wiki.libsdl.org/SDL_HINT_RENDER_DRIVER
.UE .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 .TP
.BI "\-\-rotation " value .BI "\-\-rotation " value
Set the initial display rotation. Possibles values are 0, 1, 2 and 3. Each increment adds a 90 degrees rotation counterclockwise. Set the initial display rotation. Possibles values are 0, 1, 2 and 3. Each increment adds a 90 degrees rotation counterclockwise.

View file

@ -143,12 +143,6 @@ scrcpy_print_usage(const char *arg0) {
" \"opengles2\", \"opengles\", \"metal\" and \"software\".\n" " \"opengles2\", \"opengles\", \"metal\" and \"software\".\n"
" <https://wiki.libsdl.org/SDL_HINT_RENDER_DRIVER>\n" " <https://wiki.libsdl.org/SDL_HINT_RENDER_DRIVER>\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" " --rotation value\n"
" Set the initial display rotation.\n" " Set the initial display rotation.\n"
" Possibles values are 0, 1, 2 and 3. Each increment adds a 90\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; opts->stay_awake = true;
break; break;
case OPT_RENDER_EXPIRED_FRAMES: case OPT_RENDER_EXPIRED_FRAMES:
opts->render_expired_frames = true; LOGW("Option --render-expired-frames has been removed. This "
"flag has been ignored.");
break; break;
case OPT_WINDOW_TITLE: case OPT_WINDOW_TITLE:
opts->window_title = optarg; opts->window_title = optarg;

View file

@ -69,8 +69,3 @@ decoder_push(struct decoder *decoder, const AVPacket *packet) {
#endif #endif
return true; return true;
} }
void
decoder_interrupt(struct decoder *decoder) {
video_buffer_interrupt(decoder->video_buffer);
}

View file

@ -26,7 +26,4 @@ decoder_close(struct decoder *decoder);
bool bool
decoder_push(struct decoder *decoder, const AVPacket *packet); decoder_push(struct decoder *decoder, const AVPacket *packet);
void
decoder_interrupt(struct decoder *decoder);
#endif #endif

View file

@ -305,7 +305,7 @@ scrcpy(const struct scrcpy_options *options) {
} }
fps_counter_initialized = true; fps_counter_initialized = true;
if (!video_buffer_init(&video_buffer, options->render_expired_frames)) { if (!video_buffer_init(&video_buffer)) {
goto end; goto end;
} }
video_buffer_initialized = true; video_buffer_initialized = true;
@ -398,11 +398,8 @@ scrcpy(const struct scrcpy_options *options) {
LOGD("quit..."); LOGD("quit...");
end: end:
// stop stream and controller so that they don't continue once their socket // The stream is not stopped explicitly, because it will stop by itself on
// is shutdown // end-of-stream
if (stream_started) {
stream_stop(&stream);
}
if (controller_started) { if (controller_started) {
controller_stop(&controller); controller_stop(&controller);
} }

View file

@ -72,7 +72,6 @@ struct scrcpy_options {
bool control; bool control;
bool display; bool display;
bool turn_screen_off; bool turn_screen_off;
bool render_expired_frames;
bool prefer_text; bool prefer_text;
bool window_borderless; bool window_borderless;
bool mipmaps; bool mipmaps;
@ -120,7 +119,6 @@ struct scrcpy_options {
.control = true, \ .control = true, \
.display = true, \ .display = true, \
.turn_screen_off = false, \ .turn_screen_off = false, \
.render_expired_frames = false, \
.prefer_text = false, \ .prefer_text = false, \
.window_borderless = false, \ .window_borderless = false, \
.mipmaps = true, \ .mipmaps = true, \

View file

@ -285,13 +285,6 @@ stream_start(struct stream *stream) {
return true; return true;
} }
void
stream_stop(struct stream *stream) {
if (stream->decoder) {
decoder_interrupt(stream->decoder);
}
}
void void
stream_join(struct stream *stream) { stream_join(struct stream *stream) {
sc_thread_join(&stream->thread, NULL); sc_thread_join(&stream->thread, NULL);

View file

@ -31,9 +31,6 @@ stream_init(struct stream *stream, socket_t socket,
bool bool
stream_start(struct stream *stream); stream_start(struct stream *stream);
void
stream_stop(struct stream *stream);
void void
stream_join(struct stream *stream); stream_join(struct stream *stream);

View file

@ -7,7 +7,7 @@
#include "util/log.h" #include "util/log.h"
bool bool
video_buffer_init(struct video_buffer *vb, bool wait_consumer) { video_buffer_init(struct video_buffer *vb) {
vb->producer_frame = av_frame_alloc(); vb->producer_frame = av_frame_alloc();
if (!vb->producer_frame) { if (!vb->producer_frame) {
goto error_0; goto error_0;
@ -28,18 +28,6 @@ video_buffer_init(struct video_buffer *vb, bool wait_consumer) {
goto error_3; 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 // there is initially no frame, so consider it has already been consumed
vb->pending_frame_consumed = true; vb->pending_frame_consumed = true;
@ -61,9 +49,6 @@ error_0:
void void
video_buffer_destroy(struct video_buffer *vb) { video_buffer_destroy(struct video_buffer *vb) {
if (vb->wait_consumer) {
sc_cond_destroy(&vb->pending_frame_consumed_cond);
}
sc_mutex_destroy(&vb->mutex); sc_mutex_destroy(&vb->mutex);
av_frame_free(&vb->consumer_frame); av_frame_free(&vb->consumer_frame);
av_frame_free(&vb->pending_frame); av_frame_free(&vb->pending_frame);
@ -93,12 +78,6 @@ video_buffer_producer_offer_frame(struct video_buffer *vb) {
assert(vb->cbs); assert(vb->cbs);
sc_mutex_lock(&vb->mutex); 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); av_frame_unref(vb->pending_frame);
swap_frames(&vb->producer_frame, &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); swap_frames(&vb->consumer_frame, &vb->pending_frame);
av_frame_unref(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); sc_mutex_unlock(&vb->mutex);
// consumer_frame is only written from this thread, no need to lock // consumer_frame is only written from this thread, no need to lock
return vb->consumer_frame; 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);
}
}

View file

@ -34,10 +34,7 @@ struct video_buffer {
AVFrame *consumer_frame; AVFrame *consumer_frame;
sc_mutex mutex; 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; bool pending_frame_consumed;
const struct video_buffer_callbacks *cbs; const struct video_buffer_callbacks *cbs;
@ -56,7 +53,7 @@ struct video_buffer_callbacks {
}; };
bool bool
video_buffer_init(struct video_buffer *vb, bool wait_consumer); video_buffer_init(struct video_buffer *vb);
void void
video_buffer_destroy(struct video_buffer *vb); video_buffer_destroy(struct video_buffer *vb);
@ -75,8 +72,4 @@ video_buffer_producer_offer_frame(struct video_buffer *vb);
const AVFrame * const AVFrame *
video_buffer_consumer_take_frame(struct video_buffer *vb); 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 #endif

View file

@ -58,7 +58,6 @@ static void test_options(void) {
"--push-target", "/sdcard/Movies", "--push-target", "/sdcard/Movies",
"--record", "file", "--record", "file",
"--record-format", "mkv", "--record-format", "mkv",
"--render-expired-frames",
"--serial", "0123456789abcdef", "--serial", "0123456789abcdef",
"--show-touches", "--show-touches",
"--turn-screen-off", "--turn-screen-off",
@ -87,7 +86,6 @@ static void test_options(void) {
assert(!strcmp(opts->push_target, "/sdcard/Movies")); assert(!strcmp(opts->push_target, "/sdcard/Movies"));
assert(!strcmp(opts->record_filename, "file")); assert(!strcmp(opts->record_filename, "file"));
assert(opts->record_format == SC_RECORD_FORMAT_MKV); assert(opts->record_format == SC_RECORD_FORMAT_MKV);
assert(opts->render_expired_frames);
assert(!strcmp(opts->serial, "0123456789abcdef")); assert(!strcmp(opts->serial, "0123456789abcdef"));
assert(opts->show_touches); assert(opts->show_touches);
assert(opts->turn_screen_off); assert(opts->turn_screen_off);