[gnome-remote-desktop] rdp-graphics-pipeline: Create separate render surface when needed

[Jonas Ådahl <jadahl src gnome org>]

commit 2be38c330f2ccf46277cddf6149ac04ffedbb433
Author: Pascal Nowack <Pascal Nowack gmx de>
Date:   Fri Feb 11 14:00:09 2022 +0100

    rdp-graphics-pipeline: Create separate render surface when needed
    
    The documentation of the graphics pipeline says:
    [...] "the width and height of the MPEG-4 AVC/H.264 codec bitstream
    MUST be aligned to a multiple of 16 and MUST be cropped by the region
    mask specified in the regionRects field that is embedded in the
    avc420MetaData field." ([MS-RDPEGFX] 2.2.4.4 RFX_AVC420_BITMAP_STREAM)
    
    For the NVENC session, the height of each frame needs to be aligned to
    a multiple of 64.
    64 is a multiple of 16, but mstsc actually requires the bitstream to be
    aligned to 16x16 tiles, as long as these tiles cover the surface.
    This last part is not mentioned in the documentation, but this applies
    to a lot of things of the RDP protocol.
    In case of an error in the documentation, the behaviour of mstsc and
    the RDS in MS Windows determine the behaviour of the RDP protocol.
    So, adjust the implementation in gnome-remote-desktop accordingly.
    
    Whenever the aligned height of the NVENC session, which is a multiple
    of 64, does not match the aligned height of the mapped surface, create
    a separate render surface.
    This render surface will then have the aligned size of the NVENC
    session and will be used to render the data to on the client side.
    With the work in the previous commits, the client is instructed to blit
    from the render surface to the mapped surface, when the render surface
    needs to be used.
    
    Without these changes, mstsc would abort the remote desktop connection
    upon receiving a RDPGFX_WIRE_TO_SURFACE_PDU_1 message with a protocol
    error.

 src/grd-rdp-graphics-pipeline.c | 97 +++++++++++++++++++++++++++--------------
 1 file changed, 65 insertions(+), 32 deletions(-)
---
diff --git a/src/grd-rdp-graphics-pipeline.c b/src/grd-rdp-graphics-pipeline.c
index ba36fa33..88535d28 100644
--- a/src/grd-rdp-graphics-pipeline.c
+++ b/src/grd-rdp-graphics-pipeline.c
@@ -33,6 +33,7 @@
 #include "grd-rdp-network-autodetection.h"
 #include "grd-rdp-surface.h"
 #include "grd-session-rdp.h"
+#include "grd-utils.h"
 
 #define ENC_TIMES_CHECK_INTERVAL_MS 1000
 #define MAX_TRACKED_ENC_FRAMES 1000
@@ -117,6 +118,38 @@ grd_rdp_graphics_pipeline_set_hwaccel_nvidia (GrdRdpGraphicsPipeline *graphics_p
   graphics_pipeline->hwaccel_nvidia = hwaccel_nvidia;
 }
 
+static uint16_t
+get_next_free_surface_id (GrdRdpGraphicsPipeline *graphics_pipeline)
+{
+  uint16_t surface_id = graphics_pipeline->next_surface_id;
+
+  g_mutex_lock (&graphics_pipeline->gfx_mutex);
+  while (g_hash_table_contains (graphics_pipeline->surface_table,
+                                GUINT_TO_POINTER (surface_id)))
+    ++surface_id;
+  g_mutex_unlock (&graphics_pipeline->gfx_mutex);
+
+  graphics_pipeline->next_surface_id = surface_id + 1;
+
+  return surface_id;
+}
+
+static uint32_t
+get_next_free_serial (GrdRdpGraphicsPipeline *graphics_pipeline)
+{
+  uint32_t serial = graphics_pipeline->next_serial;
+
+  g_mutex_lock (&graphics_pipeline->gfx_mutex);
+  while (g_hash_table_contains (graphics_pipeline->serial_surface_table,
+                                GUINT_TO_POINTER (serial)))
+    ++serial;
+  g_mutex_unlock (&graphics_pipeline->gfx_mutex);
+
+  graphics_pipeline->next_serial = serial + 1;
+
+  return serial;
+}
+
 void
 grd_rdp_graphics_pipeline_create_surface (GrdRdpGraphicsPipeline *graphics_pipeline,
                                           GrdRdpGfxSurface       *gfx_surface)
@@ -129,6 +162,7 @@ grd_rdp_graphics_pipeline_create_surface (GrdRdpGraphicsPipeline *graphics_pipel
   uint16_t surface_width = grd_rdp_gfx_surface_get_width (gfx_surface);
   uint16_t surface_height = grd_rdp_gfx_surface_get_height (gfx_surface);
   GfxSurfaceContext *surface_context;
+  gboolean needs_separate_render_surface = FALSE;
   HWAccelContext *hwaccel_context;
   uint32_t encode_session_id;
   uint16_t aligned_width;
@@ -155,8 +189,16 @@ grd_rdp_graphics_pipeline_create_surface (GrdRdpGraphicsPipeline *graphics_pipel
                                                &aligned_width, &aligned_height,
                                                rdp_surface->refresh_rate))
     {
+      uint16_t aligned_width_16;
+      uint16_t aligned_height_16;
+
       g_debug ("[RDP.RDPGFX] Creating NVENC session for surface %u", surface_id);
 
+      aligned_width_16 = grd_get_aligned_size (surface_width, 16);
+      aligned_height_16 = grd_get_aligned_size (surface_height, 16);
+      if (aligned_width != aligned_width_16 || aligned_height != aligned_height_16)
+        needs_separate_render_surface = TRUE;
+
       hwaccel_context = g_malloc0 (sizeof (HWAccelContext));
       hwaccel_context->api = HW_ACCEL_API_NVENC;
       hwaccel_context->encode_session_id = encode_session_id;
@@ -174,6 +216,29 @@ grd_rdp_graphics_pipeline_create_surface (GrdRdpGraphicsPipeline *graphics_pipel
   create_surface.pixelFormat = GFX_PIXEL_FORMAT_XRGB_8888;
 
   rdpgfx_context->CreateSurface (rdpgfx_context, &create_surface);
+
+  if (needs_separate_render_surface)
+    {
+      g_autoptr (GrdRdpGfxSurface) render_surface = NULL;
+      GrdRdpGfxSurfaceDescriptor surface_descriptor = {};
+
+      g_debug ("[RDP.RDPGFX] Creating separate render surface for surface %u",
+               surface_id);
+
+      surface_descriptor.flags = GRD_RDP_GFX_SURFACE_FLAG_ALIGNED_SIZE |
+                                 GRD_RDP_GFX_SURFACE_FLAG_NO_HWACCEL_SESSIONS;
+      surface_descriptor.surface_id = get_next_free_surface_id (graphics_pipeline);
+      surface_descriptor.serial = get_next_free_serial (graphics_pipeline);
+      surface_descriptor.rdp_surface = rdp_surface;
+
+      surface_descriptor.aligned_width = aligned_width;
+      surface_descriptor.aligned_height = aligned_height;
+
+      render_surface = grd_rdp_gfx_surface_new (graphics_pipeline,
+                                                &surface_descriptor);
+      grd_rdp_gfx_surface_override_render_surface (gfx_surface,
+                                                   g_steal_pointer (&render_surface));
+    }
 }
 
 void
@@ -855,38 +920,6 @@ refresh_gfx_surface_rfx_progressive (GrdRdpGraphicsPipeline *graphics_pipeline,
   return TRUE;
 }
 
-static uint16_t
-get_next_free_surface_id (GrdRdpGraphicsPipeline *graphics_pipeline)
-{
-  uint16_t surface_id = graphics_pipeline->next_surface_id;
-
-  g_mutex_lock (&graphics_pipeline->gfx_mutex);
-  while (g_hash_table_contains (graphics_pipeline->surface_table,
-                                GUINT_TO_POINTER (surface_id)))
-    ++surface_id;
-  g_mutex_unlock (&graphics_pipeline->gfx_mutex);
-
-  graphics_pipeline->next_surface_id = surface_id + 1;
-
-  return surface_id;
-}
-
-static uint32_t
-get_next_free_serial (GrdRdpGraphicsPipeline *graphics_pipeline)
-{
-  uint32_t serial = graphics_pipeline->next_serial;
-
-  g_mutex_lock (&graphics_pipeline->gfx_mutex);
-  while (g_hash_table_contains (graphics_pipeline->serial_surface_table,
-                                GUINT_TO_POINTER (serial)))
-    ++serial;
-  g_mutex_unlock (&graphics_pipeline->gfx_mutex);
-
-  graphics_pipeline->next_serial = serial + 1;
-
-  return serial;
-}
-
 static void
 map_surface_to_output (GrdRdpGraphicsPipeline *graphics_pipeline,
                        GrdRdpGfxSurface       *gfx_surface)