Skip to content

Commit

Permalink
Remove option --render-expired-frames
Browse files Browse the repository at this point in the history
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
ebccb9f.

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".
  • Loading branch information
rom1v committed Apr 11, 2021
1 parent a34bd5c commit 3ffea72
Show file tree
Hide file tree
Showing 12 changed files with 7 additions and 96 deletions.
12 changes: 0 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions app/scrcpy.1
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 2 additions & 7 deletions app/src/cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,6 @@ scrcpy_print_usage(const char *arg0) {
" \"opengles2\", \"opengles\", \"metal\" and \"software\".\n"
" <https://wiki.libsdl.org/SDL_HINT_RENDER_DRIVER>\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"
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 0 additions & 5 deletions app/src/decoder.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
3 changes: 0 additions & 3 deletions app/src/decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 3 additions & 6 deletions app/src/scrcpy.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 0 additions & 2 deletions app/src/scrcpy.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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, \
Expand Down
7 changes: 0 additions & 7 deletions app/src/stream.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions app/src/stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
38 changes: 1 addition & 37 deletions app/src/video_buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
}
9 changes: 1 addition & 8 deletions app/src/video_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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
2 changes: 0 additions & 2 deletions app/tests/test_cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 3ffea72

Please sign in to comment.