From 427f501862575debbff93b5ea15aed9ff541283f Mon Sep 17 00:00:00 2001 From: Koichiro Iwao Date: Mon, 19 Aug 2024 11:37:18 +0900 Subject: [PATCH 01/12] tconfig: add new toml config parser for gfx.toml (cherry picked from commit b3513ba8df5e423a1f282055d4436db0a18559b3) --- tests/xrdp/test_tconfig.c | 50 ++++++++ tests/xrdp/test_xrdp.h | 1 + tests/xrdp/test_xrdp_main.c | 1 + xrdp/gfx.toml | 27 ++++ xrdp/xrdp_encoder_x264.c | 35 ++++-- xrdp/xrdp_tconfig.c | 244 ++++++++++++++++++++++++++++++++++++ xrdp/xrdp_tconfig.h | 66 ++++++++++ 7 files changed, 412 insertions(+), 12 deletions(-) create mode 100644 tests/xrdp/test_tconfig.c create mode 100644 xrdp/gfx.toml create mode 100644 xrdp/xrdp_tconfig.c create mode 100644 xrdp/xrdp_tconfig.h diff --git a/tests/xrdp/test_tconfig.c b/tests/xrdp/test_tconfig.c new file mode 100644 index 00000000..586278eb --- /dev/null +++ b/tests/xrdp/test_tconfig.c @@ -0,0 +1,50 @@ +#if defined(HAVE_CONFIG_H) +#include "config_ac.h" +#endif + +#include "xrdp_tconfig.h" +#include "test_xrdp.h" +#include "xrdp.h" + +START_TEST(test_tconfig_gfx_always_success) +{ + ck_assert_int_eq(1, 1); +} +END_TEST + +START_TEST(test_tconfig_gfx_x264_load_basic) +{ + struct xrdp_tconfig_gfx gfxconfig; + int rv = tconfig_load_gfx(XRDP_TOP_SRCDIR "/xrdp/gfx.toml", &gfxconfig); + + ck_assert_int_eq(rv, 0); + + /* default */ + ck_assert_str_eq(gfxconfig.x264_param[0].preset, "ultrafast"); + ck_assert_str_eq(gfxconfig.x264_param[0].tune, "zerolatency"); + ck_assert_str_eq(gfxconfig.x264_param[0].profile, "main"); + ck_assert_int_eq(gfxconfig.x264_param[0].vbv_max_bitrate, 0); + ck_assert_int_eq(gfxconfig.x264_param[0].vbv_buffer_size, 0); + ck_assert_int_eq(gfxconfig.x264_param[0].fps_num, 24); + ck_assert_int_eq(gfxconfig.x264_param[0].fps_den, 1); + +} +END_TEST + +/******************************************************************************/ +Suite * +make_suite_tconfig_load_gfx(void) +{ + Suite *s; + TCase *tc_tconfig_load_gfx; + + s = suite_create("GfxLoad"); + + tc_tconfig_load_gfx = tcase_create("xrdp_tconfig_load_gfx"); + tcase_add_test(tc_tconfig_load_gfx, test_tconfig_gfx_always_success); + tcase_add_test(tc_tconfig_load_gfx, test_tconfig_gfx_x264_load_basic); + + suite_add_tcase(s, tc_tconfig_load_gfx); + + return s; +} diff --git a/tests/xrdp/test_xrdp.h b/tests/xrdp/test_xrdp.h index 58d33578..d2da0b29 100644 --- a/tests/xrdp/test_xrdp.h +++ b/tests/xrdp/test_xrdp.h @@ -6,5 +6,6 @@ Suite *make_suite_test_bitmap_load(void); Suite *make_suite_egfx_base_functions(void); Suite *make_suite_region(void); +Suite *make_suite_tconfig_load_gfx(void); #endif /* TEST_XRDP_H */ diff --git a/tests/xrdp/test_xrdp_main.c b/tests/xrdp/test_xrdp_main.c index 017ef102..05c9f9fd 100644 --- a/tests/xrdp/test_xrdp_main.c +++ b/tests/xrdp/test_xrdp_main.c @@ -57,6 +57,7 @@ int main (void) sr = srunner_create (make_suite_test_bitmap_load()); srunner_add_suite(sr, make_suite_egfx_base_functions()); srunner_add_suite(sr, make_suite_region()); + srunner_add_suite(sr, make_suite_tconfig_load_gfx()); srunner_set_tap(sr, "-"); srunner_run_all (sr, CK_ENV); diff --git a/xrdp/gfx.toml b/xrdp/gfx.toml new file mode 100644 index 00000000..26bcff90 --- /dev/null +++ b/xrdp/gfx.toml @@ -0,0 +1,27 @@ + +[x264.default] +preset = "ultrafast" +tune = "zerolatency" +profile = "main" +vbv_max_bitrate = 0 +vbv_buffer_size = 0 +fps_num = 24 +fps_den = 1 + + +[x264.lan] +[x264.wan] +[x264.broadband_high] +[x264.satellite] + +[x264.broadband_low] +preset = "veryfast" +tune = "zerolatency" +vbv_max_bitrate = 1800 +vbv_buffer_size = 40 + +[x264.modem] +preset = "fast" +tune = "zerolatency" +vbv_max_bitrate = 500 +vbv_buffer_size = 40 diff --git a/xrdp/xrdp_encoder_x264.c b/xrdp/xrdp_encoder_x264.c index 81fd7179..d8ff5c9b 100644 --- a/xrdp/xrdp_encoder_x264.c +++ b/xrdp/xrdp_encoder_x264.c @@ -32,6 +32,7 @@ #include "arch.h" #include "os_calls.h" #include "xrdp_encoder_x264.h" +#include "xrdp_tconfig.h" #define X264_MAX_ENCODERS 16 @@ -47,6 +48,7 @@ struct x264_encoder struct x264_global { struct x264_encoder encoders[X264_MAX_ENCODERS]; + struct xrdp_tconfig_gfx_x264_param x264_param[NUM_CONNECTION_TYPES]; }; /*****************************************************************************/ @@ -54,7 +56,17 @@ void * xrdp_encoder_x264_create(void) { LOG_DEVEL(LOG_LEVEL_TRACE, "xrdp_encoder_x264_create:"); - return g_new0(struct x264_global, 1); + + struct x264_global *xg; + struct xrdp_tconfig_gfx gfxconfig; + xg = g_new0(struct x264_global, 1); + tconfig_load_gfx(GFX_CONF, &gfxconfig); + + memcpy(&xg->x264_param, &gfxconfig.x264_param, + sizeof(struct xrdp_tconfig_gfx_x264_param) * NUM_CONNECTION_TYPES); + + return xg; + } /*****************************************************************************/ @@ -128,20 +140,19 @@ xrdp_encoder_x264_encode(void *handle, int session, int left, int top, } if ((width > 0) && (height > 0)) { - //x264_param_default_preset(&(xe->x264_params), "superfast", "zerolatency"); - x264_param_default_preset(&(xe->x264_params), "ultrafast", "zerolatency"); + x264_param_default_preset(&(xe->x264_params), + xg->x264_param[ct].preset, + xg->x264_param[ct].tune); xe->x264_params.i_threads = 1; xe->x264_params.i_width = (width + 15) & ~15; xe->x264_params.i_height = (height + 15) & ~15; - xe->x264_params.i_fps_num = 24; - xe->x264_params.i_fps_den = 1; - //xe->x264_params.b_cabac = 1; - //xe->x264_params.i_bframe = 0; - //xe->x264_params.rc.i_rc_method = X264_RC_CQP; - //xe->x264_params.rc.i_qp_constant = 23; - //x264_param_apply_profile(&(xe->x264_params), "high"); - x264_param_apply_profile(&(xe->x264_params), "main"); - //x264_param_apply_profile(&(xe->x264_params), "baseline"); + xe->x264_params.i_fps_num = xg->x264_param[ct].fps_num; + xe->x264_params.i_fps_den = xg->x264_param[ct].fps_den; + xe->x264_params.rc.i_rc_method = X264_RC_CRF; + xe->x264_params.rc.i_vbv_max_bitrate = xg->x264_param[ct].vbv_max_bitrate; + xe->x264_params.rc.i_vbv_buffer_size = xg->x264_param[ct].vbv_buffer_size; + x264_param_apply_profile(&(xe->x264_params), + xg->x264_param[ct].profile); xe->x264_enc_han = x264_encoder_open(&(xe->x264_params)); LOG(LOG_LEVEL_INFO, "xrdp_encoder_x264_encode: " "x264_encoder_open rv %p for width %d height %d", diff --git a/xrdp/xrdp_tconfig.c b/xrdp/xrdp_tconfig.c new file mode 100644 index 00000000..d16a0f4a --- /dev/null +++ b/xrdp/xrdp_tconfig.c @@ -0,0 +1,244 @@ +/** + * xrdp: A Remote Desktop Protocol server. + * + * Copyright (C) Koichiro Iwao + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * + * @file xrdp_tconfig.c + * @brief TOML config loader + * @author Koichiro Iwao + * + */ + +#if defined(HAVE_CONFIG_H) +#include +#endif + +#include +#include +#include + +#include "arch.h" +#include "os_calls.h" +#include "parse.h" +#include "toml.h" +#include "ms-rdpbcgr.h" +#include "xrdp_tconfig.h" +#include "string_calls.h" + +#define TCLOG(log_level, args...) LOG(log_level, "TConfig: " args) + +#define X264_DEFAULT_PRESET "ultrafast" +#define X264_DEFAULT_TUNE "zerolatency" +#define X264_DEFAULT_PROFILE "main" +#define X264_DEFAULT_FPS_NUM 24 +#define X264_DEFAULT_FPS_DEN 1 + +static int +tconfig_load_gfx_x264_ct(toml_table_t *tfile, const int connection_type, + struct xrdp_tconfig_gfx_x264_param *param) +{ + if (connection_type > NUM_CONNECTION_TYPES) + { + TCLOG(LOG_LEVEL_ERROR, "Invalid connection type is given"); + return 1; + } + + toml_table_t *x264 = toml_table_in(tfile, "x264"); + if (!x264) + { + TCLOG(LOG_LEVEL_WARNING, "x264 params are not defined"); + return 1; + } + + toml_table_t *x264_ct = + toml_table_in(x264, rdpbcgr_connection_type_names[connection_type]); + toml_datum_t datum; + + if (!x264_ct) + { + TCLOG(LOG_LEVEL_WARNING, "x264 params for connection type [%s] is not defined", + rdpbcgr_connection_type_names[connection_type]); + return 1; + } + + /* preset */ + datum = toml_string_in(x264_ct, "preset"); + if (datum.ok) + { + g_strncpy(param[connection_type].preset, + datum.u.s, + sizeof(param[connection_type].preset) - 1); + free(datum.u.s); + } + else if (connection_type == 0) + { + TCLOG(LOG_LEVEL_WARNING, + "x264 param preset is not set for connection type [%s], " + "adopting the default value \"" X264_DEFAULT_PRESET "\"", + rdpbcgr_connection_type_names[connection_type]); + g_strncpy(param[connection_type].preset, + X264_DEFAULT_PRESET, + sizeof(param[connection_type].preset) - 1); + } + + /* tune */ + datum = toml_string_in(x264_ct, "tune"); + if (datum.ok) + { + g_strncpy(param[connection_type].tune, + datum.u.s, + sizeof(param[connection_type].tune) - 1); + free(datum.u.s); + } + else if (connection_type == 0) + { + TCLOG(LOG_LEVEL_WARNING, + "x264 param tune is not set for connection type [%s], " + "adopting the default value \"" X264_DEFAULT_TUNE "\"", + rdpbcgr_connection_type_names[connection_type]); + g_strncpy(param[connection_type].tune, + X264_DEFAULT_TUNE, + sizeof(param[connection_type].tune) - 1); + } + + /* profile */ + datum = toml_string_in(x264_ct, "profile"); + if (datum.ok) + { + g_strncpy(param[connection_type].profile, + datum.u.s, + sizeof(param[connection_type].profile) - 1); + free(datum.u.s); + } + else if (connection_type == 0) + { + TCLOG(LOG_LEVEL_WARNING, + "x264 param profile is not set for connection type [%s], " + "adopting the default value \"" X264_DEFAULT_PROFILE "\"", + rdpbcgr_connection_type_names[connection_type]); + g_strncpy(param[connection_type].profile, + X264_DEFAULT_PROFILE, + sizeof(param[connection_type].profile) - 1); + } + + /* vbv_max_bitrate */ + datum = toml_int_in(x264_ct, "vbv_max_bitrate"); + if (datum.ok) + { + param[connection_type].vbv_max_bitrate = datum.u.i; + } + else if (connection_type == 0) + { + TCLOG(LOG_LEVEL_WARNING, + "x264 param vbv_max_bit_rate is not set for connection type [%s], " + "adopting the default value [0]", + rdpbcgr_connection_type_names[connection_type]); + param[connection_type].vbv_max_bitrate = 0; + } + + /* vbv_buffer_size */ + datum = toml_int_in(x264_ct, "vbv_buffer_size"); + if (datum.ok) + { + param[connection_type].vbv_buffer_size = datum.u.i; + } + else if (connection_type == 0) + { + TCLOG(LOG_LEVEL_WARNING, + "x264 param vbv_buffer_size is not set for connection type [%s], " + "adopting the default value [0]", + rdpbcgr_connection_type_names[connection_type]); + param[connection_type].vbv_buffer_size = 0; + } + + /* fps_num */ + datum = toml_int_in(x264_ct, "fps_num"); + if (datum.ok) + { + param[connection_type].fps_num = datum.u.i; + } + else if (connection_type == 0) + { + TCLOG(LOG_LEVEL_WARNING, + "x264 param fps_num is not set for connection type [%s], " + "adopting the default value [0]", + rdpbcgr_connection_type_names[connection_type]); + param[connection_type].fps_num = X264_DEFAULT_FPS_NUM; + } + + /* fps_den */ + datum = toml_int_in(x264_ct, "fps_den"); + if (datum.ok) + { + param[connection_type].fps_den = datum.u.i; + } + else if (connection_type == 0) + { + TCLOG(LOG_LEVEL_WARNING, + "x264 param fps_den is not set for connection type [%s], " + "adopting the default value [0]", + rdpbcgr_connection_type_names[connection_type]); + param[connection_type].fps_num = X264_DEFAULT_FPS_DEN; + } + + return 0; +} + +int +tconfig_load_gfx(const char *filename, struct xrdp_tconfig_gfx *config) +{ + FILE *fp; + char errbuf[200]; + toml_table_t *tfile; + + if ((fp = fopen(filename, "r")) == NULL) + { + TCLOG(LOG_LEVEL_ERROR, "Error loading GFX config file %s (%s)", + filename, g_get_strerror()); + return 1; + } + else if ((tfile = toml_parse_file(fp, errbuf, sizeof(errbuf))) == NULL) + { + TCLOG(LOG_LEVEL_ERROR, "Error in GFX config file %s - %s", filename, errbuf); + fclose(fp); + return 1; + } + else + { + TCLOG(LOG_LEVEL_INFO, "Loading GFX config file %s", filename); + fclose(fp); + + memset(config, 0, sizeof(struct xrdp_tconfig_gfx)); + + /* First of all, read the default params and override later */ + tconfig_load_gfx_x264_ct(tfile, 0, config->x264_param); + + for (int ct = CONNECTION_TYPE_MODEM; ct < NUM_CONNECTION_TYPES; ct++) + { + memcpy(&config->x264_param[ct], &config->x264_param[0], + sizeof(struct xrdp_tconfig_gfx_x264_param)); + + tconfig_load_gfx_x264_ct(tfile, ct, config->x264_param); + } + + toml_free(tfile); + + return 0; + } +} + diff --git a/xrdp/xrdp_tconfig.h b/xrdp/xrdp_tconfig.h new file mode 100644 index 00000000..6a5ba970 --- /dev/null +++ b/xrdp/xrdp_tconfig.h @@ -0,0 +1,66 @@ +/** + * xrdp: A Remote Desktop Protocol server. + * + * Copyright (C) Koichiro Iwao + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * + * @file xrdp_tconfig.c + * @brief TOML config loader and structures + * @author Koichiro Iwao + * + */ +#ifndef _XRDP_TCONFIG_H_ +#define _XRDP_TCONFIG_H_ + +/* The number of connection types in MS-RDPBCGR 2.2.1.3.2 */ +#define NUM_CONNECTION_TYPES 7 +#define GFX_CONF XRDP_CFG_PATH "/gfx.toml" + +/* nc stands for new config */ +struct xrdp_tconfig_gfx_x264_param +{ + char preset[16]; + char tune[16]; + char profile[16]; + int vbv_max_bitrate; + int vbv_buffer_size; + int fps_num; + int fps_den; +}; + +struct xrdp_tconfig_gfx +{ + /* store x264 parameters for each connection type */ + struct xrdp_tconfig_gfx_x264_param x264_param[NUM_CONNECTION_TYPES]; +}; + +static const char *const rdpbcgr_connection_type_names[] = +{ + "default", /* for xrdp internal use */ + "modem", + "broadband_low", + "satellite", + "broadband_high", + "wan", + "lan", + "autodetect", + 0 +}; + +int tconfig_load_gfx(const char *filename, struct xrdp_tconfig_gfx *config); + +#endif From d0839208249386de921ae572a2320ecf0c54f238 Mon Sep 17 00:00:00 2001 From: Koichiro Iwao Date: Mon, 19 Aug 2024 11:38:16 +0900 Subject: [PATCH 02/12] tconfig: Makefile changes (cherry picked from commit d50c2fd4e47c51409d4f52ccfc3887eae22691b6) --- tests/xrdp/Makefile.am | 11 +++++++++++ xrdp/Makefile.am | 3 +++ 2 files changed, 14 insertions(+) diff --git a/tests/xrdp/Makefile.am b/tests/xrdp/Makefile.am index 014cf36d..d7203c18 100644 --- a/tests/xrdp/Makefile.am +++ b/tests/xrdp/Makefile.am @@ -1,4 +1,5 @@ AM_CPPFLAGS = \ + -DXRDP_TOP_SRCDIR=\"$(top_srcdir)\" \ -I$(top_builddir) \ -I$(top_srcdir)/xrdp \ -I$(top_srcdir)/libxrdp \ @@ -28,6 +29,7 @@ test_xrdp_SOURCES = \ test_xrdp_main.c \ test_xrdp_egfx.c \ test_xrdp_region.c \ + test_tconfig.c \ test_bitmap_load.c test_xrdp_CFLAGS = \ @@ -47,6 +49,7 @@ test_xrdp_LDADD = \ $(top_builddir)/libxrdp/libxrdp.la \ $(top_builddir)/libpainter/src/libpainter.la \ $(top_builddir)/librfxcodec/src/librfxencode.la \ + $(top_builddir)/third_party/tomlc99/libtoml.la \ $(top_builddir)/xrdp/lang.o \ $(top_builddir)/xrdp/xrdp_mm.o \ $(top_builddir)/xrdp/xrdp_wm.o \ @@ -60,8 +63,16 @@ test_xrdp_LDADD = \ $(top_builddir)/xrdp/xrdp_encoder.o \ $(top_builddir)/xrdp/xrdp_process.o \ $(top_builddir)/xrdp/xrdp_login_wnd.o \ + $(top_builddir)/xrdp/xrdp_tconfig.o \ $(top_builddir)/xrdp/xrdp_main_utils.o \ $(PIXMAN_LIBS) \ $(IMLIB2_LIBS) \ @CHECK_LIBS@ \ @CMOCKA_LIBS@ + +if XRDP_X264 +AM_CPPFLAGS += -DXRDP_X264 $(XRDP_X264_CFLAGS) +test_xrdp_LDADD += \ + $(top_builddir)/xrdp/xrdp_encoder_x264.o \ + $(XRDP_X264_LIBS) +endif diff --git a/xrdp/Makefile.am b/xrdp/Makefile.am index 4244cbc1..9ac8bf1e 100644 --- a/xrdp/Makefile.am +++ b/xrdp/Makefile.am @@ -73,6 +73,8 @@ xrdp_SOURCES = \ xrdp_egfx.h \ xrdp_wm.c \ xrdp_main_utils.c \ + xrdp_tconfig.c \ + xrdp_tconfig.h \ $(XRDP_EXTRA_SOURCES) xrdp_LDADD = \ @@ -103,6 +105,7 @@ SUFFIXES = .in $(subst_verbose)$(SUBST_VARS) $< > $@ dist_xrdpsysconf_DATA = \ + gfx.toml \ xrdp_keyboard.ini nodist_xrdpsysconf_DATA = \ From 12c9e79dee3f31221ca102bea9a40d26fd9d4044 Mon Sep 17 00:00:00 2001 From: Koichiro Iwao Date: Mon, 19 Aug 2024 11:30:16 +0900 Subject: [PATCH 03/12] x264: apply encoding parameters per connection type (cherry picked from commit 010b6a3dbf25754679cc7b6ce4bd46173570f942) --- xrdp/xrdp_encoder.c | 6 +++++- xrdp/xrdp_encoder_x264.c | 12 +++++++++++- xrdp/xrdp_encoder_x264.h | 3 ++- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/xrdp/xrdp_encoder.c b/xrdp/xrdp_encoder.c index eaa52204..f6eb36e8 100644 --- a/xrdp/xrdp_encoder.c +++ b/xrdp/xrdp_encoder.c @@ -759,6 +759,9 @@ gfx_wiretosurface1(struct xrdp_encoder *self, short *crects; struct xrdp_enc_gfx_cmd *enc_gfx_cmd = &(enc->u.gfx); int mon_index; + int connection_type; + + connection_type = self->mm->wm->client_info->mcs_connection_type; s = &ls; g_memset(s, 0, sizeof(struct stream)); @@ -912,7 +915,8 @@ gfx_wiretosurface1(struct xrdp_encoder *self, width, height, twidth, theight, 0, enc_gfx_cmd->data, crects, num_rects_c, - s->p, &bitmap_data_length, NULL); + s->p, &bitmap_data_length, + connection_type, NULL); if (error == 0) { xstream_seek(s, bitmap_data_length); diff --git a/xrdp/xrdp_encoder_x264.c b/xrdp/xrdp_encoder_x264.c index d8ff5c9b..29f1dcc9 100644 --- a/xrdp/xrdp_encoder_x264.c +++ b/xrdp/xrdp_encoder_x264.c @@ -101,7 +101,8 @@ xrdp_encoder_x264_encode(void *handle, int session, int left, int top, int width, int height, int twidth, int theight, int format, const char *data, short *crects, int num_crects, - char *cdata, int *cdata_bytes, int *flags_ptr) + char *cdata, int *cdata_bytes, int connection_type, + int *flags_ptr) { struct x264_global *xg; struct x264_encoder *xe; @@ -117,6 +118,7 @@ xrdp_encoder_x264_encode(void *handle, int session, int left, int top, int y; int cx; int cy; + int ct; /* connection_type */ x264_picture_t pic_in; x264_picture_t pic_out; @@ -125,6 +127,14 @@ xrdp_encoder_x264_encode(void *handle, int session, int left, int top, flags = 0; xg = (struct x264_global *) handle; xe = &(xg->encoders[session % X264_MAX_ENCODERS]); + + /* validate connection type */ + ct = connection_type; + if (ct > CONNECTION_TYPE_LAN || ct < CONNECTION_TYPE_MODEM) + { + ct = CONNECTION_TYPE_LAN; + } + if ((xe->x264_enc_han == NULL) || (xe->width != width) || (xe->height != height)) { diff --git a/xrdp/xrdp_encoder_x264.h b/xrdp/xrdp_encoder_x264.h index 0d7ec5ba..373ef5c5 100644 --- a/xrdp/xrdp_encoder_x264.h +++ b/xrdp/xrdp_encoder_x264.h @@ -32,7 +32,8 @@ xrdp_encoder_x264_encode(void *handle, int session, int left, int top, int width, int height, int twidth, int theight, int format, const char *data, short *crects, int num_crects, - char *cdata, int *cdata_bytes, int *flags_ptr); + char *cdata, int *cdata_bytes, int connection_type, + int *flags_ptr); #endif From 1fcde206829b5812d66ff54dd5bfcb840e1353ce Mon Sep 17 00:00:00 2001 From: Koichiro Iwao Date: Mon, 19 Aug 2024 15:16:32 +0900 Subject: [PATCH 04/12] x264: Update x264 encoding parameters (cherry picked from commit 6aeb364c8d03f8cd4ba2092308efb2ea4329ac63) --- xrdp/gfx.toml | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/xrdp/gfx.toml b/xrdp/gfx.toml index 26bcff90..5b7f27c4 100644 --- a/xrdp/gfx.toml +++ b/xrdp/gfx.toml @@ -1,27 +1,37 @@ - [x264.default] preset = "ultrafast" tune = "zerolatency" -profile = "main" +profile = "main" # profile is forced to baseline if preset == ultrafast vbv_max_bitrate = 0 vbv_buffer_size = 0 fps_num = 24 fps_den = 1 - [x264.lan] +# inherits default + [x264.wan] +vbv_max_bitrate = 15000 +vbv_buffer_size = 1500 + [x264.broadband_high] +preset = "superfast" +vbv_max_bitrate = 8000 +vbv_buffer_Size = 800 + [x264.satellite] +preset = "superfast" +vbv_max_bitrate = 5000 +vbv_buffer_size = 500 [x264.broadband_low] preset = "veryfast" tune = "zerolatency" -vbv_max_bitrate = 1800 -vbv_buffer_size = 40 +vbv_max_bitrate = 1600 +vbv_buffer_size = 66 [x264.modem] preset = "fast" tune = "zerolatency" -vbv_max_bitrate = 500 -vbv_buffer_size = 40 +vbv_max_bitrate = 1200 +vbv_buffer_size = 50 From 6b98637bacb718266afb1f7e320beed2c5e107cd Mon Sep 17 00:00:00 2001 From: Koichiro Iwao Date: Sat, 24 Aug 2024 00:07:11 +0900 Subject: [PATCH 05/12] tconfig: set proper default value for fps_den (cherry picked from commit 16ef3dc3a8bb390b70d242f11f44e17c2d298c40) --- xrdp/xrdp_tconfig.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xrdp/xrdp_tconfig.c b/xrdp/xrdp_tconfig.c index d16a0f4a..82b9c895 100644 --- a/xrdp/xrdp_tconfig.c +++ b/xrdp/xrdp_tconfig.c @@ -193,7 +193,7 @@ tconfig_load_gfx_x264_ct(toml_table_t *tfile, const int connection_type, "x264 param fps_den is not set for connection type [%s], " "adopting the default value [0]", rdpbcgr_connection_type_names[connection_type]); - param[connection_type].fps_num = X264_DEFAULT_FPS_DEN; + param[connection_type].fps_den = X264_DEFAULT_FPS_DEN; } return 0; From abb5714f8f84af9eff7af75bf263c62f01b24eb5 Mon Sep 17 00:00:00 2001 From: Koichiro Iwao Date: Sat, 24 Aug 2024 00:09:46 +0900 Subject: [PATCH 06/12] tconfig: refine logging (cherry picked from commit b9d593bc11a47ab48d84f20e0fec5c6bb808f5f5) --- xrdp/xrdp_tconfig.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/xrdp/xrdp_tconfig.c b/xrdp/xrdp_tconfig.c index 82b9c895..663f9bce 100644 --- a/xrdp/xrdp_tconfig.c +++ b/xrdp/xrdp_tconfig.c @@ -52,16 +52,18 @@ static int tconfig_load_gfx_x264_ct(toml_table_t *tfile, const int connection_type, struct xrdp_tconfig_gfx_x264_param *param) { + TCLOG(LOG_LEVEL_TRACE, "[x264]"); + if (connection_type > NUM_CONNECTION_TYPES) { - TCLOG(LOG_LEVEL_ERROR, "Invalid connection type is given"); + TCLOG(LOG_LEVEL_ERROR, "[x264] Invalid connection type is given"); return 1; } toml_table_t *x264 = toml_table_in(tfile, "x264"); if (!x264) { - TCLOG(LOG_LEVEL_WARNING, "x264 params are not defined"); + TCLOG(LOG_LEVEL_WARNING, "[x264] x264 params are not defined"); return 1; } @@ -88,8 +90,8 @@ tconfig_load_gfx_x264_ct(toml_table_t *tfile, const int connection_type, else if (connection_type == 0) { TCLOG(LOG_LEVEL_WARNING, - "x264 param preset is not set for connection type [%s], " - "adopting the default value \"" X264_DEFAULT_PRESET "\"", + "[x264.%s] preset is not set, adopting the default value \"" + X264_DEFAULT_PRESET "\"", rdpbcgr_connection_type_names[connection_type]); g_strncpy(param[connection_type].preset, X264_DEFAULT_PRESET, @@ -108,8 +110,8 @@ tconfig_load_gfx_x264_ct(toml_table_t *tfile, const int connection_type, else if (connection_type == 0) { TCLOG(LOG_LEVEL_WARNING, - "x264 param tune is not set for connection type [%s], " - "adopting the default value \"" X264_DEFAULT_TUNE "\"", + "[x264.%s] tune is not set, adopting the default value \"" + X264_DEFAULT_TUNE"\"", rdpbcgr_connection_type_names[connection_type]); g_strncpy(param[connection_type].tune, X264_DEFAULT_TUNE, @@ -128,8 +130,8 @@ tconfig_load_gfx_x264_ct(toml_table_t *tfile, const int connection_type, else if (connection_type == 0) { TCLOG(LOG_LEVEL_WARNING, - "x264 param profile is not set for connection type [%s], " - "adopting the default value \"" X264_DEFAULT_PROFILE "\"", + "[x264.%s] profile is not set, adopting the default value \"" + X264_DEFAULT_PROFILE"\"", rdpbcgr_connection_type_names[connection_type]); g_strncpy(param[connection_type].profile, X264_DEFAULT_PROFILE, @@ -145,8 +147,7 @@ tconfig_load_gfx_x264_ct(toml_table_t *tfile, const int connection_type, else if (connection_type == 0) { TCLOG(LOG_LEVEL_WARNING, - "x264 param vbv_max_bit_rate is not set for connection type [%s], " - "adopting the default value [0]", + "[x264.%s] vbv_max_bitrate is not set, adopting the default value [0]", rdpbcgr_connection_type_names[connection_type]); param[connection_type].vbv_max_bitrate = 0; } @@ -160,8 +161,7 @@ tconfig_load_gfx_x264_ct(toml_table_t *tfile, const int connection_type, else if (connection_type == 0) { TCLOG(LOG_LEVEL_WARNING, - "x264 param vbv_buffer_size is not set for connection type [%s], " - "adopting the default value [0]", + "[x264.%s] vbv_buffer_size is not set, adopting the default value [0]", rdpbcgr_connection_type_names[connection_type]); param[connection_type].vbv_buffer_size = 0; } @@ -175,9 +175,9 @@ tconfig_load_gfx_x264_ct(toml_table_t *tfile, const int connection_type, else if (connection_type == 0) { TCLOG(LOG_LEVEL_WARNING, - "x264 param fps_num is not set for connection type [%s], " - "adopting the default value [0]", - rdpbcgr_connection_type_names[connection_type]); + "[x264.%s] fps_num is not set, adopting the default value [%d]", + rdpbcgr_connection_type_names[connection_type], + X264_DEFAULT_FPS_NUM); param[connection_type].fps_num = X264_DEFAULT_FPS_NUM; } @@ -190,9 +190,9 @@ tconfig_load_gfx_x264_ct(toml_table_t *tfile, const int connection_type, else if (connection_type == 0) { TCLOG(LOG_LEVEL_WARNING, - "x264 param fps_den is not set for connection type [%s], " - "adopting the default value [0]", - rdpbcgr_connection_type_names[connection_type]); + "[x264.%s] fps_den is not set, adopting the default value [%d]", + rdpbcgr_connection_type_names[connection_type], + X264_DEFAULT_FPS_DEN); param[connection_type].fps_den = X264_DEFAULT_FPS_DEN; } From 89277e5aea86269e93ec1a20716b29b7131662f9 Mon Sep 17 00:00:00 2001 From: Koichiro Iwao Date: Sat, 24 Aug 2024 22:30:30 +0900 Subject: [PATCH 07/12] x264: add CI test (cherry picked from commit 72ede776edce02605d3e30edfdcf74cb6048e3fe) --- .github/workflows/build.yml | 3 ++- scripts/install_xrdp_build_dependencies_with_apt.sh | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a117bce4..939cb9be 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -113,7 +113,8 @@ jobs: --disable-pixman" CONF_FLAGS_amd64_max: "--enable-ipv6 --enable-jpeg --enable-fuse --enable-mp3lame --enable-fdkaac --enable-opus --enable-rfxcodec --enable-painter - --enable-pixman --with-imlib2 --with-freetype2 --enable-tests" + --enable-pixman --with-imlib2 --with-freetype2 --enable-tests + --enable-x264" CONF_FLAGS_i386_max: "--enable-ipv6 --enable-jpeg --enable-mp3lame --enable-opus --enable-rfxcodec --enable-painter --disable-pixman --with-imlib2 --with-freetype2 diff --git a/scripts/install_xrdp_build_dependencies_with_apt.sh b/scripts/install_xrdp_build_dependencies_with_apt.sh index dec91a65..beb049c3 100755 --- a/scripts/install_xrdp_build_dependencies_with_apt.sh +++ b/scripts/install_xrdp_build_dependencies_with_apt.sh @@ -95,7 +95,8 @@ in libfdk-aac-dev \ libimlib2-dev \ libopus-dev \ - libpixman-1-dev" + libpixman-1-dev \ + libx264-dev" ;; *) echo "unsupported feature set: $FEATURE_SET" From d56396b7712c79a40f59def5ff4cac6f8fedd1e3 Mon Sep 17 00:00:00 2001 From: Koichiro Iwao Date: Fri, 23 Aug 2024 16:53:30 +0900 Subject: [PATCH 08/12] tconfig: add config which to prefer H264 vs RFX (cherry picked from commit 07e4e23c7b3e8b8c436c6e61d3c1e1e25943fec1) --- tests/xrdp/Makefile.am | 9 ++- tests/xrdp/gfx/gfx.toml | 40 +++++++++++ tests/xrdp/gfx/gfx_codec_h264_only.toml | 18 +++++ tests/xrdp/gfx/gfx_codec_h264_preferred.toml | 18 +++++ tests/xrdp/gfx/gfx_codec_order_undefined.toml | 18 +++++ tests/xrdp/gfx/gfx_codec_rfx_only.toml | 18 +++++ tests/xrdp/gfx/gfx_codec_rfx_preferred.toml | 18 +++++ .../xrdp/gfx/gfx_codec_rfx_preferred_odd.toml | 18 +++++ tests/xrdp/test_tconfig.c | 45 +++++++++++- xrdp/gfx.toml | 3 + xrdp/xrdp_tconfig.c | 70 +++++++++++++++++++ xrdp/xrdp_tconfig.h | 7 ++ 12 files changed, 280 insertions(+), 2 deletions(-) create mode 100644 tests/xrdp/gfx/gfx.toml create mode 100644 tests/xrdp/gfx/gfx_codec_h264_only.toml create mode 100644 tests/xrdp/gfx/gfx_codec_h264_preferred.toml create mode 100644 tests/xrdp/gfx/gfx_codec_order_undefined.toml create mode 100644 tests/xrdp/gfx/gfx_codec_rfx_only.toml create mode 100644 tests/xrdp/gfx/gfx_codec_rfx_preferred.toml create mode 100644 tests/xrdp/gfx/gfx_codec_rfx_preferred_odd.toml diff --git a/tests/xrdp/Makefile.am b/tests/xrdp/Makefile.am index d7203c18..26870bf0 100644 --- a/tests/xrdp/Makefile.am +++ b/tests/xrdp/Makefile.am @@ -19,7 +19,14 @@ EXTRA_DIST = \ test_not4_8bit.bmp \ test_not4_24bit.bmp \ test1.jpg \ - test_alpha_blend.png + test_alpha_blend.png \ + gfx/gfx.toml\ + gfx/gfx_codec_order_undefined.toml \ + gfx/gfx_codec_h264_preferred.toml \ + gfx/gfx_codec_h264_only.toml \ + gfx/gfx_codec_rfx_preferred.toml \ + gfx/gfx_codec_rfx_preferred_odd.toml \ + gfx/gfx_codec_rfx_only.toml TESTS = test_xrdp check_PROGRAMS = test_xrdp diff --git a/tests/xrdp/gfx/gfx.toml b/tests/xrdp/gfx/gfx.toml new file mode 100644 index 00000000..af3fcf86 --- /dev/null +++ b/tests/xrdp/gfx/gfx.toml @@ -0,0 +1,40 @@ +[codec] +order = [ "H.264", "RFX" ] + +[x264.default] +preset = "ultrafast" +tune = "zerolatency" +profile = "main" # profile is forced to baseline if preset == ultrafast +vbv_max_bitrate = 0 +vbv_buffer_size = 0 +fps_num = 24 +fps_den = 1 + +[x264.lan] +# inherits default + +[x264.wan] +vbv_max_bitrate = 15000 +vbv_buffer_size = 1500 + +[x264.broadband_high] +preset = "superfast" +vbv_max_bitrate = 8000 +vbv_buffer_Size = 800 + +[x264.satellite] +preset = "superfast" +vbv_max_bitrate = 5000 +vbv_buffer_size = 500 + +[x264.broadband_low] +preset = "veryfast" +tune = "zerolatency" +vbv_max_bitrate = 1600 +vbv_buffer_size = 66 + +[x264.modem] +preset = "fast" +tune = "zerolatency" +vbv_max_bitrate = 1200 +vbv_buffer_size = 50 diff --git a/tests/xrdp/gfx/gfx_codec_h264_only.toml b/tests/xrdp/gfx/gfx_codec_h264_only.toml new file mode 100644 index 00000000..86fa7d70 --- /dev/null +++ b/tests/xrdp/gfx/gfx_codec_h264_only.toml @@ -0,0 +1,18 @@ +[codec] +order = [ "H.264" ] + +[x264.default] +preset = "ultrafast" +tune = "zerolatency" +profile = "main" # profile is forced to baseline if preset == ultrafast +vbv_max_bitrate = 0 +vbv_buffer_size = 0 +fps_num = 24 +fps_den = 1 + +[x264.lan] +[x264.wan] +[x264.broadband_high] +[x264.satellite] +[x264.broadband_low] +[x264.modem] diff --git a/tests/xrdp/gfx/gfx_codec_h264_preferred.toml b/tests/xrdp/gfx/gfx_codec_h264_preferred.toml new file mode 100644 index 00000000..7d5b11ad --- /dev/null +++ b/tests/xrdp/gfx/gfx_codec_h264_preferred.toml @@ -0,0 +1,18 @@ +[codec] +order = [ "H.264", "RFX" ] + +[x264.default] +preset = "ultrafast" +tune = "zerolatency" +profile = "main" # profile is forced to baseline if preset == ultrafast +vbv_max_bitrate = 0 +vbv_buffer_size = 0 +fps_num = 24 +fps_den = 1 + +[x264.lan] +[x264.wan] +[x264.broadband_high] +[x264.satellite] +[x264.broadband_low] +[x264.modem] diff --git a/tests/xrdp/gfx/gfx_codec_order_undefined.toml b/tests/xrdp/gfx/gfx_codec_order_undefined.toml new file mode 100644 index 00000000..7432cb97 --- /dev/null +++ b/tests/xrdp/gfx/gfx_codec_order_undefined.toml @@ -0,0 +1,18 @@ +[codec] +order = [ ] + +[x264.default] +preset = "ultrafast" +tune = "zerolatency" +profile = "main" # profile is forced to baseline if preset == ultrafast +vbv_max_bitrate = 0 +vbv_buffer_size = 0 +fps_num = 24 +fps_den = 1 + +[x264.lan] +[x264.wan] +[x264.broadband_high] +[x264.satellite] +[x264.broadband_low] +[x264.modem] diff --git a/tests/xrdp/gfx/gfx_codec_rfx_only.toml b/tests/xrdp/gfx/gfx_codec_rfx_only.toml new file mode 100644 index 00000000..9ab14ea2 --- /dev/null +++ b/tests/xrdp/gfx/gfx_codec_rfx_only.toml @@ -0,0 +1,18 @@ +[codec] +order = [ "RFX" ] + +[x264.default] +preset = "ultrafast" +tune = "zerolatency" +profile = "main" # profile is forced to baseline if preset == ultrafast +vbv_max_bitrate = 0 +vbv_buffer_size = 0 +fps_num = 24 +fps_den = 1 + +[x264.lan] +[x264.wan] +[x264.broadband_high] +[x264.satellite] +[x264.broadband_low] +[x264.modem] diff --git a/tests/xrdp/gfx/gfx_codec_rfx_preferred.toml b/tests/xrdp/gfx/gfx_codec_rfx_preferred.toml new file mode 100644 index 00000000..c09d029b --- /dev/null +++ b/tests/xrdp/gfx/gfx_codec_rfx_preferred.toml @@ -0,0 +1,18 @@ +[codec] +order = [ "RFX", "H.264" ] + +[x264.default] +preset = "ultrafast" +tune = "zerolatency" +profile = "main" # profile is forced to baseline if preset == ultrafast +vbv_max_bitrate = 0 +vbv_buffer_size = 0 +fps_num = 24 +fps_den = 1 + +[x264.lan] +[x264.wan] +[x264.broadband_high] +[x264.satellite] +[x264.broadband_low] +[x264.modem] diff --git a/tests/xrdp/gfx/gfx_codec_rfx_preferred_odd.toml b/tests/xrdp/gfx/gfx_codec_rfx_preferred_odd.toml new file mode 100644 index 00000000..ff5c7015 --- /dev/null +++ b/tests/xrdp/gfx/gfx_codec_rfx_preferred_odd.toml @@ -0,0 +1,18 @@ +[codec] +order = [ "RFX", "H.264", "RFX" ] + +[x264.default] +preset = "ultrafast" +tune = "zerolatency" +profile = "main" # profile is forced to baseline if preset == ultrafast +vbv_max_bitrate = 0 +vbv_buffer_size = 0 +fps_num = 24 +fps_den = 1 + +[x264.lan] +[x264.wan] +[x264.broadband_high] +[x264.satellite] +[x264.broadband_low] +[x264.modem] diff --git a/tests/xrdp/test_tconfig.c b/tests/xrdp/test_tconfig.c index 586278eb..5219b267 100644 --- a/tests/xrdp/test_tconfig.c +++ b/tests/xrdp/test_tconfig.c @@ -6,6 +6,8 @@ #include "test_xrdp.h" #include "xrdp.h" +#define GFXCONF_STUBDIR XRDP_TOP_SRCDIR "/tests/xrdp/gfx/" + START_TEST(test_tconfig_gfx_always_success) { ck_assert_int_eq(1, 1); @@ -15,7 +17,7 @@ END_TEST START_TEST(test_tconfig_gfx_x264_load_basic) { struct xrdp_tconfig_gfx gfxconfig; - int rv = tconfig_load_gfx(XRDP_TOP_SRCDIR "/xrdp/gfx.toml", &gfxconfig); + int rv = tconfig_load_gfx(GFXCONF_STUBDIR "/gfx.toml", &gfxconfig); ck_assert_int_eq(rv, 0); @@ -31,6 +33,46 @@ START_TEST(test_tconfig_gfx_x264_load_basic) } END_TEST +START_TEST(test_tconfig_gfx_codec_order) +{ + struct xrdp_tconfig_gfx gfxconfig; + + /* H264 earlier */ + tconfig_load_gfx(GFXCONF_STUBDIR "/gfx_codec_h264_preferred.toml", &gfxconfig); + ck_assert_int_gt(gfxconfig.codec.h264_idx, -1); + ck_assert_int_gt(gfxconfig.codec.rfx_idx, -1); + ck_assert_int_lt(gfxconfig.codec.h264_idx, gfxconfig.codec.rfx_idx); + + /* H264 only */ + tconfig_load_gfx(GFXCONF_STUBDIR "/gfx_codec_h264_only.toml", &gfxconfig); + ck_assert_int_gt(gfxconfig.codec.h264_idx, -1); + ck_assert_int_eq(gfxconfig.codec.rfx_idx, -1); + + /* RFX earlier */ + tconfig_load_gfx(GFXCONF_STUBDIR "/gfx_codec_rfx_preferred.toml", &gfxconfig); + ck_assert_int_gt(gfxconfig.codec.h264_idx, -1); + ck_assert_int_gt(gfxconfig.codec.rfx_idx, -1); + ck_assert_int_lt(gfxconfig.codec.rfx_idx, gfxconfig.codec.h264_idx); + + /* RFX appears twice like: RFX, H264, RFX */ + tconfig_load_gfx(GFXCONF_STUBDIR "/gfx_codec_rfx_preferred_odd.toml", &gfxconfig); + ck_assert_int_gt(gfxconfig.codec.h264_idx, -1); + ck_assert_int_gt(gfxconfig.codec.rfx_idx, -1); + ck_assert_int_lt(gfxconfig.codec.rfx_idx, gfxconfig.codec.h264_idx); + + /* RFX only */ + tconfig_load_gfx(GFXCONF_STUBDIR "/gfx_codec_rfx_only.toml", &gfxconfig); + ck_assert_int_eq(gfxconfig.codec.h264_idx, -1); + ck_assert_int_gt(gfxconfig.codec.rfx_idx, -1); + + /* H264 is preferred if order undefined */ + tconfig_load_gfx(GFXCONF_STUBDIR "/gfx_codec_order_undefined.toml", &gfxconfig); + ck_assert_int_gt(gfxconfig.codec.h264_idx, -1); + ck_assert_int_gt(gfxconfig.codec.rfx_idx, -1); + ck_assert_int_lt(gfxconfig.codec.h264_idx, gfxconfig.codec.rfx_idx); +} +END_TEST + /******************************************************************************/ Suite * make_suite_tconfig_load_gfx(void) @@ -43,6 +85,7 @@ make_suite_tconfig_load_gfx(void) tc_tconfig_load_gfx = tcase_create("xrdp_tconfig_load_gfx"); tcase_add_test(tc_tconfig_load_gfx, test_tconfig_gfx_always_success); tcase_add_test(tc_tconfig_load_gfx, test_tconfig_gfx_x264_load_basic); + tcase_add_test(tc_tconfig_load_gfx, test_tconfig_gfx_codec_order); suite_add_tcase(s, tc_tconfig_load_gfx); diff --git a/xrdp/gfx.toml b/xrdp/gfx.toml index 5b7f27c4..af3fcf86 100644 --- a/xrdp/gfx.toml +++ b/xrdp/gfx.toml @@ -1,3 +1,6 @@ +[codec] +order = [ "H.264", "RFX" ] + [x264.default] preset = "ultrafast" tune = "zerolatency" diff --git a/xrdp/xrdp_tconfig.c b/xrdp/xrdp_tconfig.c index 663f9bce..26ce5bc4 100644 --- a/xrdp/xrdp_tconfig.c +++ b/xrdp/xrdp_tconfig.c @@ -199,6 +199,73 @@ tconfig_load_gfx_x264_ct(toml_table_t *tfile, const int connection_type, return 0; } +static int tconfig_load_gfx_order(toml_table_t *tfile, struct xrdp_tconfig_gfx *config) +{ + /* + * This config loader is not responsible to check if xrdp is built with + * H264/RFX support. Just loads configurations as-is. + */ + + TCLOG(LOG_LEVEL_TRACE, "[codec]"); + + int h264_found = 0; + int rfx_found = 0; + + config->codec.h264_idx = -1; + config->codec.rfx_idx = -1; + + toml_table_t *codec; + toml_array_t *order; + + if ((codec = toml_table_in(tfile, "codec")) != NULL && + (order = toml_array_in(codec, "order")) != NULL) + { + for (int i = 0; ; i++) + { + toml_datum_t datum = toml_string_at(order, i); + + if (datum.ok) + { + if (h264_found == 0 && + (g_strcasecmp(datum.u.s, "h264") == 0 || + g_strcasecmp(datum.u.s, "h.264") == 0)) + { + h264_found = 1; + config->codec.h264_idx = i; + } + if (rfx_found == 0 && + g_strcasecmp(datum.u.s, "rfx") == 0) + { + rfx_found = 1; + config->codec.rfx_idx = i; + } + free(datum.u.s); + } + else + { + break; + } + } + } + + if (h264_found == 0 && rfx_found == 0) + { + /* prefer H264 if no priority found */ + config->codec.h264_idx = 0; + config->codec.rfx_idx = 1; + + TCLOG(LOG_LEVEL_WARNING, "[codec] could not get GFX codec order, using default order" + " h264_idx [%d], rfx_idx [%d]", + config->codec.h264_idx, config->codec.rfx_idx); + + return 1; + } + + TCLOG(LOG_LEVEL_DEBUG, "[codec] h264_idx [%d], rfx_idx [%d]", + config->codec.h264_idx, config->codec.rfx_idx); + return 0; +} + int tconfig_load_gfx(const char *filename, struct xrdp_tconfig_gfx *config) { @@ -225,6 +292,9 @@ tconfig_load_gfx(const char *filename, struct xrdp_tconfig_gfx *config) memset(config, 0, sizeof(struct xrdp_tconfig_gfx)); + /* Load GFX order */ + tconfig_load_gfx_order(tfile, config); + /* First of all, read the default params and override later */ tconfig_load_gfx_x264_ct(tfile, 0, config->x264_param); diff --git a/xrdp/xrdp_tconfig.h b/xrdp/xrdp_tconfig.h index 6a5ba970..8364dcd8 100644 --- a/xrdp/xrdp_tconfig.h +++ b/xrdp/xrdp_tconfig.h @@ -42,8 +42,15 @@ struct xrdp_tconfig_gfx_x264_param int fps_den; }; +struct xrdp_tconfig_gfx_codec_order +{ + int h264_idx; + int rfx_idx; +}; + struct xrdp_tconfig_gfx { + struct xrdp_tconfig_gfx_codec_order codec; /* store x264 parameters for each connection type */ struct xrdp_tconfig_gfx_x264_param x264_param[NUM_CONNECTION_TYPES]; }; From 689a2620d3b043807392755cce214a26bc2863ed Mon Sep 17 00:00:00 2001 From: Koichiro Iwao Date: Mon, 26 Aug 2024 18:18:08 +0900 Subject: [PATCH 09/12] Introduce XRDP_H264 macro to indicate any H264 encoder enabled (cherry picked from commit 7238f8f99d1d123d28335dc152b1257b4fdc5113) --- xrdp/xrdp.h | 8 ++++++++ xrdp/xrdp_mm.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/xrdp/xrdp.h b/xrdp/xrdp.h index b2a8ae97..0e34b4e0 100644 --- a/xrdp/xrdp.h +++ b/xrdp/xrdp.h @@ -38,6 +38,14 @@ #include "xrdp_client_info.h" #include "log.h" +#if defined(XRDP_X264) || defined(XRDP_OPENH264) || defined(XRDP_NVENC) +#if !defined(XRDP_H264) +#define XRDP_H264 1 +#endif +#else +#undef XRDP_H264 +#endif + /* xrdp.c */ long g_xrdp_sync(long (*sync_func)(long param1, long param2), long sync_param1, diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index 6b29640b..5f997370 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -1353,7 +1353,7 @@ xrdp_mm_egfx_caps_advertise(void *user, int caps_count, /* prefer h264, todo use setting in xrdp.ini for this */ if (best_h264_index >= 0) { -#if defined(XRDP_X264) || defined(XRDP_NVENC) +#if defined(XRDP_H264) best_index = best_h264_index; self->egfx_flags = XRDP_EGFX_H264; #endif From 5bbeb03848f9f03586301d3edcdce913d06f1ed2 Mon Sep 17 00:00:00 2001 From: Koichiro Iwao Date: Mon, 26 Aug 2024 18:23:10 +0900 Subject: [PATCH 10/12] GFX: use the preferred codec preferred in the config (H264 or RFX) (cherry picked from commit 2c2585cc90e525b72620440cb857a151c042d6f5) --- xrdp/xrdp_mm.c | 22 +++++++++++++++++----- xrdp/xrdp_types.h | 3 +++ xrdp/xrdp_wm.c | 10 +++++++++- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index 5f997370..3357fe07 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -1345,19 +1345,31 @@ xrdp_mm_egfx_caps_advertise(void *user, int caps_count, break; } } +#if defined(XRDP_H264) + struct xrdp_tconfig_gfx_codec_order co = self->wm->gfx_config->codec; + bool_t use_h264 = (best_h264_index >= 0 && (best_pro_index < 0 || (co.h264_idx >= 0 && co.h264_idx < co.rfx_idx))); + + if (use_h264) + { + best_index = best_h264_index; + self->egfx_flags = XRDP_EGFX_H264; + } + else if (best_pro_index >= 0) + { + best_index = best_pro_index; + self->egfx_flags = XRDP_EGFX_RFX_PRO; + } +#else if (best_pro_index >= 0) { best_index = best_pro_index; self->egfx_flags = XRDP_EGFX_RFX_PRO; } - /* prefer h264, todo use setting in xrdp.ini for this */ if (best_h264_index >= 0) { -#if defined(XRDP_H264) - best_index = best_h264_index; - self->egfx_flags = XRDP_EGFX_H264; -#endif } +#endif + if (best_index >= 0) { LOG(LOG_LEVEL_INFO, " replying version 0x%8.8x flags 0x%8.8x", diff --git a/xrdp/xrdp_types.h b/xrdp/xrdp_types.h index c2195a9c..9aa744ba 100644 --- a/xrdp/xrdp_types.h +++ b/xrdp/xrdp_types.h @@ -29,6 +29,7 @@ #include "fifo.h" #include "guid.h" #include "xrdp_client_info.h" +#include "xrdp_tconfig.h" #define MAX_NR_CHANNELS 16 #define MAX_CHANNEL_NAME 16 @@ -566,6 +567,8 @@ struct xrdp_wm /* configuration derived from xrdp.ini */ struct xrdp_config *xrdp_config; + /* configuration derived from gfx.toml */ + struct xrdp_tconfig_gfx *gfx_config; struct xrdp_region *screen_dirty_region; int last_screen_draw_time; diff --git a/xrdp/xrdp_wm.c b/xrdp/xrdp_wm.c index 33c360e4..55d61380 100644 --- a/xrdp/xrdp_wm.c +++ b/xrdp/xrdp_wm.c @@ -114,8 +114,9 @@ xrdp_wm_create(struct xrdp_process *owner, self->target_surface = self->screen; self->current_surface_index = 0xffff; /* screen */ - /* to store configuration from xrdp.ini */ + /* to store configuration from xrdp.ini, gfx.toml */ self->xrdp_config = g_new0(struct xrdp_config, 1); + self->gfx_config = g_new0(struct xrdp_tconfig_gfx, 1); /* Load the channel config so libxrdp can check whether drdynvc is enabled or not */ @@ -162,6 +163,11 @@ xrdp_wm_delete(struct xrdp_wm *self) g_free(self->xrdp_config); } + if (self->gfx_config) + { + g_free(self->gfx_config); + } + /* free self */ g_free(self); } @@ -642,6 +648,8 @@ xrdp_wm_init(struct xrdp_wm *self) load_xrdp_config(self->xrdp_config, self->session->xrdp_ini, self->screen->bpp); + tconfig_load_gfx(XRDP_CFG_PATH "/gfx.toml", self->gfx_config); + /* Remove a font loaded on the previous config */ xrdp_font_delete(self->default_font); self->painter->font = NULL; /* May be set to the default_font */ From 401cd845f80ea8ad497cc69ca0d69d37e50a024b Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 28 Aug 2024 12:00:23 +0100 Subject: [PATCH 11/12] Rework codec order in tconfig Replaced codec idx values with a list of codecs from the config file. Added some logging for debugging (cherry picked from commit 1ac216da1d9c311b04c01d229333617b3e1d3815) --- tests/xrdp/test_tconfig.c | 32 +++++----- xrdp/xrdp_mm.c | 47 +++++++-------- xrdp/xrdp_tconfig.c | 119 +++++++++++++++++++++++++++----------- xrdp/xrdp_tconfig.h | 25 +++++++- 4 files changed, 147 insertions(+), 76 deletions(-) diff --git a/tests/xrdp/test_tconfig.c b/tests/xrdp/test_tconfig.c index 5219b267..5a7b6e5c 100644 --- a/tests/xrdp/test_tconfig.c +++ b/tests/xrdp/test_tconfig.c @@ -39,37 +39,37 @@ START_TEST(test_tconfig_gfx_codec_order) /* H264 earlier */ tconfig_load_gfx(GFXCONF_STUBDIR "/gfx_codec_h264_preferred.toml", &gfxconfig); - ck_assert_int_gt(gfxconfig.codec.h264_idx, -1); - ck_assert_int_gt(gfxconfig.codec.rfx_idx, -1); - ck_assert_int_lt(gfxconfig.codec.h264_idx, gfxconfig.codec.rfx_idx); + ck_assert_int_eq(gfxconfig.codec.codec_count, 2); + ck_assert_int_eq(gfxconfig.codec.codecs[0], XTC_H264); + ck_assert_int_eq(gfxconfig.codec.codecs[1], XTC_RFX); /* H264 only */ tconfig_load_gfx(GFXCONF_STUBDIR "/gfx_codec_h264_only.toml", &gfxconfig); - ck_assert_int_gt(gfxconfig.codec.h264_idx, -1); - ck_assert_int_eq(gfxconfig.codec.rfx_idx, -1); + ck_assert_int_eq(gfxconfig.codec.codec_count, 1); + ck_assert_int_eq(gfxconfig.codec.codecs[0], XTC_H264); /* RFX earlier */ tconfig_load_gfx(GFXCONF_STUBDIR "/gfx_codec_rfx_preferred.toml", &gfxconfig); - ck_assert_int_gt(gfxconfig.codec.h264_idx, -1); - ck_assert_int_gt(gfxconfig.codec.rfx_idx, -1); - ck_assert_int_lt(gfxconfig.codec.rfx_idx, gfxconfig.codec.h264_idx); + ck_assert_int_eq(gfxconfig.codec.codec_count, 2); + ck_assert_int_eq(gfxconfig.codec.codecs[0], XTC_RFX); + ck_assert_int_eq(gfxconfig.codec.codecs[1], XTC_H264); /* RFX appears twice like: RFX, H264, RFX */ tconfig_load_gfx(GFXCONF_STUBDIR "/gfx_codec_rfx_preferred_odd.toml", &gfxconfig); - ck_assert_int_gt(gfxconfig.codec.h264_idx, -1); - ck_assert_int_gt(gfxconfig.codec.rfx_idx, -1); - ck_assert_int_lt(gfxconfig.codec.rfx_idx, gfxconfig.codec.h264_idx); + ck_assert_int_eq(gfxconfig.codec.codec_count, 2); + ck_assert_int_eq(gfxconfig.codec.codecs[0], XTC_RFX); + ck_assert_int_eq(gfxconfig.codec.codecs[1], XTC_H264); /* RFX only */ tconfig_load_gfx(GFXCONF_STUBDIR "/gfx_codec_rfx_only.toml", &gfxconfig); - ck_assert_int_eq(gfxconfig.codec.h264_idx, -1); - ck_assert_int_gt(gfxconfig.codec.rfx_idx, -1); + ck_assert_int_eq(gfxconfig.codec.codec_count, 1); + ck_assert_int_eq(gfxconfig.codec.codecs[0], XTC_RFX); /* H264 is preferred if order undefined */ tconfig_load_gfx(GFXCONF_STUBDIR "/gfx_codec_order_undefined.toml", &gfxconfig); - ck_assert_int_gt(gfxconfig.codec.h264_idx, -1); - ck_assert_int_gt(gfxconfig.codec.rfx_idx, -1); - ck_assert_int_lt(gfxconfig.codec.h264_idx, gfxconfig.codec.rfx_idx); + ck_assert_int_eq(gfxconfig.codec.codec_count, 2); + ck_assert_int_eq(gfxconfig.codec.codecs[0], XTC_H264); + ck_assert_int_eq(gfxconfig.codec.codecs[1], XTC_RFX); } END_TEST diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index 3357fe07..8e113072 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -1270,7 +1270,6 @@ xrdp_mm_egfx_caps_advertise(void *user, int caps_count, struct xrdp_mm *self; struct xrdp_bitmap *screen; int index; - int best_index; int best_h264_index; int best_pro_index; int error; @@ -1298,7 +1297,6 @@ xrdp_mm_egfx_caps_advertise(void *user, int caps_count, } /* sort by version */ g_qsort(ver_flags, caps_count, sizeof(struct ver_flags_t), cmpverfunc); - best_index = -1; best_h264_index = -1; best_pro_index = -1; for (index = 0; index < caps_count; index++) @@ -1345,31 +1343,34 @@ xrdp_mm_egfx_caps_advertise(void *user, int caps_count, break; } } -#if defined(XRDP_H264) - struct xrdp_tconfig_gfx_codec_order co = self->wm->gfx_config->codec; - bool_t use_h264 = (best_h264_index >= 0 && (best_pro_index < 0 || (co.h264_idx >= 0 && co.h264_idx < co.rfx_idx))); - if (use_h264) + int best_index = -1; + struct xrdp_tconfig_gfx_codec_order *co = &self->wm->gfx_config->codec; + char cobuff[64]; + + LOG(LOG_LEVEL_INFO, "Codec search order is %s", + tconfig_codec_order_to_str(co, cobuff, sizeof(cobuff))); + for (index = 0 ; index < (unsigned int)co->codec_count ; ++index) { - best_index = best_h264_index; - self->egfx_flags = XRDP_EGFX_H264; - } - else if (best_pro_index >= 0) - { - best_index = best_pro_index; - self->egfx_flags = XRDP_EGFX_RFX_PRO; - } -#else - if (best_pro_index >= 0) - { - best_index = best_pro_index; - self->egfx_flags = XRDP_EGFX_RFX_PRO; - } - if (best_h264_index >= 0) - { - } +#if defined(XRDP_H264) + if (co->codecs[index] == XTC_H264 && best_h264_index >= 0) + { + LOG(LOG_LEVEL_INFO, "Matched H264 mode"); + best_index = best_h264_index; + self->egfx_flags = XRDP_EGFX_H264; + break; + } #endif + if (co->codecs[index] == XTC_RFX && best_pro_index >= 0) + { + LOG(LOG_LEVEL_INFO, "Matched RFX mode"); + best_index = best_pro_index; + self->egfx_flags = XRDP_EGFX_RFX_PRO; + break; + } + } + if (best_index >= 0) { LOG(LOG_LEVEL_INFO, " replying version 0x%8.8x flags 0x%8.8x", diff --git a/xrdp/xrdp_tconfig.c b/xrdp/xrdp_tconfig.c index 26ce5bc4..ec161987 100644 --- a/xrdp/xrdp_tconfig.c +++ b/xrdp/xrdp_tconfig.c @@ -48,6 +48,54 @@ #define X264_DEFAULT_FPS_NUM 24 #define X264_DEFAULT_FPS_DEN 1 +const char * +tconfig_codec_order_to_str( + const struct xrdp_tconfig_gfx_codec_order *codec_order, + char *buff, + unsigned int bufflen) +{ + if (bufflen < (8 * codec_order->codec_count)) + { + snprintf(buff, bufflen, "???"); + } + else + { + unsigned int p = 0; + int i; + for (i = 0 ; i < codec_order->codec_count; ++i) + { + if (p > 0) + { + buff[p++] = ','; + } + + switch (codec_order->codecs[i]) + { + case XTC_H264: + buff[p++] = 'H'; + buff[p++] = '2'; + buff[p++] = '6'; + buff[p++] = '4'; + break; + + case XTC_RFX: + buff[p++] = 'R'; + buff[p++] = 'F'; + buff[p++] = 'X'; + break; + + default: + buff[p++] = '?'; + buff[p++] = '?'; + buff[p++] = '?'; + } + } + buff[p++] = '\0'; + } + + return buff; +} + static int tconfig_load_gfx_x264_ct(toml_table_t *tfile, const int connection_type, struct xrdp_tconfig_gfx_x264_param *param) @@ -201,6 +249,8 @@ tconfig_load_gfx_x264_ct(toml_table_t *tfile, const int connection_type, static int tconfig_load_gfx_order(toml_table_t *tfile, struct xrdp_tconfig_gfx *config) { + char buff[64]; + /* * This config loader is not responsible to check if xrdp is built with * H264/RFX support. Just loads configurations as-is. @@ -211,8 +261,7 @@ static int tconfig_load_gfx_order(toml_table_t *tfile, struct xrdp_tconfig_gfx * int h264_found = 0; int rfx_found = 0; - config->codec.h264_idx = -1; - config->codec.rfx_idx = -1; + config->codec.codec_count = 0; toml_table_t *codec; toml_array_t *order; @@ -231,13 +280,15 @@ static int tconfig_load_gfx_order(toml_table_t *tfile, struct xrdp_tconfig_gfx * g_strcasecmp(datum.u.s, "h.264") == 0)) { h264_found = 1; - config->codec.h264_idx = i; + config->codec.codecs[config->codec.codec_count] = XTC_H264; + ++config->codec.codec_count; } if (rfx_found == 0 && g_strcasecmp(datum.u.s, "rfx") == 0) { rfx_found = 1; - config->codec.rfx_idx = i; + config->codec.codecs[config->codec.codec_count] = XTC_RFX; + ++config->codec.codec_count; } free(datum.u.s); } @@ -251,18 +302,19 @@ static int tconfig_load_gfx_order(toml_table_t *tfile, struct xrdp_tconfig_gfx * if (h264_found == 0 && rfx_found == 0) { /* prefer H264 if no priority found */ - config->codec.h264_idx = 0; - config->codec.rfx_idx = 1; + config->codec.codecs[0] = XTC_H264; + config->codec.codecs[1] = XTC_RFX; + config->codec.codec_count = 2; - TCLOG(LOG_LEVEL_WARNING, "[codec] could not get GFX codec order, using default order" - " h264_idx [%d], rfx_idx [%d]", - config->codec.h264_idx, config->codec.rfx_idx); + TCLOG(LOG_LEVEL_WARNING, "[codec] could not get GFX codec order, " + "using default order %s", + tconfig_codec_order_to_str(&config->codec, buff, sizeof(buff))); return 1; } - TCLOG(LOG_LEVEL_DEBUG, "[codec] h264_idx [%d], rfx_idx [%d]", - config->codec.h264_idx, config->codec.rfx_idx); + TCLOG(LOG_LEVEL_DEBUG, "[codec] %s", + tconfig_codec_order_to_str(&config->codec, buff, sizeof(buff))); return 0; } @@ -273,42 +325,39 @@ tconfig_load_gfx(const char *filename, struct xrdp_tconfig_gfx *config) char errbuf[200]; toml_table_t *tfile; + memset(config, 0, sizeof(struct xrdp_tconfig_gfx)); + if ((fp = fopen(filename, "r")) == NULL) { TCLOG(LOG_LEVEL_ERROR, "Error loading GFX config file %s (%s)", filename, g_get_strerror()); return 1; } - else if ((tfile = toml_parse_file(fp, errbuf, sizeof(errbuf))) == NULL) + + if ((tfile = toml_parse_file(fp, errbuf, sizeof(errbuf))) == NULL) { TCLOG(LOG_LEVEL_ERROR, "Error in GFX config file %s - %s", filename, errbuf); fclose(fp); return 1; } - else + + TCLOG(LOG_LEVEL_INFO, "Loading GFX config file %s", filename); + fclose(fp); + + /* Load GFX order */ + tconfig_load_gfx_order(tfile, config); + + /* First of all, read the default params and override later */ + tconfig_load_gfx_x264_ct(tfile, 0, config->x264_param); + + for (int ct = CONNECTION_TYPE_MODEM; ct < NUM_CONNECTION_TYPES; ct++) { - TCLOG(LOG_LEVEL_INFO, "Loading GFX config file %s", filename); - fclose(fp); - - memset(config, 0, sizeof(struct xrdp_tconfig_gfx)); - - /* Load GFX order */ - tconfig_load_gfx_order(tfile, config); - - /* First of all, read the default params and override later */ - tconfig_load_gfx_x264_ct(tfile, 0, config->x264_param); - - for (int ct = CONNECTION_TYPE_MODEM; ct < NUM_CONNECTION_TYPES; ct++) - { - memcpy(&config->x264_param[ct], &config->x264_param[0], - sizeof(struct xrdp_tconfig_gfx_x264_param)); - - tconfig_load_gfx_x264_ct(tfile, ct, config->x264_param); - } - - toml_free(tfile); - - return 0; + config->x264_param[ct] = config->x264_param[0]; + tconfig_load_gfx_x264_ct(tfile, ct, config->x264_param); } + + toml_free(tfile); + + return 0; } diff --git a/xrdp/xrdp_tconfig.h b/xrdp/xrdp_tconfig.h index 8364dcd8..d1fc0f11 100644 --- a/xrdp/xrdp_tconfig.h +++ b/xrdp/xrdp_tconfig.h @@ -42,10 +42,17 @@ struct xrdp_tconfig_gfx_x264_param int fps_den; }; +enum xrdp_tconfig_codecs +{ + XTC_H264, + XTC_RFX +}; + struct xrdp_tconfig_gfx_codec_order { - int h264_idx; - int rfx_idx; + + enum xrdp_tconfig_codecs codecs[2]; + unsigned int codec_count; }; struct xrdp_tconfig_gfx @@ -68,6 +75,20 @@ static const char *const rdpbcgr_connection_type_names[] = 0 }; +/** + * Provide a string representation of a codec order + * + * @param codec_order Codec order struct + * @param buff Buffer for result + * @param bufflen Length of above + * @return Convenience copy of buff + */ +const char * +tconfig_codec_order_to_str( + const struct xrdp_tconfig_gfx_codec_order *codec_order, + char *buff, + unsigned int bufflen); + int tconfig_load_gfx(const char *filename, struct xrdp_tconfig_gfx *config); #endif From 8bf9ed48afad7d77f50b0edff95e81aeb4c3496f Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 29 Aug 2024 12:17:45 +0100 Subject: [PATCH 12/12] Further changes to selectable H.264 support - Fix CI errors - tconfig_load_gfx() removes H.264 from the supported codec list if significant errors are found loading the H.264 configuration - tconfig_load_gfx() always produces a usable config, even if the specified file can't be loaded (cherry picked from commit 535151127286f7aee80159b137a97a90d9d03e8c) --- tests/xrdp/gfx/gfx_missing_h264.toml | 9 ++++ tests/xrdp/test_tconfig.c | 24 +++++++++ xrdp/xrdp_mm.c | 6 ++- xrdp/xrdp_tconfig.c | 81 ++++++++++++++++++++++++---- xrdp/xrdp_tconfig.h | 16 ++++-- 5 files changed, 122 insertions(+), 14 deletions(-) create mode 100644 tests/xrdp/gfx/gfx_missing_h264.toml diff --git a/tests/xrdp/gfx/gfx_missing_h264.toml b/tests/xrdp/gfx/gfx_missing_h264.toml new file mode 100644 index 00000000..90e2eeb4 --- /dev/null +++ b/tests/xrdp/gfx/gfx_missing_h264.toml @@ -0,0 +1,9 @@ +[codec] +order = [ "H.264", "RFX" ] + +[x264.lan] +[x264.wan] +[x264.broadband_high] +[x264.satellite] +[x264.broadband_low] +[x264.modem] diff --git a/tests/xrdp/test_tconfig.c b/tests/xrdp/test_tconfig.c index 5a7b6e5c..81184984 100644 --- a/tests/xrdp/test_tconfig.c +++ b/tests/xrdp/test_tconfig.c @@ -73,6 +73,28 @@ START_TEST(test_tconfig_gfx_codec_order) } END_TEST +START_TEST(test_tconfig_gfx_missing_file) +{ + struct xrdp_tconfig_gfx gfxconfig; + + /* Check RFX config is returned if the file doesn't exist */ + tconfig_load_gfx(GFXCONF_STUBDIR "/no_such_file.toml", &gfxconfig); + ck_assert_int_eq(gfxconfig.codec.codec_count, 1); + ck_assert_int_eq(gfxconfig.codec.codecs[0], XTC_RFX); +} +END_TEST + +START_TEST(test_tconfig_gfx_missing_h264) +{ + struct xrdp_tconfig_gfx gfxconfig; + + /* Check RFX config only is returned if H.264 parameters are missing */ + tconfig_load_gfx(GFXCONF_STUBDIR "/gfx_missing_h264.toml", &gfxconfig); + ck_assert_int_eq(gfxconfig.codec.codec_count, 1); + ck_assert_int_eq(gfxconfig.codec.codecs[0], XTC_RFX); +} +END_TEST + /******************************************************************************/ Suite * make_suite_tconfig_load_gfx(void) @@ -86,6 +108,8 @@ make_suite_tconfig_load_gfx(void) tcase_add_test(tc_tconfig_load_gfx, test_tconfig_gfx_always_success); tcase_add_test(tc_tconfig_load_gfx, test_tconfig_gfx_x264_load_basic); tcase_add_test(tc_tconfig_load_gfx, test_tconfig_gfx_codec_order); + tcase_add_test(tc_tconfig_load_gfx, test_tconfig_gfx_missing_file); + tcase_add_test(tc_tconfig_load_gfx, test_tconfig_gfx_missing_h264); suite_add_tcase(s, tc_tconfig_load_gfx); diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index 8e113072..bc4868c7 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -1277,6 +1277,10 @@ xrdp_mm_egfx_caps_advertise(void *user, int caps_count, int flags; struct ver_flags_t *ver_flags; +#if !defined(XRDP_H264) + UNUSED_VAR(best_h264_index); +#endif + LOG(LOG_LEVEL_INFO, "xrdp_mm_egfx_caps_advertise:"); self = (struct xrdp_mm *) user; screen = self->wm->screen; @@ -1350,7 +1354,7 @@ xrdp_mm_egfx_caps_advertise(void *user, int caps_count, LOG(LOG_LEVEL_INFO, "Codec search order is %s", tconfig_codec_order_to_str(co, cobuff, sizeof(cobuff))); - for (index = 0 ; index < (unsigned int)co->codec_count ; ++index) + for (index = 0 ; index < co->codec_count ; ++index) { #if defined(XRDP_H264) if (co->codecs[index] == XTC_H264 && best_h264_index >= 0) diff --git a/xrdp/xrdp_tconfig.c b/xrdp/xrdp_tconfig.c index ec161987..744cc3d8 100644 --- a/xrdp/xrdp_tconfig.c +++ b/xrdp/xrdp_tconfig.c @@ -318,14 +318,61 @@ static int tconfig_load_gfx_order(toml_table_t *tfile, struct xrdp_tconfig_gfx * return 0; } +/** + * Determines whether a codec is enabled + * @param co Ordered codec list + * @param code Code of codec to look for + * @return boolean + */ +static int +codec_enabled(const struct xrdp_tconfig_gfx_codec_order *co, + enum xrdp_tconfig_codecs code) +{ + for (unsigned short i = 0; i < co->codec_count; ++i) + { + if (co->codecs[i] == code) + { + return 1; + } + } + + return 0; +} + +/** + * Disables a Codec by removing it from the codec list + * @param co Ordered codec list + * @param code Code of codec to remove from list + * + * The order of the passed-in codec list is preserved. + */ +static void +disable_codec(struct xrdp_tconfig_gfx_codec_order *co, + enum xrdp_tconfig_codecs code) +{ + unsigned short j = 0; + for (unsigned short i = 0; i < co->codec_count; ++i) + { + if (co->codecs[i] != code) + { + co->codecs[j++] = co->codecs[i]; + } + } + co->codec_count = j; +} + int tconfig_load_gfx(const char *filename, struct xrdp_tconfig_gfx *config) { FILE *fp; char errbuf[200]; toml_table_t *tfile; + int rv = 0; - memset(config, 0, sizeof(struct xrdp_tconfig_gfx)); + /* Default to just RFX support. in case we can't load anything */ + config->codec.codec_count = 1; + config->codec.codecs[0] = XTC_RFX; + memset(config->x264_param, 0, sizeof(config->x264_param)); if ((fp = fopen(filename, "r")) == NULL) { @@ -344,20 +391,34 @@ tconfig_load_gfx(const char *filename, struct xrdp_tconfig_gfx *config) TCLOG(LOG_LEVEL_INFO, "Loading GFX config file %s", filename); fclose(fp); - /* Load GFX order */ + /* Load GFX codec order */ tconfig_load_gfx_order(tfile, config); - /* First of all, read the default params and override later */ - tconfig_load_gfx_x264_ct(tfile, 0, config->x264_param); - - for (int ct = CONNECTION_TYPE_MODEM; ct < NUM_CONNECTION_TYPES; ct++) + /* H.264 configuration */ + if (codec_enabled(&config->codec, XTC_H264)) { - config->x264_param[ct] = config->x264_param[0]; - tconfig_load_gfx_x264_ct(tfile, ct, config->x264_param); + /* First of all, read the default params */ + if (tconfig_load_gfx_x264_ct(tfile, 0, config->x264_param) != 0) + { + /* We can't read the H.264 defaults. Disable H.264 */ + LOG(LOG_LEVEL_WARNING, "H.264 support will be disabled"); + disable_codec(&config->codec, XTC_H264); + rv = 1; + } + else + { + /* Copy default params to other connection types, and + * then override them */ + for (int ct = CONNECTION_TYPE_MODEM; ct < NUM_CONNECTION_TYPES; + ct++) + { + config->x264_param[ct] = config->x264_param[0]; + tconfig_load_gfx_x264_ct(tfile, ct, config->x264_param); + } + } } - toml_free(tfile); - return 0; + return rv; } diff --git a/xrdp/xrdp_tconfig.h b/xrdp/xrdp_tconfig.h index d1fc0f11..649d6490 100644 --- a/xrdp/xrdp_tconfig.h +++ b/xrdp/xrdp_tconfig.h @@ -50,9 +50,8 @@ enum xrdp_tconfig_codecs struct xrdp_tconfig_gfx_codec_order { - enum xrdp_tconfig_codecs codecs[2]; - unsigned int codec_count; + unsigned short codec_count; }; struct xrdp_tconfig_gfx @@ -89,6 +88,17 @@ tconfig_codec_order_to_str( char *buff, unsigned int bufflen); -int tconfig_load_gfx(const char *filename, struct xrdp_tconfig_gfx *config); +/** + * Loads the GFX config from the specified file + * + * @param filename Name of file to load + * @param config Struct to receive result + * @return 0 for success + * + * In the event of failure, an error is logged. A minimal + * useable configuration is always returned + */ +int +tconfig_load_gfx(const char *filename, struct xrdp_tconfig_gfx *config); #endif