[v0.10] add OpenH264 support

[metalefty]

• Add OpenH264 support (cherry-pick of add openh264 support #3311)
• Enable to select preferred H.264 encoder in gfx.toml
• Make OpenH264 encoding parameters configurable in gfx.toml
• Add tests for OpenH264 params and prefered H264 encoder
• Add --enable-openh264 to CI

@matt335672 Could you review this?

This works very well on my side. Comparing x264 and OpenH264 is easy. Just edit h264_encoder in gfx.conf and reconnect. It looks x264 is smoother and faster but OpenH264 can control bitrate stricter than x264 because OpenH264 can skip frames so as not to exceed the max bitrate.

BTW, we forgot to add gfx_missing_h264.toml to EXTRA_DIST before. The test passed. The result is the same as expected, but the process is not what we expect.

ok 18 - ../../../../tests/xrdp/test_tconfig.c:xrdp_tconfig_load_gfx:test_tconfig_gfx_codec_order: Passed
PASS: test_xrdp 18 - ../../../../tests/xrdp/test_tconfig.c:xrdp_tconfig_load_gfx:test_tconfig_gfx_codec_order: Passed
[2024-11-28T14:35:36.509+0000] [ERROR] TConfig: Error loading GFX config file ../../../../tests/xrdp/gfx//no_such_file.toml (No such file or directory)
ok 19 - ../../../../tests/xrdp/test_tconfig.c:xrdp_tconfig_load_gfx:test_tconfig_gfx_missing_file: Passed
PASS: test_xrdp 19 - ../../../../tests/xrdp/test_tconfig.c:xrdp_tconfig_load_gfx:test_tconfig_gfx_missing_file: Passed
[2024-11-28T14:35:36.510+0000] [ERROR] TConfig: Error loading GFX config file ../../../../tests/xrdp/gfx//gfx_missing_h264.toml (No such file or directory)
ok 20 - ../../../../tests/xrdp/test_tconfig.c:xrdp_tconfig_load_gfx:test_tconfig_gfx_missing_h264: Passed
PASS: test_xrdp 20 - ../../../../tests/xrdp/test_tconfig.c:xrdp_tconfig_load_gfx:test_tconfig_gfx_missing_h264: Passed
[2024-11-28T14:35:36.511+0000] [INFO ] TConfig: Loading GFX config file ../../../../tests/xgg/uurdp/gfx//gfx.toml
ok 21 - ../../../../tests/xrdp/test_tconfig.c:xrdp_tconfig_load_gfx:test_tconfig_gfx_oh264_load_basic: Passed
PASS: test_xrdp 21 - ../../../../tests/xrdp/test_tconfig.c:xrdp_tconfig_load_gfx:test_tconfig_gfx_oh264_load_basic: Passed

[matt335672]

Out of time today, but here's a suggested fix for the CI fail:-

--- a/xrdp/xrdp_encoder_openh264.c
+++ b/xrdp/xrdp_encoder_openh264.c
@@ -53,6 +53,20 @@ struct openh264_global
         openh264_param[NUM_CONNECTION_TYPES];
 };
 
+// The method invocations on ISVCEncoder are different for C and C++, as
+// ISVCEncoder is a true class in C++, but an emulated one in C
+#ifdef __cplusplus
+#define ENC_GET_DEFAULT_PARAMS(obj,pParam) (obj)->GetDefaultParams(pParam)
+#define ENC_INITIALIZE_EXT(obj,pParam)     (obj)->InitializeExt(pParam)
+#define ENC_ENCODE_FRAME(obj,kpSrcPic,pBsInfo) \
+    (obj)->EncodeFrame(kpSrcPic, pBsInfo)
+#else
+#define ENC_GET_DEFAULT_PARAMS(obj,pParam) (*obj)->GetDefaultParams(obj, pParam)
+#define ENC_INITIALIZE_EXT(obj,pParam)     (*obj)->InitializeExt(obj, pParam)
+#define ENC_ENCODE_FRAME(obj,kpSrcPic,pBsInfo) \
+    (*obj)->EncodeFrame(obj, kpSrcPic, pBsInfo)
+#endif
+
 /*****************************************************************************/
 void *
 xrdp_encoder_openh264_create(void)
@@ -167,7 +181,7 @@ xrdp_encoder_openh264_encode(void *handle, int session, int left, int top,
             LOG(LOG_LEVEL_INFO, "xrdp_encoder_openh264_encode: "
                 "WelsCreateSVCEncoder rv %p for width %d height %d",
                 oe->openh264_enc_han, width, height);
-            status = (*oe->openh264_enc_han)->GetDefaultParams(
+            status = ENC_GET_DEFAULT_PARAMS(
                          oe->openh264_enc_han, &encParamExt);
             LOG(LOG_LEVEL_INFO, "xrdp_encoder_openh264_encode: "
                 "GetDefaultParams rv %d", status);
@@ -191,7 +205,7 @@ xrdp_encoder_openh264_encode(void *handle, int session, int left, int top,
                 slc->iVideoHeight = encParamExt.iPicHeight;
                 slc->iSpatialBitrate = encParamExt.iTargetBitrate;
                 slc->iMaxSpatialBitrate = encParamExt.iMaxBitrate;
-                status = (*oe->openh264_enc_han)->InitializeExt(
+                status = ENC_INITIALIZE_EXT(
                              oe->openh264_enc_han, &encParamExt);
                 LOG(LOG_LEVEL_INFO, "xrdp_encoder_openh264_encode: "
                     "InitializeExt rv %d", status);
@@ -270,8 +284,7 @@ xrdp_encoder_openh264_encode(void *handle, int session, int left, int top,
             }
         }
         g_memset(&info, 0, sizeof(info));
-        status = (*oe->openh264_enc_han)->EncodeFrame(oe->openh264_enc_han,
-                 &pic1, &info);
+        status = ENC_ENCODE_FRAME(oe->openh264_enc_han, &pic1, &info);
         if (status != 0)
         {
             LOG(LOG_LEVEL_TRACE, "OpenH264: Failed to encode frame");

[metalefty]

I had no idea about the CI failure, thanks!

[matt335672]

That looks fine to me. I had a couple of thoughts about the design:-

1. I was wondering whether we could fold the x264 and Openh264 parameter sets together in gfx.toml, but on reflection I think they're better left as separate sets, in case we need to add something else to one of them.
2. I was also wondering about h264_encoder and whether this could be better folded into the order parameter. Again, on reflection, I think it's better this way, as it's easier for different distros to edit this setting (if they need to) using a single sed command.

[metalefty]

Thanks for the comments.

1. x264 and OpenH264 parameters are not fully compatible. Some parameters can be set on one side but not on the other. So I decided to make separate sets of configurations. For clarity, I just exposed the encoder's internal parameters to gfx.toml. For example, EnableFrameSkip corresponds to OpenH264's internal bEnableFrameSkip. vbv_max_bitrate corresponds to x264's i_vbv_max_bitrate. I believe this makes it easy to refer to the OpenH264 or x264 API documentation to find out what the parameters mean.
2. Exactly, that was my intention.