From 9e6e4e520c2f37ff71ce0045372b359e62f1067d Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Tue, 4 Oct 2016 13:26:32 -0700 Subject: [PATCH 1/4] GUACAMOLE-172: Clarify processing lag calculations. --- src/libguac/user-handlers.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libguac/user-handlers.c b/src/libguac/user-handlers.c index 9cd558c1..40ccb893 100644 --- a/src/libguac/user-handlers.c +++ b/src/libguac/user-handlers.c @@ -104,9 +104,8 @@ int __guac_handle_sync(guac_user* user, int argc, char** argv) { /* Update lag statistics if at least one frame has been rendered */ if (user->last_frame_duration != 0) { - /* Approximate processing lag by summing the frame duration deltas */ - int processing_lag = user->processing_lag + frame_duration - - user->last_frame_duration; + /* Calculate lag using the previous frame as a baseline */ + int processing_lag = frame_duration - user->last_frame_duration; /* Adjust back to zero if cumulative error leads to a negative value */ if (processing_lag < 0) @@ -116,8 +115,8 @@ int __guac_handle_sync(guac_user* user, int argc, char** argv) { } - /* Record duration of frame */ - user->last_frame_duration = frame_duration; + /* Record baseline duration of frame by excluding lag */ + user->last_frame_duration = frame_duration - user->processing_lag; if (user->sync_handler) return user->sync_handler(user, timestamp); From f641d91b552fdf27ac7de7e253912c7b31d7c0dd Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 7 Oct 2016 14:02:17 -0700 Subject: [PATCH 2/4] GUACAMOLE-172: Exclude server-side rendering time from next frame's required wait (if render times are consistent, then including that time will result in duplicate waiting: once within the render loop, and again when actually flushing the display). --- src/protocols/rdp/rdp.c | 10 +++++++--- src/protocols/vnc/vnc.c | 9 ++++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index e759888a..cf55c753 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -839,14 +839,18 @@ static int guac_rdp_handle_connection(guac_client* client) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, "Connection closed."); + /* Record end of frame, excluding server-side rendering time (we assume + * server-side rendering time will be consistent between any two + * subsequent frames, and that this time should thus be excluded from + * the required wait period of the next frame). */ + last_frame_end = guac_timestamp_current(); + + /* Flush frame */ /* End of frame */ guac_common_display_flush(rdp_client->display); guac_client_end_frame(client); guac_socket_flush(client->socket); - /* Record end of frame */ - last_frame_end = guac_timestamp_current(); - } pthread_mutex_lock(&(rdp_client->rdp_lock)); diff --git a/src/protocols/vnc/vnc.c b/src/protocols/vnc/vnc.c index f9ab6873..4f1e8da1 100644 --- a/src/protocols/vnc/vnc.c +++ b/src/protocols/vnc/vnc.c @@ -408,14 +408,17 @@ void* guac_vnc_client_thread(void* data) { if (wait_result < 0) guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, "Connection closed."); + /* Record end of frame, excluding server-side rendering time (we assume + * server-side rendering time will be consistent between any two + * subsequent frames, and that this time should thus be excluded from + * the required wait period of the next frame). */ + last_frame_end = guac_timestamp_current(); + /* Flush frame */ guac_common_surface_flush(vnc_client->display->default_surface); guac_client_end_frame(client); guac_socket_flush(client->socket); - /* Record end of frame */ - last_frame_end = guac_timestamp_current(); - } /* Kill client and finish connection */ From 234f98705ea8b9a56fda43fb7c1682e675760cb2 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 14 Oct 2016 13:31:41 -0700 Subject: [PATCH 3/4] GUACAMOLE-172: Use frame start as end of previous frame (ignore server-side time). --- src/protocols/rdp/rdp.c | 14 +++++++------- src/protocols/vnc/vnc.c | 12 ++++++------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index cf55c753..91f6eb98 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -832,6 +832,13 @@ static int guac_rdp_handle_connection(guac_client* client) { break; } while (wait_result > 0); + + /* Record end of frame, excluding server-side rendering time (we + * assume server-side rendering time will be consistent between any + * two subsequent frames, and that this time should thus be + * excluded from the required wait period of the next frame). */ + last_frame_end = frame_start; + } /* If an error occurred, fail */ @@ -839,14 +846,7 @@ static int guac_rdp_handle_connection(guac_client* client) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, "Connection closed."); - /* Record end of frame, excluding server-side rendering time (we assume - * server-side rendering time will be consistent between any two - * subsequent frames, and that this time should thus be excluded from - * the required wait period of the next frame). */ - last_frame_end = guac_timestamp_current(); - /* Flush frame */ - /* End of frame */ guac_common_display_flush(rdp_client->display); guac_client_end_frame(client); guac_socket_flush(client->socket); diff --git a/src/protocols/vnc/vnc.c b/src/protocols/vnc/vnc.c index 4f1e8da1..db56d8bb 100644 --- a/src/protocols/vnc/vnc.c +++ b/src/protocols/vnc/vnc.c @@ -402,18 +402,18 @@ void* guac_vnc_client_thread(void* data) { } while (wait_result > 0); + /* Record end of frame, excluding server-side rendering time (we + * assume server-side rendering time will be consistent between any + * two subsequent frames, and that this time should thus be + * excluded from the required wait period of the next frame). */ + last_frame_end = frame_start; + } /* If an error occurs, log it and fail */ if (wait_result < 0) guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, "Connection closed."); - /* Record end of frame, excluding server-side rendering time (we assume - * server-side rendering time will be consistent between any two - * subsequent frames, and that this time should thus be excluded from - * the required wait period of the next frame). */ - last_frame_end = guac_timestamp_current(); - /* Flush frame */ guac_common_surface_flush(vnc_client->display->default_surface); guac_client_end_frame(client); From 6131ad03413aadf6667762f20dd28351f80c382a Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 15 Jan 2017 13:59:15 -0800 Subject: [PATCH 4/4] GUACAMOLE-172: Ignore insane timestamps when calculating lag. --- src/libguac/user-handlers.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/libguac/user-handlers.c b/src/libguac/user-handlers.c index 40ccb893..4f1f59b0 100644 --- a/src/libguac/user-handlers.c +++ b/src/libguac/user-handlers.c @@ -95,29 +95,35 @@ int __guac_handle_sync(guac_user* user, int argc, char** argv) { if (timestamp > user->client->last_sent_timestamp) return -1; - /* Update stored timestamp */ - user->last_received_timestamp = timestamp; + /* Only update lag calculations if timestamp is sane */ + if (timestamp >= user->last_received_timestamp) { - /* Calculate length of frame, including network and processing lag */ - frame_duration = current - timestamp; + /* Update stored timestamp */ + user->last_received_timestamp = timestamp; - /* Update lag statistics if at least one frame has been rendered */ - if (user->last_frame_duration != 0) { + /* Calculate length of frame, including network and processing lag */ + frame_duration = current - timestamp; - /* Calculate lag using the previous frame as a baseline */ - int processing_lag = frame_duration - user->last_frame_duration; + /* Update lag statistics if at least one frame has been rendered */ + if (user->last_frame_duration != 0) { - /* Adjust back to zero if cumulative error leads to a negative value */ - if (processing_lag < 0) - processing_lag = 0; + /* Calculate lag using the previous frame as a baseline */ + int processing_lag = frame_duration - user->last_frame_duration; - user->processing_lag = processing_lag; + /* Adjust back to zero if cumulative error leads to a negative + * value */ + if (processing_lag < 0) + processing_lag = 0; + + user->processing_lag = processing_lag; + + } + + /* Record baseline duration of frame by excluding lag */ + user->last_frame_duration = frame_duration - user->processing_lag; } - /* Record baseline duration of frame by excluding lag */ - user->last_frame_duration = frame_duration - user->processing_lag; - if (user->sync_handler) return user->sync_handler(user, timestamp); return 0;