GUACAMOLE-377: Restore heuristic detection of RDP frame boundaries.

apache · Sep 9, 2024 · 7a55ca0 · 7a55ca0
1 parent 9627a2f
commit 7a55ca0
Show file tree

Hide file tree

Showing 6 changed files with 9 additions and 123 deletions.
diff --git a/src/protocols/rdp/client.h b/src/protocols/rdp/client.h
@@ -32,7 +32,7 @@
  * milliseconds. If the server is silent for at least this amount of time, the
  * frame will be considered finished.
  */
-#define GUAC_RDP_FRAME_TIMEOUT 0
+#define GUAC_RDP_FRAME_TIMEOUT 16
 
 /**
  * The amount of time to wait for a new message from the RDP server when

diff --git a/src/protocols/rdp/gdi.c b/src/protocols/rdp/gdi.c
@@ -38,30 +38,16 @@ void guac_rdp_gdi_mark_frame(rdpContext* context, int starting) {
     guac_client* client = ((rdp_freerdp_context*) context)->client;
     guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
 
-    /* The server supports defining explicit frames */
-    rdp_client->frames_supported = 1;
-
     /* A new frame is beginning */
     if (starting) {
         rdp_client->in_frame = 1;
         return;
     }
 
-    /* The current frame has ended */
-    guac_timestamp frame_end = guac_timestamp_current();
-    int time_elapsed = frame_end - client->last_sent_timestamp;
-    rdp_client->in_frame = 0;
-
     /* A new frame has been received from the RDP server and processed */
+    rdp_client->in_frame = 0;
     rdp_client->frames_received++;
 
-    /* Flush a new frame if the client is ready for it */
-    if (time_elapsed >= guac_client_get_processing_lag(client)) {
-        guac_display_end_multiple_frames(rdp_client->display, rdp_client->frames_received);
-        guac_socket_flush(client->socket);
-        rdp_client->frames_received = 0;
-    }
-
 }
 
 BOOL guac_rdp_gdi_frame_marker(rdpContext* context, const FRAME_MARKER_ORDER* frame_marker) {
@@ -88,19 +74,6 @@ BOOL guac_rdp_gdi_surface_frame_marker(rdpContext* context, const SURFACE_FRAME_
 
 }
 
-BOOL guac_rdp_gdi_begin_paint(rdpContext* context) {
-
-    guac_client* client = ((rdp_freerdp_context*) context)->client;
-    guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
-
-    /* Leverage BeginPaint handler to detect start of frame for RDPGFX channel */
-    if (rdp_client->settings->enable_gfx && rdp_client->frames_supported)
-        guac_rdp_gdi_mark_frame(context, 1);
-
-    return TRUE;
-
-}
-
 BOOL guac_rdp_gdi_end_paint(rdpContext* context) {
 
     guac_client* client = ((rdp_freerdp_context*) context)->client;
@@ -135,11 +108,6 @@ BOOL guac_rdp_gdi_end_paint(rdpContext* context) {
 
     guac_display_layer_close_raw(default_layer, dst_context);
 
-    /* Next frame */
-    if (gdi->inGfxFrame) {
-        guac_rdp_gdi_mark_frame(context, 0);
-    }
-
     return TRUE;
 
 }

diff --git a/src/protocols/rdp/gdi.h b/src/protocols/rdp/gdi.h
@@ -72,22 +72,8 @@ BOOL guac_rdp_gdi_frame_marker(rdpContext* context, const FRAME_MARKER_ORDER* fr
 BOOL guac_rdp_gdi_surface_frame_marker(rdpContext* context, const SURFACE_FRAME_MARKER* surface_frame_marker);
 
 /**
- * Handler called when a paint operation is beginning. This function is
- * expected to be called by the FreeRDP GDI implementation of RemoteFX when a
- * new frame has started.
- *
- * @param context
- *     The rdpContext associated with the current RDP session.
- *
- * @return
- *     TRUE if successful, FALSE otherwise.
- */
-BOOL guac_rdp_gdi_begin_paint(rdpContext* context);
-
-/**
- * Handler called when a paint operation is complete. This function is
- * expected to be called by the FreeRDP GDI implementation of RemoteFX when a
- * new frame has been completed.
+ * Handler called when FreeRDP has finished performing updates to the backing
+ * surface of its GDI (graphics) implementation.
  *
  * @param context
  *     The rdpContext associated with the current RDP session.

diff --git a/src/protocols/rdp/pointer.c b/src/protocols/rdp/pointer.c
@@ -75,14 +75,6 @@ BOOL guac_rdp_pointer_set(rdpContext* context, POINTER_SET_CONST rdpPointer* poi
     guac_client* client = ((rdp_freerdp_context*) context)->client;
     guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
 
-    /* Add explicit frame boundaries around cursor set operation if not already
-     * in a frame (the RDP protocol does not send nor expect frame boundaries
-     * for cursor changes, but Guacamole does expect this) */
-    int in_frame = rdp_client->in_frame;
-
-    if (rdp_client->frames_supported && !in_frame)
-        guac_rdp_gdi_mark_frame(context, 1);
-
     guac_display_layer* src_layer = ((guac_rdp_pointer*) pointer)->layer;
     guac_display_layer_raw_context* src_context = guac_display_layer_open_raw(src_layer);
 
@@ -111,9 +103,7 @@ BOOL guac_rdp_pointer_set(rdpContext* context, POINTER_SET_CONST rdpPointer* poi
     guac_display_layer_close_raw(cursor_layer, dst_context);
     guac_display_layer_close_raw(src_layer, src_context);
 
-    if (rdp_client->frames_supported && !in_frame)
-        guac_rdp_gdi_mark_frame(context, 0);
-
+    guac_display_end_mouse_frame(rdp_client->display);
     return TRUE;
 
 }
@@ -135,20 +125,10 @@ BOOL guac_rdp_pointer_set_null(rdpContext* context) {
     guac_client* client = ((rdp_freerdp_context*) context)->client;
     guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
 
-    /* Add explicit frame boundaries around cursor set operation if not already
-     * in a frame (the RDP protocol does not send nor expect frame boundaries
-     * for cursor changes, but Guacamole does expect this) */
-    int in_frame = rdp_client->in_frame;
-
-    if (rdp_client->frames_supported && !in_frame)
-        guac_rdp_gdi_mark_frame(context, 1);
-
     /* Set cursor to empty/blank graphic */
     guac_display_set_cursor(rdp_client->display, GUAC_DISPLAY_CURSOR_NONE);
 
-    if (rdp_client->frames_supported && !in_frame)
-        guac_rdp_gdi_mark_frame(context, 0);
-
+    guac_display_end_mouse_frame(rdp_client->display);
     return TRUE;
 
 }
@@ -158,19 +138,9 @@ BOOL guac_rdp_pointer_set_default(rdpContext* context) {
     guac_client* client = ((rdp_freerdp_context*) context)->client;
     guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
 
-    /* Add explicit frame boundaries around cursor set operation if not already
-     * in a frame (the RDP protocol does not send nor expect frame boundaries
-     * for cursor changes, but Guacamole does expect this) */
-    int in_frame = rdp_client->in_frame;
-
-    if (rdp_client->frames_supported && !in_frame)
-        guac_rdp_gdi_mark_frame(context, 1);
-
     /* Set cursor to embedded pointer */
     guac_display_set_cursor(rdp_client->display, GUAC_DISPLAY_CURSOR_POINTER);
 
-    if (rdp_client->frames_supported && !in_frame)
-        guac_rdp_gdi_mark_frame(context, 0);
-
+    guac_display_end_mouse_frame(rdp_client->display);
     return TRUE;
 }
diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c
@@ -159,7 +159,6 @@ BOOL rdp_freerdp_pre_connect(freerdp* instance) {
 
     /* Set up GDI */
     GUAC_RDP_CONTEXT(instance)->update->DesktopResize = guac_rdp_gdi_desktop_resize;
-    GUAC_RDP_CONTEXT(instance)->update->BeginPaint = guac_rdp_gdi_begin_paint;
     GUAC_RDP_CONTEXT(instance)->update->EndPaint = guac_rdp_gdi_end_paint;
 
     GUAC_RDP_CONTEXT(instance)->update->SurfaceFrameMarker = guac_rdp_gdi_surface_frame_marker;
@@ -528,14 +527,9 @@ static int guac_rdp_handle_connection(guac_client* client) {
                 GUAC_RDP_FRAME_START_TIMEOUT);
         if (wait_result > 0) {
 
-            int processing_lag = guac_client_get_processing_lag(client);
-
             /* Read server messages until frame is built */
             do {
 
-                guac_timestamp frame_end;
-                int frame_remaining;
-
                 /* Handle any queued FreeRDP events (this may result in RDP
                  * messages being sent) */
                 pthread_mutex_lock(&(rdp_client->message_lock));
@@ -548,34 +542,7 @@ static int guac_rdp_handle_connection(guac_client* client) {
                     break;
                 }
 
-                /* Continue handling inbound data if we are in the middle of an RDP frame */
-                if (rdp_client->in_frame) {
-                    wait_result = rdp_guac_client_wait_for_messages(client, GUAC_RDP_FRAME_START_TIMEOUT);
-                    if (wait_result >= 0)
-                        continue;
-                }
-
-                /* Calculate time remaining in frame */
-                guac_timestamp frame_start = client->last_sent_timestamp;
-                frame_end = guac_timestamp_current();
-                frame_remaining = frame_start + GUAC_RDP_FRAME_DURATION
-                                - frame_end;
-
-                /* Calculate time that client needs to catch up */
-                int time_elapsed = frame_end - frame_start;
-                int required_wait = processing_lag - time_elapsed;
-
-                /* Increase the duration of this frame if client is lagging */
-                if (required_wait > GUAC_RDP_FRAME_TIMEOUT)
-                    wait_result = rdp_guac_client_wait_for_messages(client,
-                            required_wait);
-
-                /* Wait again if frame remaining */
-                else if (frame_remaining > 0)
-                    wait_result = rdp_guac_client_wait_for_messages(client,
-                            GUAC_RDP_FRAME_TIMEOUT);
-                else
-                    break;
+                wait_result = rdp_guac_client_wait_for_messages(client, GUAC_RDP_FRAME_TIMEOUT);
 
             } while (wait_result > 0);
 
@@ -600,7 +567,7 @@ static int guac_rdp_handle_connection(guac_client* client) {
 
         /* Flush frame only if successful and an RDP frame is not known to be
          * in progress */
-        else if (!rdp_client->frames_supported || rdp_client->frames_received) {
+        else if (!rdp_client->in_frame) {
             guac_display_end_multiple_frames(rdp_client->display, rdp_client->frames_received);
             guac_socket_flush(client->socket);
             rdp_client->frames_received = 0;

diff --git a/src/protocols/rdp/rdp.h b/src/protocols/rdp/rdp.h
@@ -106,11 +106,6 @@ typedef struct guac_rdp_client {
      */
     guac_display_layer* current_surface;
 
-    /**
-     * Whether the RDP server supports defining explicit frame boundaries.
-     */
-    int frames_supported;
-
     /**
      * Whether the RDP server has reported that a new frame is in progress, and
      * we are now receiving updates relevant to that frame.