fix(egfx): zero cbAvc420EncodedBitstream1 for chroma-only AVC444 streams

[Greg Lamberson (glamberson)]

Avc444BitmapStream::encode always writes the length of the first sub-stream into cbAvc420EncodedBitstream1. MS-RDPEGFX 2.2.4.5 and 2.2.4.6 define that field as the size of the YUV420 frame carried in avc420EncodedBitstream1 and say it MUST be set to zero when no YUV420 frame is present. With LC set to 2 the first sub-stream carries only the Chroma420 view, so the field has to be zero, and today the encoder writes the sub-stream length instead. That reaches the wire through send_avc444v2_frame and through anything else that serializes a chroma-only stream.

The fixtures date from 2022 (#47) and differ only in byte 0, and #47 doesn't say where the bytes came from. AVC_444_MESSAGE_INCORRECT_LEN held LC 2 with a zero size, and the serialization test pinned the other one. The spec asks for zero, so I renamed the fixtures after what they contain and pointed the serialization test at the zero-size form. Both decode tests stay, because the decoder has to keep accepting the non-zero form that the encoder wrote until now.

LC 0 and LC 1 are unchanged, since both carry a YUV420 frame in the first sub-stream. I read FreeRDP's client parser (rdpgfx_decode_AVC444 in channels/rdpgfx/client/rdpgfx_codec.c, 3.32.0), which uses the field only when LC is 0, but I haven't run a client against the zero form. The point of the change is to put on the wire what 2.2.4.5 defines, before chroma-only updates get used more widely.

The encoder also rejects a second sub-stream unless LC is 0, and LC 0 without one, because with the first length zero a receiver would read a stray second sub-stream as part of the first. The server's sender already refuses those shapes, so only a hand-built value reaches the check.

The xtask fmt, lints, tests, typos and locks checks pass. The existing AVC444v2 server test now also reads the raw stream info word for all three LC values, and three new tests cover the second sub-stream rule.

[github-actions]

The PR correctly makes Avc444BitmapStream::encode write zero into cbAvc420EncodedBitstream1 when LC=CHROMA, matching MS-RDPEGFX 2.2.4.5/2.2.4.6 (no YUV420p frame in the first sub-stream), while leaving LC=0/1 unchanged; the decoder still accepts the non-zero form some servers send. The fixture rename is faithful: the two 88-byte arrays differ only in byte 0 (0x00 vs 0x54, the 84-byte stream1 size), and the serialization test now pins the spec-compliant zero-length form, byte-identical to the fixed encoder output (stream_info 0x80000000). Wire impact is confined to the CHROMA path via send_avc444v2_frame; the v1 path maps no-chroma to LUMA and is unchanged. No protocol or correctness defects found; one low-severity optional test-duplication cleanup from code-compressor is accepted, its fixture-derivation suggestion is rejected.

[github-actions]

The PR correctly zeroes cbAvc420EncodedBitstream1 for CHROMA (LC=2) streams per MS-RDPEGFX 2.2.4.5/2.2.4.6, in the right place (Encode for Avc444BitmapStream), leaving LC 0/1 untouched. Verified that the decoder already accepts both the zero-length and declared-length forms, preserving round-trips and keeping compatibility with servers (e.g. FreeRDP) that send the non-zero form; fixture arithmetic (84 = 0x54) confirms the renamed constants are consistent. Three low-severity follow-ups remain: the encode/decode asymmetry for stream2 under LC 1/2, a v2 test comment citing the v1 spec section, and 84 duplicated fixture bytes. The interop concern about changing wire output is resolved by spec conformance but warrants documenting the interoperability basis.

1. [protocol] Encoder still emits avc420EncodedBitstream2 under LC values that forbid it — low 🟡 — crates/ironrdp-egfx/src/pdu/avc.rs
   MS-RDPEGFX 2.2.4.5/2.2.4.6 define LC 0x1 and 0x2 as carrying no second sub-stream, yet encode serializes stream2 whenever it is Some and size() counts it, while decode drops stream2 for those LC values and folds trailing bytes into stream1.data when cbAvc420EncodedBitstream1 is zero. The new CHROMA cb=0 emission makes any such output actively corrupting: a conformant receiver cannot tell an appended stream2 apart from the Chroma420 H.264 data. Pre-existing on lines not added by this PR, but adjacent to the changed code; consider enforcing stream2.is_none() (or warning) for LC 1/2 during encode.
2. [code-compressor] Derive AVC_444_CHROMA_MESSAGE_WITH_LEN instead of duplicating 84 fixture bytes — low 🟡 — crates/ironrdp-testsuite-core/src/graphics_messages.rs
   The two renamed 88-byte fixtures are identical except byte 0 (0x00 vs 0x54, the LE cbAvc420EncodedBitstream1 of 84; stream1.size() = 4 + 10 + 70). Two independent literals mean future edits to the shared H.264 payload must be applied twice or the decoder-compat and round-trip tests silently diverge. A const block deriving WITH_LEN from AVC_444_CHROMA_MESSAGE and setting byte 0 to 84 keeps the compared bytes exact and makes the 'same stream with length filled in' relationship executable; name or comment the 84 constant since 0x54 would no longer be visible.

[Greg Lamberson (glamberson)]

The item in the review of 2026-09-30 about avc420EncodedBitstream2 under LC 1 and 2 was right, and the zero length for chroma-only streams is what made it matter. The encoder wrote stream2 whenever it was set, and with the first length zero a receiver takes everything after the stream info as the first sub-stream, so a stray second sub-stream would have corrupted the H.264 data. Encode now returns an invalid field error for stream2 unless it's present exactly when the encoding is LUMA_AND_CHROMA, which also covers LC 0 without a second sub-stream. The check runs before anything is written, and the field carries the invariant. The server's sender already refused those shapes, so only a hand-built value could reach it. Three tests pin the two invalid directions and an LC 0 round trip.

[Greg Lamberson (glamberson)]

I'm correcting a claim from the PR body, after checking FreeRDP's source. I had said FreeRDP's shadow server writes the non-zero form. In 3.32.0 it writes a non-zero length for chroma-only frames, but only because it sends an empty first sub-stream and no chroma data, so it's no evidence for that form and I'm dropping the claim. What I did verify is the client's behavior, in rdpgfx_decode_AVC444 in channels/rdpgfx/client/rdpgfx_codec.c, which uses the length only when LC is 0 and takes the rest of the command as the first sub-stream otherwise. The encoder comment now names that function and version, and the fixture comment no longer says some servers send the non-zero form. The body will say the decoder keeps accepting it because the encoder wrote it until now. I haven't run a client against the zero form.

[Greg Lamberson (glamberson)]

On the item in the review of 2026-09-30 about deriving AVC_444_CHROMA_MESSAGE_WITH_LEN from the other fixture, I kept both arrays as literals and added a doc line instead. The tests already catch a divergence, because the decode test for the non-zero form compares against a value whose data is sliced from the zero array, so changing the payload of only one array fails it (I tried both directions). A derived array would also hide the actual bytes of the non-zero form, and no other fixture in that file is built from another. The doc now says that only byte 0 differs and that it holds the length of the first sub-stream, which is 84 bytes (4 for the region count, 10 for the rectangle and quality values, and 70 of H.264).

[Greg Lamberson (glamberson)]

I pushed the changes described in the comments above as one commit rebased on current master, and updated the body to match.

[Benoît Cortier (CBenoit)]

PR automation is failing because of a picky-krb 0.12.5 incompatibility, fixed on master by #2074. Please rebase on master to fix it.

Update: no rebase needed anymore. picky-krb 0.12.5 was yanked from crates.io (re-released as 0.13.0), so the API check builds again without changes to this branch. PR automation has been re-run here and passes.

[github-actions]

This pull request may overlap with #2002.

Both touch the AVC444v2 LC/sub-stream wire shape in ironrdp-egfx: this PR tightens Avc444BitmapStream encode (stream2 presence invariant, zero stream1 size under CHROMA) and updates the avc444v2 sender wire-shape test, while #2002 adds AVC444/444v2 tiles to mixed-codec frames using the same LC encoding, first and optional second sub-stream structure, and the send_avc444v2_frame path that already validates this stream shape.

This notice is advisory only. Automated review continues as usual, and how these pull requests relate is for maintainers and authors to decide.

Note

LLM-assisted content (no human feedback).

[Benoît Cortier (CBenoit)]

LGTM