From 95bfb349a8833ea51c1932df6e4c17445131e61c Mon Sep 17 00:00:00 2001 From: Jay Sorg Date: Fri, 22 Mar 2024 14:20:54 -0700 Subject: [PATCH 1/2] gfx send multiple wire to surface messages when compressed data is larger than max_compressed_bytes --- librfxcodec | 2 +- xrdp/xrdp_encoder.c | 278 +++++++++++++++++++++++++------------------- 2 files changed, 159 insertions(+), 121 deletions(-) diff --git a/librfxcodec b/librfxcodec index ab9b65ce..015fdfff 160000 --- a/librfxcodec +++ b/librfxcodec @@ -1 +1 @@ -Subproject commit ab9b65cee1d96eefe154de721fb375f7a4d3946a +Subproject commit 015fdffff98584c47966aca617acdc0d1e6dbdca diff --git a/xrdp/xrdp_encoder.c b/xrdp/xrdp_encoder.c index 5717c8fa..0b44414c 100644 --- a/xrdp/xrdp_encoder.c +++ b/xrdp/xrdp_encoder.c @@ -28,11 +28,22 @@ #include "thread_calls.h" #include "fifo.h" #include "xrdp_egfx.h" +#include "string_calls.h" #ifdef XRDP_RFXCODEC #include "rfxcodec_encode.h" #endif +#define DEFAULT_XRDP_GFX_FRAMES_IN_FLIGHT 2 +/* limits used for validate env var XRDP_GFX_FRAMES_IN_FLIGHT */ +#define MIN_XRDP_GFX_FRAMES_IN_FLIGHT 1 +#define MAX_XRDP_GFX_FRAMES_IN_FLIGHT 16 + +#define DEFAULT_XRDP_GFX_MAX_COMPRESSED_BYTES (3 * 1024 * 1024) +/* limits used for validate env var XRDP_GFX_MAX_COMPRESSED_BYTES */ +#define MIN_XRDP_GFX_MAX_COMPRESSED_BYTES (64 * 1024) +#define MAX_XRDP_GFX_MAX_COMPRESSED_BYTES (256 * 1024 * 1024) + #define XRDP_SURCMD_PREFIX_BYTES 256 #define OUT_DATA_BYTES_DEFAULT_SIZE (16 * 1024 * 1024) @@ -85,40 +96,6 @@ xrdp_enc_data_done_destructor(void *item, void *closure) g_free(enc_done); } -/*****************************************************************************/ -static unsigned int -get_largest_monitor_pixels(struct xrdp_mm *mm) -{ - unsigned int max_pixels; - - struct xrdp_client_info *client_info = mm->wm->client_info; - struct display_size_description *display_sizes; - display_sizes = &client_info->display_sizes; - - if (display_sizes->monitorCount < 1) - { - max_pixels = display_sizes->session_width * - display_sizes->session_height; - } - else - { - max_pixels = 0; - struct monitor_info *minfo = display_sizes->minfo; - unsigned int i; - for (i = 0 ; i < display_sizes->monitorCount; ++i) - { - unsigned int pixels = (minfo[i].right + 1) - minfo[i].left; - pixels *= (minfo[i].bottom + 1) - minfo[i].top; - if (pixels > max_pixels) - { - max_pixels = pixels; - } - } - } - - return max_pixels; -} - /*****************************************************************************/ struct xrdp_encoder * xrdp_encoder_create(struct xrdp_mm *mm) @@ -223,10 +200,44 @@ xrdp_encoder_create(struct xrdp_mm *mm) self->xrdp_encoder_term = g_create_wait_obj(buf); if (client_info->gfx) { - // Assume compressor needs to cope with largest monitor with - // ineffective compression - self->frames_in_flight = 2; - self->max_compressed_bytes = get_largest_monitor_pixels(mm) * 4; + const char *env_var = g_getenv("XRDP_GFX_FRAMES_IN_FLIGHT"); + self->frames_in_flight = DEFAULT_XRDP_GFX_FRAMES_IN_FLIGHT; + if (env_var != NULL) + { + int fif = g_atoix(env_var); + if (fif >= MIN_XRDP_GFX_FRAMES_IN_FLIGHT && + fif <= MAX_XRDP_GFX_FRAMES_IN_FLIGHT) + { + self->frames_in_flight = fif; + LOG(LOG_LEVEL_INFO, "xrdp_encoder_create: " + "XRDP_GFX_FRAMES_IN_FLIGHT set to %d", fif); + } + else + { + LOG(LOG_LEVEL_INFO, "xrdp_encoder_create: " + "XRDP_GFX_FRAMES_IN_FLIGHT set but invalid %s", + env_var); + } + } + env_var = g_getenv("XRDP_GFX_MAX_COMPRESSED_BYTES"); + self->max_compressed_bytes = DEFAULT_XRDP_GFX_MAX_COMPRESSED_BYTES; + if (env_var != NULL) + { + int mcb = g_atoix(env_var); + if (mcb >= MIN_XRDP_GFX_MAX_COMPRESSED_BYTES && + mcb <= MAX_XRDP_GFX_MAX_COMPRESSED_BYTES) + { + self->max_compressed_bytes = mcb; + LOG(LOG_LEVEL_INFO, "xrdp_encoder_create: " + "XRDP_GFX_MAX_COMPRESSED_BYTES set to %d", mcb); + } + else + { + LOG(LOG_LEVEL_INFO, "xrdp_encoder_create: " + "XRDP_GFX_MAX_COMPRESSED_BYTES set but invalid %s", + env_var); + } + } LOG_DEVEL(LOG_LEVEL_INFO, "Using %d max_compressed_bytes for encoder", self->max_compressed_bytes); } @@ -563,16 +574,50 @@ process_enc_h264(struct xrdp_encoder *self, XRDP_ENC_DATA *enc) #ifdef XRDP_RFXCODEC +/*****************************************************************************/ +static int +gfx_send_done(struct xrdp_encoder *self, XRDP_ENC_DATA *enc, + int comp_bytes, int pad_bytes, char *comp_pad_data, + int got_frame_id, int frame_id, int is_last) + +{ + XRDP_ENC_DATA_DONE *enc_done; + + enc_done = g_new0(XRDP_ENC_DATA_DONE, 1); + if (enc_done == NULL) + { + return 1; + } + ENC_SET_BIT(enc_done->flags, ENC_DONE_FLAGS_GFX_BIT); + enc_done->enc = enc; + enc_done->last = is_last; + enc_done->pad_bytes = pad_bytes; + enc_done->comp_bytes = comp_bytes; + enc_done->comp_pad_data = comp_pad_data; + if (got_frame_id) + { + ENC_SET_BIT(enc_done->flags, ENC_DONE_FLAGS_FRAME_ID_BIT); + enc_done->frame_id = frame_id; + } + /* inform main thread done */ + tc_mutex_lock(self->mutex); + fifo_add_item(self->fifo_processed, enc_done); + tc_mutex_unlock(self->mutex); + /* signal completion for main thread */ + g_set_wait_obj(self->xrdp_encoder_event_processed); + return 0; +} + /*****************************************************************************/ static struct stream * gfx_wiretosurface1(struct xrdp_encoder *self, struct xrdp_egfx_bulk *bulk, struct stream *in_s, - struct xrdp_enc_gfx_cmd *enc_gfx_cmd) + XRDP_ENC_DATA *enc) { (void)self; (void)bulk; (void)in_s; - (void)enc_gfx_cmd; + (void)enc; return NULL; } @@ -580,7 +625,7 @@ gfx_wiretosurface1(struct xrdp_encoder *self, static struct stream * gfx_wiretosurface2(struct xrdp_encoder *self, struct xrdp_egfx_bulk *bulk, struct stream *in_s, - struct xrdp_enc_gfx_cmd *enc_gfx_cmd) + XRDP_ENC_DATA *enc) { int index; int surface_id; @@ -598,10 +643,10 @@ gfx_wiretosurface2(struct xrdp_encoder *self, int bitmap_data_length; struct rfx_tile *tiles; struct rfx_rect *rfxrects; + int tiles_compressed; int flags; + int total_tiles; int tiles_written; - int do_free; - int do_send; int mon_index; if (!s_check_rem(in_s, 15)) @@ -686,59 +731,72 @@ gfx_wiretosurface2(struct xrdp_encoder *self, RFX_FORMAT_YUV, RFX_FLAGS_RLGR1 | RFX_FLAGS_PRO1); if (self->codec_handle_prfx_gfx[mon_index] == NULL) - { - return NULL; - } - } - - do_free = 0; - do_send = 0; - if (ENC_IS_BIT_SET(flags, 0)) - { - /* already compressed */ - bitmap_data_length = enc_gfx_cmd->data_bytes; - bitmap_data = enc_gfx_cmd->data; - do_send = 1; - } - else - { - bitmap_data_length = self->max_compressed_bytes; - bitmap_data = g_new(char, bitmap_data_length); - if (bitmap_data == NULL) { g_free(tiles); g_free(rfxrects); return NULL; } - do_free = 1; - tiles_written = rfxcodec_encode(self->codec_handle_prfx_gfx[mon_index], - bitmap_data, - &bitmap_data_length, - enc_gfx_cmd->data, - width, height, - ((width + 63) & ~63) * 4, - rfxrects, num_rects_d, - tiles, num_rects_c, - self->quants, self->num_quants); - if (tiles_written > 0) - { - do_send = 1; - } } - g_free(tiles); - g_free(rfxrects); - rv = NULL; - if (do_send) + bitmap_data_length = self->max_compressed_bytes; + bitmap_data = g_new(char, bitmap_data_length); + if (bitmap_data == NULL) { + g_free(tiles); + g_free(rfxrects); + return NULL; + } + rv = NULL; + tiles_written = 0; + total_tiles = num_rects_c; + for (;;) + { + tiles_compressed = + rfxcodec_encode(self->codec_handle_prfx_gfx[mon_index], + bitmap_data, + &bitmap_data_length, + enc->u.gfx.data, + width, height, + ((width + 63) & ~63) * 4, + rfxrects, num_rects_d, + tiles + tiles_written, total_tiles - tiles_written, + self->quants, self->num_quants); + if (tiles_compressed < 1) + { + break; + } + tiles_written += tiles_compressed; rv = xrdp_egfx_wire_to_surface2(bulk, surface_id, codec_id, codec_context_id, pixel_format, bitmap_data, bitmap_data_length); + if (rv == NULL) + { + break; + } + LOG_DEVEL(LOG_LEVEL_ERROR, "gfx_wiretosurface2: " + "tiles_compressed %d total_tiles %d tiles_written %d", + tiles_compressed, total_tiles, + tiles_written); + if (tiles_written >= total_tiles) + { + /* ok, done with last tile set */ + break; + } + /* we have another tile set, send this one to main thread */ + if (gfx_send_done(self, enc, (int)(rv->end - rv->data), 0, + rv->data, 0, 0, 0) != 0) + { + free_stream(rv); + rv = NULL; + break; + } + g_free(rv); /* don't call free_stream() here so s->data is valid */ + rv = NULL; + bitmap_data_length = self->max_compressed_bytes; } - if (do_free) - { - g_free(bitmap_data); - } + g_free(tiles); + g_free(rfxrects); + g_free(bitmap_data); return rv; } @@ -939,20 +997,14 @@ process_enc_egfx(struct xrdp_encoder *self, XRDP_ENC_DATA *enc) struct stream *s; struct stream in_s; struct xrdp_egfx_bulk *bulk; - XRDP_ENC_DATA_DONE *enc_done; - struct fifo *fifo_processed; - tbus mutex; - tbus event_processed; int cmd_id; int cmd_bytes; int frame_id; int got_frame_id; + int error; char *holdp; char *holdend; - fifo_processed = self->fifo_processed; - mutex = self->mutex; - event_processed = self->xrdp_encoder_event_processed; bulk = self->mm->egfx->bulk; g_memset(&in_s, 0, sizeof(in_s)); in_s.data = enc->u.gfx.cmd; @@ -977,10 +1029,10 @@ process_enc_egfx(struct xrdp_encoder *self, XRDP_ENC_DATA *enc) switch (cmd_id) { case XR_RDPGFX_CMDID_WIRETOSURFACE_1: /* 0x0001 */ - s = gfx_wiretosurface1(self, bulk, &in_s, &(enc->u.gfx)); + s = gfx_wiretosurface1(self, bulk, &in_s, enc); break; case XR_RDPGFX_CMDID_WIRETOSURFACE_2: /* 0x0002 */ - s = gfx_wiretosurface2(self, bulk, &in_s, &(enc->u.gfx)); + s = gfx_wiretosurface2(self, bulk, &in_s, enc); break; case XR_RDPGFX_CMDID_SOLIDFILL: /* 0x0004 */ s = gfx_solidfill(self, bulk, &in_s); @@ -1010,38 +1062,24 @@ process_enc_egfx(struct xrdp_encoder *self, XRDP_ENC_DATA *enc) default: break; } - if (s == NULL) - { - LOG(LOG_LEVEL_ERROR, "process_enc_egfx: cmd_id %d s = nil", cmd_id); - return 1; - } /* setup for next cmd */ in_s.p = holdp + cmd_bytes; in_s.end = holdend; - /* setup enc_done struct */ - enc_done = g_new0(XRDP_ENC_DATA_DONE, 1); - if (enc_done == NULL) + if (s != NULL) { - free_stream(s); - return 1; + /* send message to main thread */ + error = gfx_send_done(self, enc, (int) (s->end - s->data), + 0, s->data, got_frame_id, frame_id, + !s_check_rem(&in_s, 8)); + if (error != 0) + { + LOG(LOG_LEVEL_ERROR, "process_enc_egfx: gfx_send_done failed " + "error %d", error); + free_stream(s); + return 1; + } + g_free(s); /* don't call free_stream() here so s->data is valid */ } - ENC_SET_BIT(enc_done->flags, ENC_DONE_FLAGS_GFX_BIT); - enc_done->enc = enc; - enc_done->last = !s_check_rem(&in_s, 8); - enc_done->comp_bytes = (int) (s->end - s->data); - enc_done->comp_pad_data = s->data; - if (got_frame_id) - { - ENC_SET_BIT(enc_done->flags, ENC_DONE_FLAGS_FRAME_ID_BIT); - enc_done->frame_id = frame_id; - } - g_free(s); /* don't call free_stream() here so s->data is valid */ - /* inform main thread done */ - tc_mutex_lock(mutex); - fifo_add_item(fifo_processed, enc_done); - tc_mutex_unlock(mutex); - /* signal completion for main thread */ - g_set_wait_obj(event_processed); } return 0; } From 898e1ca135922172698a70b0a267d67aa1958c10 Mon Sep 17 00:00:00 2001 From: Jay Sorg Date: Fri, 22 Mar 2024 14:53:38 -0700 Subject: [PATCH 2/2] format change --- xrdp/xrdp_encoder.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xrdp/xrdp_encoder.c b/xrdp/xrdp_encoder.c index 0b44414c..b7a51e5e 100644 --- a/xrdp/xrdp_encoder.c +++ b/xrdp/xrdp_encoder.c @@ -206,7 +206,7 @@ xrdp_encoder_create(struct xrdp_mm *mm) { int fif = g_atoix(env_var); if (fif >= MIN_XRDP_GFX_FRAMES_IN_FLIGHT && - fif <= MAX_XRDP_GFX_FRAMES_IN_FLIGHT) + fif <= MAX_XRDP_GFX_FRAMES_IN_FLIGHT) { self->frames_in_flight = fif; LOG(LOG_LEVEL_INFO, "xrdp_encoder_create: " @@ -225,7 +225,7 @@ xrdp_encoder_create(struct xrdp_mm *mm) { int mcb = g_atoix(env_var); if (mcb >= MIN_XRDP_GFX_MAX_COMPRESSED_BYTES && - mcb <= MAX_XRDP_GFX_MAX_COMPRESSED_BYTES) + mcb <= MAX_XRDP_GFX_MAX_COMPRESSED_BYTES) { self->max_compressed_bytes = mcb; LOG(LOG_LEVEL_INFO, "xrdp_encoder_create: "