You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2022/01/07 05:45:18 UTC

[GitHub] [guacamole-server] mike-jumper opened a new pull request #363: GUACAMOLE-377: Add support for RemoteFX.

mike-jumper opened a new pull request #363:
URL: https://github.com/apache/guacamole-server/pull/363


   This change adds initial RemoteFX support for RDP, enabled via the `enable-gfx` parameter. As RemoteFX includes explicit frame boundaries, and thus allows us to actually track when frames are received, when Guacamole needs to combine them, etc., these changes also include enhancements to the Guacamole protocol that allow the client to track performance with respect to framerate and frame dropping.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-server] necouchman commented on a change in pull request #363: GUACAMOLE-377: Add support for RemoteFX.

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #363:
URL: https://github.com/apache/guacamole-server/pull/363#discussion_r805330780



##########
File path: src/protocols/rdp/gdi.c
##########
@@ -371,9 +372,110 @@ BOOL guac_rdp_gdi_set_bounds(rdpContext* context, const rdpBounds* bounds) {
 
 }
 
+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->frames_received++;
+
+    /* Flush a new frame if the client is ready for it */
+    if (time_elapsed >= guac_client_get_processing_lag(client)) {
+        guac_common_display_flush(rdp_client->display);
+        guac_client_end_multiple_frames(client, 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) {
+    guac_rdp_gdi_mark_frame(context, frame_marker->action == FRAME_START);
+    return TRUE;
+}
+
+BOOL guac_rdp_gdi_surface_frame_marker(rdpContext* context, const SURFACE_FRAME_MARKER* surface_frame_marker) {
+
+    guac_rdp_gdi_mark_frame(context, surface_frame_marker->frameAction == SURFACECMD_FRAMEACTION_END);
+
+    if (context->settings->FrameAcknowledge > 0)
+        IFCALL(context->update->SurfaceFrameAcknowledge, context,
+                surface_frame_marker->frameId);
+
+    return TRUE;
+
+}
+
+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)
+        guac_rdp_gdi_mark_frame(context, 1);
+
+    return TRUE;
+
+}
+
 BOOL guac_rdp_gdi_end_paint(rdpContext* context) {
-    /* IGNORE */
+
+    guac_client* client = ((rdp_freerdp_context*) context)->client;
+    guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+    rdpGdi* gdi = context->gdi;
+
+    /* Leverate EndPaint handler to detect end of frame for RDPGFX channel
+     * (ignore otherwise) */
+    if (!rdp_client->settings->enable_gfx)
+        return TRUE;

Review comment:
       This comment may be a bit confusing - at least, it was to me initially. It seems like it's more likely describing what's done if the block of code below it is not true/executed, not what that block of code is actually doing?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-server] necouchman commented on a change in pull request #363: GUACAMOLE-377: Add support for RemoteFX.

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #363:
URL: https://github.com/apache/guacamole-server/pull/363#discussion_r805330780



##########
File path: src/protocols/rdp/gdi.c
##########
@@ -371,9 +372,110 @@ BOOL guac_rdp_gdi_set_bounds(rdpContext* context, const rdpBounds* bounds) {
 
 }
 
+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->frames_received++;
+
+    /* Flush a new frame if the client is ready for it */
+    if (time_elapsed >= guac_client_get_processing_lag(client)) {
+        guac_common_display_flush(rdp_client->display);
+        guac_client_end_multiple_frames(client, 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) {
+    guac_rdp_gdi_mark_frame(context, frame_marker->action == FRAME_START);
+    return TRUE;
+}
+
+BOOL guac_rdp_gdi_surface_frame_marker(rdpContext* context, const SURFACE_FRAME_MARKER* surface_frame_marker) {
+
+    guac_rdp_gdi_mark_frame(context, surface_frame_marker->frameAction == SURFACECMD_FRAMEACTION_END);
+
+    if (context->settings->FrameAcknowledge > 0)
+        IFCALL(context->update->SurfaceFrameAcknowledge, context,
+                surface_frame_marker->frameId);
+
+    return TRUE;
+
+}
+
+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)
+        guac_rdp_gdi_mark_frame(context, 1);
+
+    return TRUE;
+
+}
+
 BOOL guac_rdp_gdi_end_paint(rdpContext* context) {
-    /* IGNORE */
+
+    guac_client* client = ((rdp_freerdp_context*) context)->client;
+    guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+    rdpGdi* gdi = context->gdi;
+
+    /* Leverate EndPaint handler to detect end of frame for RDPGFX channel
+     * (ignore otherwise) */
+    if (!rdp_client->settings->enable_gfx)
+        return TRUE;

Review comment:
       This comment may be a bit confusing - at least, it was to me initially. It seems like it's more likely describing what's done if the block of code below it is not true/executed, not what that block of code is actually doing?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org