Bug 503474 - KRDP uses deprecated AVC420 with modern RDP clients
Summary: KRDP uses deprecated AVC420 with modern RDP clients
Status: RESOLVED NOT A BUG
Alias: None
Product: KRdp
Classification: Plasma
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Unassigned bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-04-28 11:08 UTC by armin.novak
Modified: 2025-09-03 12:36 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
patch in support for AVC444 (5.08 KB, patch)
2025-04-28 19:40 UTC, armin.novak
Details

Note You need to log in before you can comment on or make changes to this bug.
Description armin.novak 2025-04-28 11:08:47 UTC
Reference: https://invent.kde.org/plasma/krdp/-/issues/22

SUMMARY

krdp is using a deprecated mode to send graphics data

AVC420 was only ever used with windows 8.1, all more modern systems use AVC444
pack the H264 data in such a frame if you don't happen to have a windows 8.1 client connecting.

also look at `FreeRDP` shadow server, that one does handle all supported formats for GFX depending on capabilities of the connecting client.
Comment 1 armin.novak 2025-04-28 19:40:45 UTC
Created attachment 180745 [details]
patch in support for AVC444

Patch for AVC444 frame support 

Determine frame format to be used for sending from the capabilities used in CapsConfirm:

1. If RDPGFX_CAPVERSION_81 use AVC420
2. If RDPGFX_CAPVERSION_102, RDPGFX_CAPVERSION_101 or RDPGFX_CAPVERSION_100 use AVC444v1
3. If >= RDPGFX_CAPVERSION_103 use AVC444v2
Comment 2 TraceyC 2025-04-29 00:57:30 UTC
Thanks for the patch! The KDE bug tracker is for bugs rather than development work, however. The issue you filed should be here in Bugzilla. 

The next step is to create a merge request with your patch. That *is* done on Invent. In the MR, please add "Fixes: Bug xxx" so it gets linked to this report.
https://community.kde.org/Infrastructure/GitLab#Submitting_a_merge_request

If you want to discuss the development work, please reach out in our Matrix channel for new contributors
https://community.kde.org/Get_Involved#New_Contributor?_Say_Hello_In_Matrix

If you need information on getting started with KDE development, please see
[https://community.kde.org/Get_Involved/development](https://community.kde.org/Get_Involved/development)

Thanks again.
Comment 3 armin.novak 2025-04-29 06:23:48 UTC
sorry, this IS a bug, it violates the RDP specs!
ONLY windows 8.1 capability supports that mode, but krdp does use it even with more moderen ones!
Comment 4 Arjen Hiemstra 2025-04-29 09:15:03 UTC
The RDGFX extension specification does not state that the server MUST send YUV444 frames. In fact, for `RDPGFX_CAPSET_VERSION104` and up, it explicitly states that the `RDPGFX_CAPS_FLAG_AVC_DISABLED` is defined as:

> If this flag is set, it indicates that usage of the MPEG-4 AVC/H.264 Codec in any mode is not supported in the 
> RDPGFX_WIRE_TO_SURFACE_PDU_1 (section 2.2.2.1) message. If the flag is not set, the client MUST be capable of processing:
> 1. The MPEG-4 AVC/H.264 Codec in YUV444 mode in the RDPGFX_WIRE_TO_SURFACE_PDU_1 message.
> 2. The MPEG-4 AVC/H.264 Codec in YUV420 mode in the RDPGFX_WIRE_TO_SURFACE_PDU_1 message in the same frame as other codecs.

Note point 2 here, which strongly implies that servers are fine to send YUV420, though it is never specified what *servers* are supposed to do, only *clients*. For earlier CAPSETs, the specification isn't very clear, only stating that if not disabled, the client is required to support decoding YUV444. The current code even has a more explicit interpretation and only supports the CAPSETs where YUV420 is explicitly supported.

Now, the entire reason we send YUV420 is that we prefer using hardware encoding for the video stream, which guarantees the highest throughput for video. However, hardware encoders often do not support encoding YUV444, only YUV420. For example, this is the output of `vainfo` for my AMD gpu:

> VAProfileH264ConstrainedBaseline/VAEntrypointEncSlice
>    VAConfigAttribRTFormat                 : VA_RT_FORMAT_YUV420
>    VAConfigAttribRateControl              : VA_RC_CBR
>                                             VA_RC_VBR
>                                             VA_RC_CQP
>                                             VA_RC_QVBR
>    VAConfigAttribEncPackedHeaders         : VA_ENC_PACKED_HEADER_SEQUENCE
>                                             VA_ENC_PACKED_HEADER_PICTURE
>                                             VA_ENC_PACKED_HEADER_SLICE
>                                             VA_ENC_PACKED_HEADER_MISC
>                                             VA_ENC_PACKED_HEADER_RAW_DATA
>    VAConfigAttribEncMaxRefFrames          : l0=1
>                                             l1=1
>    VAConfigAttribEncMaxSlices             : 128
>    VAConfigAttribEncSliceStructure        : VA_ENC_SLICE_STRUCTURE_ARBITRARY_MACROBLOCKS
>                                             VA_ENC_SLICE_STRUCTURE_EQUAL_ROWS
>    VAConfigAttribMaxPictureWidth          : 4096
>    VAConfigAttribMaxPictureHeight         : 4096
>    VAConfigAttribEncQualityRange          : number of supported quality levels is 32
>    VAConfigAttribEncIntraRefresh          : VA_ENC_INTRA_REFRESH_ROLLING_COLUMN
>                                             VA_ENC_INTRA_REFRESH_ROLLING_ROW
>                                             VA_ENC_INTRA_REFRESH_P_FRAME
>    VAConfigAttribEncROI                   : num_roi_regions=32
>                                             roi_rc_priority_support=0
>                                             roi_rc_qp_delta_support=1
>   VAConfigAttribEncRateControlExt        : max_num_temporal_layers_minus1=3 temporal_layer_bitrate_control_flag=1
>   VAConfigAttribMaxFrameSize             : max_frame_size=1
>                                             multiple_pass=0

Note the bit where it says "VAConfigAttribRTFormat: VA_RT_FORMAT_YUV420" and no other formats. So even if we wanted to support YUV444. it would mean forcing software encoding which is not a good tradeoff.
Comment 5 armin.novak 2025-04-29 14:18:01 UTC
> The RDGFX extension specification does not state that the server MUST send YUV444 frames. In fact, for `RDPGFX_CAPSET_VERSION104` and > up, it explicitly states that the `RDPGFX_CAPS_FLAG_AVC_DISABLED` is defined as:

yes, and `AVC444` is supported for all of the 10.x caps whereas `AVC420` is not.

>Now, the entire reason we send YUV420 is that we prefer using hardware encoding for the video stream, which guarantees the highest >throughput for video. However, hardware encoders often do not support encoding YUV444, only YUV420. For example, this is the output of `>vainfo` for my AMD gpu:

off topic for this issue.
we're talking about frame format here. If you read the spec for AVC444 you will realize there is no H264 YUV444 involved, only YUV420 on the content side. (2 YUV420 frames with can be recombined to a full YUV444 image), but as said, off topic for *this* issue

you can just check the patch, it is just wrapping the data in the corresponding container.

As a sidenote: The AVC420 mode is not used *at all* for windows 10 and derivates (at least not to my knowledge), so this is the same as other legacy paths that tend to be buggy.
you also fail to implement the required progressive codec that is required by quite a lot of clients up to this date.
Comment 6 armin.novak 2025-04-29 14:23:37 UTC
sorry, forgot the references:
* [MS-RDPEGFX] 2.2.4.6 RFX_AVC444V2_BITMAP_STREAM describes the frame types supported by AVC444 mode (check the LC member)
* [MS-RDPEGFX] 2.2.4.6 RFX_AVC444V2_BITMAP_STREAM is a legacy version of the AVC444 mode that was shortly used for the first windows 10 versions. While for your use case (only LUMA frames) there is no difference, the chroma frame format differs and leads to bugs in implementations due to YUV alignments
Comment 7 TraceyC 2025-04-29 20:30:23 UTC
(In reply to armin.novak from comment #3)
> sorry, this IS a bug, it violates the RDP specs!

The patch still needs to be submitted via a merge request on Gitlab, not to our bug tracker.  Thanks for your understanding.