Upstream/h264 avc444 passthrough - #210
Conversation
H.264 passthrough had two independent defects. On Windows hosts it
negotiated no H.264 at all, and where it did engage it delivered about
half the saving it should.
GfxAVC444 = FALSE was set to make the client advertise AVC420-only,
which is not what the combination does. FreeRDP gates the entire RDPGFX
V10+ capability block on
!GfxH264 || GfxAVC444
(rdpgfx_main.c:254, rdpgfx_send_caps_advertise_pdu), so with GfxH264 set
and GfxAVC444 clear no V10 capability set is sent at all -- the client
advertises only V8 and V8.1. Windows will not offer H.264 at V8.1: it
falls back to CLEARCODEC and CAPROGRESSIVE, which guacd then decodes and
re-encodes as JPEG/WebP, exactly the cost the feature exists to avoid.
The display stays correct, so the regression is invisible without
measuring CPU or counting codecs on the wire. Over a minute of 1080p
video on Windows 11 Pro: 4050 CLEARCODEC + 1447 CAPROGRESSIVE + zero
H.264 frames before, all frames codec 15 after. xrdp is unaffected,
which is why this went unnoticed -- it accepts AVC420 at V8.1 and never
needed the V10 capsets.
AVC444 was disabled because forwarding it was broken, not because
AVC420 was wanted: only bitstream[0] reached the client, rendering as a
green/magenta split. Forwarding one view cannot work -- both bitstreams
decode through the same H264_CONTEXT in FreeRDP's avc444_decompress, so
they are one H.264 sequence with alternating views. Dropping either
breaks reference frames for the other, and the decoder reports no error
because what arrives is well formed. Both views are now forwarded,
tagged with a view field on the h264 instruction; the client decodes
both, draws only view 0, and combines the auxiliary view's chroma into
full 4:4:4 in a WebGL2 pass, falling back to 4:2:0 if it cannot.
The GDI decode is now skipped for captured H.264 commands. Previously
the wrapper always called through to the original handler, so only the
re-encode was saved. RemoteFX, planar and progressive still decode
normally, so a server mixing codecs within a frame stays correct.
guacd CPU with 1080p video playing, sampled per-thread over 30s:
xrdp (AVC420) ~100% of a core -> 2.0%
Windows 11 (AVC444) 90.6% of a core -> 2.1%
Configuration is unchanged: the per-connection enable-h264 argument is
the only switch, and there are no environment variables.
Recordings capture the raw protocol stream, so a recording of an H.264 session contains h264 instructions. recordings.html loaded neither H264Decoder.js nor Yuv444.js, so playback of those sessions rendered nothing.
Review notes from the upstream issue thread: - The auxiliary-view snapshot was sized to the auxiliary frame's own dimensions. The v1 chroma layout pads that view to a multiple of 16 rows, so the snapshot could be taller than the picture the renderer draws; with no rects the whole thing is blitted, painting a blank strip below the image. Size it from the rendered canvas instead. - The per-frame watchdog was cleared before combineFrame() ran, so the 4:4:4 path had no timeout despite the comment saying otherwise: a copyTo() that never settled would hold the ordered display queue with nothing able to release it. Clear it once the frame is done with instead -- in the finally for the synchronous paths, and after the copy settles for the combine. - Drop the unreachable view != 0 branch after the combine hand-off. - '0' no longer coerces to false in override(), so ?h264ChromaFilter=0 sets threshold zero rather than switching the filter off. Boolean overrides are unaffected: they coerce, and 0 is falsy. - Drop the stale stats() reference from client.html; the decoder has no such method.
|
I think CI is failing on main, not on this branch — git diff origin/main HEAD -- src Cargo.toml Cargo.lock .github is empty, so there's no Rust in this PR at all. Two pre-existing issues, both time-based: clippy 1.98 flags needless_late_init at tunnel.rs:259/273 and result_large_err at api.rs:2143 (CI installs unpinned stable), and cargo audit hits RUSTSEC-2026-0258 on h2 0.4.15, published five days after v1.9.9. |
|
I built this branch this evening and can confirm it seems to work as intended. On the previous 1.9.9 Rustguac release, a single RDP session to a Windows 11 box doing AVC444 I was seeing the Rustguac server hit 40-50% CPU load. On the client machine I was running Edge, and dev tools confirms WebCodecs::VideoDecoder was showing a h264 session running. Video playback through the Rustguac session was buttery smooth, and no video artifacts present in the session so it appears the two bitstreams are being combined correctly. This looks like a nice win. |
|
I am running a custom patched versions of xrdp and xorgxrdp-glamor-gbm. I'll stand up a standard release versions on a VM and try to reproduce over the next few days. Unfortunately, I am traveling for work at the moment and have limited time until the coming weekend. |
guac_rdp_rdpgfx_channel_connected() installed the SurfaceCommand and
CapsConfirm wrappers unconditionally, every time it ran. It runs again when
the RDPGFX channel reconnects, which xrdp provokes with the display resize it
performs at login, and the second pass saved the wrapper the first pass had
installed:
rdp_client->orig_surface_command = (void*) rdpgfx->SurfaceCommand;
rdpgfx->SurfaceCommand = guac_rdp_gfx_surface_command;
leaving orig_surface_command pointing at guac_rdp_gfx_surface_command itself.
Any surface command that is not H.264 falls through to orig(context, cmd),
re-enters the wrapper, and recurses until the stack is exhausted and the guacd
child dies -- a white screen in the browser, with no "Client terminated
successfully" in the log and the segfault visible only outside a container.
H.264 commands take skip_decode and never reach that call, so a server sending
nothing but H.264 is unaffected. A stock xrdp is the opposite case: one trace
showed 7440 planar and 12 progressive surface commands against 437 AVC420, so
it dies on the first frame. Distro xrdp packages (0.10.1 on Ubuntu) are built
--enable-rfxcodec with no H.264 encoder at all, so they send planar for
everything and hit this immediately.
The wrappers were also installed for every GFX session regardless of
enable_h264, so this reproduced with RFX alone and H.264 unticked -- no
H.264-capable server required.
Both halves are fixed: install only when the connection asked for H.264, and
only when nothing is installed yet. guac_rdp_client is guac_mem_zalloc'd, so
the NULL check is sound on first entry.
|
Found and fixed the white screen in latest commit — it was a crash in the passthrough, not a rendering Root cause
rdp_client->orig_surface_command = (void*) rdpgfx->SurfaceCommand; // already ours
rdpgfx->SurfaceCommand = guac_rdp_gfx_surface_command;so Since the wrappers were installed for every GFX session and Repro without any H.264 server Ubuntu's xrdp 0.10.1 is built I don't currently have a compiled version of stock xrdp with --enable-x264 compiled in. Please give |
3 participants
Fixes the two defects from #208 : the GfxAVC444 gate that leaves Windows hosts negotiating no H.264 at all, and the server-side decode that ran on every frame despite being unused.
Review notes from the issue thread are folded into f16eed7. Built and running against FreeRDP 3 on Debian 13; guacd CPU with 1080p video goes from ~100% of a core to ~2%.
The v1 AVC444 chroma layout is untested — neither xrdp nor the Windows hosts here emit it.