From 5b77fa34878ae894433988f5f6dec24eb4280d0d Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sun, 12 Jan 2025 09:16:57 +0100 Subject: [PATCH 01/11] [codec,h264] reset h264 instance when resetting the context reinitialize the h264 encoder. this allows switching the backend --- libfreerdp/codec/h264.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/libfreerdp/codec/h264.c b/libfreerdp/codec/h264.c index df10d1f3a..46790ea4c 100644 --- a/libfreerdp/codec/h264.c +++ b/libfreerdp/codec/h264.c @@ -659,11 +659,6 @@ static BOOL h264_context_init(H264_CONTEXT* h264) if (!h264) return FALSE; - h264->log = WLog_Get(TAG); - - if (!h264->log) - return FALSE; - h264->subsystem = NULL; InitOnceExecuteOnce(&subsystems_once, h264_register_subsystems, NULL, NULL); @@ -691,6 +686,12 @@ BOOL h264_context_reset(H264_CONTEXT* h264, UINT32 width, UINT32 height) h264->width = width; h264->height = height; + + if (h264->subsystem && h264->subsystem->Uninit) + h264->subsystem->Uninit(h264); + if (!h264_context_init(h264)) + return FALSE; + return yuv_context_reset(h264->yuv, width, height); } @@ -700,6 +701,11 @@ H264_CONTEXT* h264_context_new(BOOL Compressor) if (!h264) return NULL; + h264->log = WLog_Get(TAG); + + if (!h264->log) + goto fail; + h264->Compressor = Compressor; if (Compressor) From db278ec457b181b05b095516b80ccf3e5569dd51 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sun, 12 Jan 2025 08:11:53 +0100 Subject: [PATCH 02/11] [primitives] only run benchmark if opencl enabled there is no need to compare implementation performance if only one optimized version is available. --- libfreerdp/primitives/primitives.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libfreerdp/primitives/primitives.c b/libfreerdp/primitives/primitives.c index e28e244f4..9aa15e446 100644 --- a/libfreerdp/primitives/primitives.c +++ b/libfreerdp/primitives/primitives.c @@ -91,8 +91,6 @@ static BOOL CALLBACK primitives_init_generic_cb(PINIT_ONCE once, PVOID param, PV static BOOL primitives_init_optimized(primitives_t* prims) { - primitives_init_generic(prims); - #if defined(HAVE_CPU_OPTIMIZED_PRIMITIVES) primitives_init_add_opt(prims); primitives_init_andor_opt(prims); @@ -220,7 +218,7 @@ static BOOL primitives_autodetect_best(primitives_t* prims) }; const struct prim_benchmark* best = NULL; -#if !defined(HAVE_CPU_OPTIMIZED_PRIMITIVES) && !defined(WITH_OPENCL) +#if !defined(HAVE_CPU_OPTIMIZED_PRIMITIVES) || !defined(WITH_OPENCL) { struct prim_benchmark* cur = &testcases[0]; cur->prims = primitives_get_by_type(cur->flags); From 720d48583a8549b7ddb0183e25637ecef064c962 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sun, 12 Jan 2025 06:30:19 +0100 Subject: [PATCH 03/11] [codec,h264] add basic unit test --- libfreerdp/codec/test/CMakeLists.txt | 1 + libfreerdp/codec/test/TestFreeRDPCodecH264.c | 192 +++++++++++++++++++ 2 files changed, 193 insertions(+) create mode 100644 libfreerdp/codec/test/TestFreeRDPCodecH264.c diff --git a/libfreerdp/codec/test/CMakeLists.txt b/libfreerdp/codec/test/CMakeLists.txt index 33173e10e..5f607e492 100644 --- a/libfreerdp/codec/test/CMakeLists.txt +++ b/libfreerdp/codec/test/CMakeLists.txt @@ -11,6 +11,7 @@ set(TESTS TestFreeRDPCodecPlanar.c TestFreeRDPCodecCopy.c TestFreeRDPCodecClear.c + TestFreeRDPCodecH264.c TestFreeRDPCodecInterleaved.c TestFreeRDPCodecProgressive.c TestFreeRDPCodecRemoteFX.c diff --git a/libfreerdp/codec/test/TestFreeRDPCodecH264.c b/libfreerdp/codec/test/TestFreeRDPCodecH264.c new file mode 100644 index 000000000..2433263b2 --- /dev/null +++ b/libfreerdp/codec/test/TestFreeRDPCodecH264.c @@ -0,0 +1,192 @@ + +#include +#include +#include +#include + +#include + +#include +#include + +static const char* print_ns(UINT64 start, UINT64 end, char* buffer, size_t len) +{ + const UINT64 diff = end - start; + const unsigned ns = diff % 1000; + const unsigned us = (diff / 1000) % 1000; + const unsigned ms = (diff / 1000000) % 1000; + const unsigned s = (diff / 1000000000ULL) % 1000; + (void)_snprintf(buffer, len, "%03u %03u %03u %03uns", s, ms, us, ns); + return buffer; +} + +static BOOL testContextOptions(BOOL compressor, uint32_t width, uint32_t height) +{ + BOOL rc = FALSE; + const UINT64 start = winpr_GetUnixTimeNS(); + H264_CONTEXT* h264 = h264_context_new(FALSE); + if (!h264) + return FALSE; + + struct optpair_s + { + H264_CONTEXT_OPTION opt; + uint32_t val; + }; + const struct optpair_s optpair[] = { { H264_CONTEXT_OPTION_RATECONTROL, H264_RATECONTROL_VBR }, + { H264_CONTEXT_OPTION_BITRATE, 2323 }, + { H264_CONTEXT_OPTION_FRAMERATE, 23 }, + { H264_CONTEXT_OPTION_QP, 21 }, + { H264_CONTEXT_OPTION_USAGETYPE, 23 } }; + for (size_t x = 0; x < ARRAYSIZE(optpair); x++) + { + const struct optpair_s* cur = &optpair[x]; + if (!h264_context_set_option(h264, cur->opt, cur->val)) + goto fail; + } + if (!h264_context_reset(h264, width, height)) + goto fail; + + rc = TRUE; +fail: + h264_context_free(h264); + const UINT64 end = winpr_GetUnixTimeNS(); + + char buffer[64] = { 0 }; + printf("[%s] %" PRIu32 "x%" PRIu32 " took %s\n", __func__, width, height, + print_ns(start, end, buffer, sizeof(buffer))); + return rc; +} + +static void* allocRGB(uint32_t format, uint32_t width, uint32_t height, uint32_t* pstride) +{ + const size_t bpp = FreeRDPGetBytesPerPixel(format); + const size_t stride = bpp * width + 32; + WINPR_ASSERT(pstride); + *pstride = WINPR_ASSERTING_INT_CAST(uint32_t, stride); + + uint8_t* rgb = calloc(stride, height); + if (!rgb) + return NULL; + + for (size_t x = 0; x < height; x++) + { + winpr_RAND(&rgb[x * stride], width * bpp); + } + return rgb; +} + +static BOOL compareRGB(const uint8_t* src, const uint8_t* dst, uint32_t format, size_t width, + size_t stride, size_t height) +{ + const size_t bpp = FreeRDPGetBytesPerPixel(format); + for (size_t y = 0; y < height; y++) + { + const uint8_t* csrc = &src[y * stride]; + const uint8_t* cdst = &dst[y * stride]; + const int rc = memcmp(csrc, cdst, width * bpp); + // TODO: Both, AVC420 encoding and decoding are lossy. + // TODO: Find a proper error margin to check for +#if 0 + if (rc != 0) + return FALSE; +#endif + } + return TRUE; +} + +static BOOL testEncode(uint32_t format, uint32_t width, uint32_t height) +{ + BOOL rc = FALSE; + void* src = NULL; + void* out = NULL; + RDPGFX_H264_METABLOCK meta = { 0 }; + H264_CONTEXT* h264 = h264_context_new(TRUE); + H264_CONTEXT* h264dec = h264_context_new(FALSE); + if (!h264 || !h264dec) + goto fail; + + if (!h264_context_reset(h264, width, height)) + goto fail; + if (!h264_context_reset(h264dec, width, height)) + goto fail; + + uint32_t stride = 0; + uint32_t ostride = 0; + src = allocRGB(format, width, height, &stride); + out = allocRGB(format, width, height, &ostride); + if (!src || !out || (stride < width) || (stride != ostride)) + goto fail; + + const RECTANGLE_16 rect = { .left = 0, .top = 0, .right = width, .bottom = height }; + uint32_t dstsize = 0; + uint8_t* dst = NULL; + if (avc420_compress(h264, src, format, stride, width, height, &rect, &dst, &dstsize, &meta) < 0) + goto fail; + if ((dstsize == 0) || !dst) + goto fail; + + if (avc420_decompress(h264dec, dst, dstsize, out, format, stride, width, height, &rect, 1) < 0) + goto fail; + + rc = compareRGB(src, out, format, width, stride, height); +fail: + h264_context_free(h264); + h264_context_free(h264dec); + free_h264_metablock(&meta); + free(src); + free(out); + return rc; +} + +int TestFreeRDPCodecH264(int argc, char* argv[]) +{ + WINPR_UNUSED(argc); + WINPR_UNUSED(argv); + + UINT32 width = 124; + UINT32 height = 54; + const UINT32 formats[] = { + PIXEL_FORMAT_ABGR15, PIXEL_FORMAT_ARGB15, PIXEL_FORMAT_BGR15, PIXEL_FORMAT_BGR16, + PIXEL_FORMAT_BGR24, PIXEL_FORMAT_RGB15, PIXEL_FORMAT_RGB16, PIXEL_FORMAT_RGB24, + PIXEL_FORMAT_ABGR32, PIXEL_FORMAT_ARGB32, PIXEL_FORMAT_XBGR32, PIXEL_FORMAT_XRGB32, + PIXEL_FORMAT_BGRA32, PIXEL_FORMAT_RGBA32, PIXEL_FORMAT_BGRX32, PIXEL_FORMAT_RGBX32, + }; + + if (argc == 3) + { + errno = 0; + width = strtoul(argv[1], NULL, 0); + height = strtoul(argv[2], NULL, 0); + if ((errno != 0) || (width == 0) || (height == 0)) + { + char buffer[128] = { 0 }; + (void)fprintf(stderr, "%s failed: width=%" PRIu32 ", height=%" PRIu32 ", errno=%s\n", + __func__, width, height, winpr_strerror(errno, buffer, sizeof(buffer))); + return -1; + } + } + +#if !defined(WITH_MEDIACODEC) && !defined(WITH_MEDIA_FOUNDATION) && !defined(WITH_OPENH264) && \ + !defined(WITH_VIDEO_FFMPEG) + (void)fprintf(stderr, "[%s] skipping, no H264 encoder/decoder support compiled in\n", __func__); + return 0; +#endif + + if (!testContextOptions(FALSE, width, height)) + return -1; + if (!testContextOptions(TRUE, width, height)) + return -1; + + for (size_t x = 0; x < ARRAYSIZE(formats); x++) + { + const UINT32 SrcFormat = formats[x]; + for (size_t y = 0; y < ARRAYSIZE(formats); y++) + { + if (!testEncode(SrcFormat, width, height)) + return -1; + } + } + + return 0; +} From 06402de8012ccba9754523dcdd8db55035090771 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sun, 12 Jan 2025 09:07:05 +0100 Subject: [PATCH 04/11] [server,shadow] properly clean up encoder --- server/shadow/shadow_encoder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/shadow/shadow_encoder.c b/server/shadow/shadow_encoder.c index 98c13234a..b3d596ffb 100644 --- a/server/shadow/shadow_encoder.c +++ b/server/shadow/shadow_encoder.c @@ -504,7 +504,7 @@ rdpShadowEncoder* shadow_encoder_new(rdpShadowClient* client) if (shadow_encoder_init(encoder) < 0) { - free(encoder); + shadow_encoder_free(encoder); return NULL; } From 5fa44d30bef0a158a14436f9a44b1508ef56cd18 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sun, 12 Jan 2025 09:42:27 +0100 Subject: [PATCH 05/11] [channels,remdesk] fix return value check --- channels/remdesk/server/remdesk_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/channels/remdesk/server/remdesk_main.c b/channels/remdesk/server/remdesk_main.c index c4552df69..181efeb21 100644 --- a/channels/remdesk/server/remdesk_main.c +++ b/channels/remdesk/server/remdesk_main.c @@ -279,7 +279,7 @@ static UINT remdesk_recv_ctl_verify_password_pdu(RemdeskServerContext* context, const size_t cbExpertBlobW = header->DataLength - 4; pdu.expertBlob = ConvertWCharNToUtf8Alloc(expertBlobW, cbExpertBlobW / sizeof(WCHAR), NULL); - if (pdu.expertBlob) + if (!pdu.expertBlob) return ERROR_INTERNAL_ERROR; WLog_INFO(TAG, "ExpertBlob: %s", pdu.expertBlob); From 45a7597cfa8aa3c5959ac73bf93220d8e57b8639 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sun, 12 Jan 2025 09:44:33 +0100 Subject: [PATCH 06/11] [gdi,graphics] fix region leak --- libfreerdp/gdi/graphics.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/libfreerdp/gdi/graphics.c b/libfreerdp/gdi/graphics.c index cc668b35a..e9ed622eb 100644 --- a/libfreerdp/gdi/graphics.c +++ b/libfreerdp/gdi/graphics.c @@ -160,12 +160,16 @@ static BOOL gdi_Bitmap_Decompress(rdpContext* context, rdpBitmap* bitmap, const { if ((codecId == RDP_CODEC_ID_REMOTEFX) || (codecId == RDP_CODEC_ID_IMAGE_REMOTEFX)) { - REGION16 invalidRegion; + REGION16 invalidRegion = { 0 }; region16_init(&invalidRegion); - if (!rfx_process_message(context->codecs->rfx, pSrcData, SrcSize, bitmap->left, - bitmap->top, bitmap->data, bitmap->format, gdi->stride, - WINPR_ASSERTING_INT_CAST(UINT32, gdi->height), &invalidRegion)) + const BOOL rc = + rfx_process_message(context->codecs->rfx, pSrcData, SrcSize, bitmap->left, + bitmap->top, bitmap->data, bitmap->format, gdi->stride, + WINPR_ASSERTING_INT_CAST(UINT32, gdi->height), &invalidRegion); + region16_uninit(&invalidRegion); + + if (!rc) { WLog_ERR(TAG, "rfx_process_message failed"); return FALSE; From ac4be9b9418c20e06c533bd741150ad3d51f27f6 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sun, 12 Jan 2025 09:46:15 +0100 Subject: [PATCH 07/11] [codec,rfx] fix region leak --- libfreerdp/codec/rfx.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libfreerdp/codec/rfx.c b/libfreerdp/codec/rfx.c index 06937cb89..52d492c86 100644 --- a/libfreerdp/codec/rfx.c +++ b/libfreerdp/codec/rfx.c @@ -1876,6 +1876,8 @@ skip_encoding_loop: WLog_Print(context->priv->log, WLOG_ERROR, "failed"); rfx_message_free(context, message); + region16_uninit(&tilesRegion); + region16_uninit(&rectsRegion); return NULL; } From 621ac46988bc7353fc8985a1807365a03a2a244a Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sun, 12 Jan 2025 10:04:24 +0100 Subject: [PATCH 08/11] [ci,abi] bump base ref to 3.6.0 new github runner with ubuntu 24.04 only support webkit2gtk-4.1 we added support for that with 3.6.0 --- .github/workflows/abi-checker.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/abi-checker.yml b/.github/workflows/abi-checker.yml index 1a5cf75e9..197e94fb4 100644 --- a/.github/workflows/abi-checker.yml +++ b/.github/workflows/abi-checker.yml @@ -6,7 +6,7 @@ on: API_BASE_REF: description: 'Base revision for ABI compatibility check' required: true - default: '3.0.0' + default: '3.6.0' pull_request: branches: [ master, stable* ] schedule: From 2da7b8c606914614ec295d937932ef569c3ea921 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sun, 12 Jan 2025 10:24:20 +0100 Subject: [PATCH 09/11] [primitives,opencl] do not fail on opencl not supported --- libfreerdp/primitives/primitives.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libfreerdp/primitives/primitives.c b/libfreerdp/primitives/primitives.c index 9aa15e446..f968bf9d8 100644 --- a/libfreerdp/primitives/primitives.c +++ b/libfreerdp/primitives/primitives.c @@ -91,6 +91,8 @@ static BOOL CALLBACK primitives_init_generic_cb(PINIT_ONCE once, PVOID param, PV static BOOL primitives_init_optimized(primitives_t* prims) { + primitives_init_generic(prims); + #if defined(HAVE_CPU_OPTIMIZED_PRIMITIVES) primitives_init_add_opt(prims); primitives_init_andor_opt(prims); From 99cfbebe634e6542d6cb90af470ff82ac5d08dd2 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sun, 12 Jan 2025 15:48:22 +0100 Subject: [PATCH 10/11] [primitives] fix documentation --- include/freerdp/primitives.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/freerdp/primitives.h b/include/freerdp/primitives.h index 311c65b21..05f0d6607 100644 --- a/include/freerdp/primitives.h +++ b/include/freerdp/primitives.h @@ -280,7 +280,7 @@ typedef enum * If that does not exist or does not work on the platform any other (e.g. usually pure * software) is returned * - * @param hint the type of primitives to return. + * @param type the type of primitives to return. * @return A primitive implementation matching the hint closest or \b NULL in case of failure. * @since version 3.11.0 */ From 17315c593655d476cccfb9a1b56e41f37030f8e1 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Sun, 12 Jan 2025 15:48:42 +0100 Subject: [PATCH 11/11] [codec,yuv] fix thread count calculation --- libfreerdp/codec/yuv.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libfreerdp/codec/yuv.c b/libfreerdp/codec/yuv.c index bad8b4e8e..abf675903 100644 --- a/libfreerdp/codec/yuv.c +++ b/libfreerdp/codec/yuv.c @@ -171,12 +171,12 @@ BOOL yuv_context_reset(YUV_CONTEXT* WINPR_RESTRICT context, UINT32 width, UINT32 if (context->useThreads) { - const UINT32 pw = (width + TILE_SIZE - width % TILE_SIZE) / TILE_SIZE; - const UINT32 ph = (height + TILE_SIZE - height % TILE_SIZE) / TILE_SIZE; + const size_t pw = (width + TILE_SIZE - width % TILE_SIZE) / TILE_SIZE; + const size_t ph = height + TILE_SIZE - height % TILE_SIZE / TILE_SIZE; - /* We´ve calculated the amount of workers for 64x64 tiles, but the decoder - * might get 16x16 tiles mixed in. */ - const UINT32 count = pw * ph * 16; + /* calculate absolute maximum of work tiles to be used. + */ + const size_t count = MAX((height + context->heightStep) / context->heightStep, pw * ph); context->work_object_count = 0; if (context->encoder) @@ -215,7 +215,7 @@ BOOL yuv_context_reset(YUV_CONTEXT* WINPR_RESTRICT context, UINT32 width, UINT32 memset(wtmp, 0, count * sizeof(PTP_WORK)); context->work_objects = (PTP_WORK*)wtmp; - context->work_object_count = count; + context->work_object_count = WINPR_ASSERTING_INT_CAST(uint32_t, count); } rc = TRUE; fail: