fix(graphics): don't require CONTEXT block on every progressive frame#1395
Merged
Benoît Cortier (CBenoit) merged 1 commit intoJul 1, 2026
Conversation
Per MS-RDPEGFX 2.2.4.2 the SYNC + CONTEXT blocks establish a codec
context once (keyed by codec_context_id) and are not repeated on
subsequent frames that reference the same context. The decoder's strict
requirement that CONTEXT be present on every decode_bitmap call rejected
every frame after the first with MissingBlock("CONTEXT"), freezing the
image on the coarse first pass.
Real-world servers (xrdp, GNOME Remote Desktop) omit the CONTEXT block
on every frame after the first one that established the context. Fall
back to the value stored when the context was first created; only error
when the very first frame for a context arrives without CONTEXT.
Benoît Cortier (CBenoit)
approved these changes
Jul 1, 2026
Benoît Cortier (CBenoit)
left a comment
Benoît Cortier (CBenoit)
left a comment
Member
There was a problem hiding this comment.
LGTM, thank you!
Member
|
question: I just noticed: on the surface-resize branch, a CONTEXT-less resize frame now reallocates using the fallback flag and discards accumulated tile state. Pre-existing, but worth confirming that's acceptable for the resize + omitted-CONTEXT combination.
I won't block this PR on this, but can we get the two-frame regression fixture you offered? Frame 1 with CONTEXT establishing a |
There was a problem hiding this comment.
Pull request overview
This PR fixes progressive RemoteFX (MS-RDPEGFX) decoding by no longer requiring a CONTEXT block on every WireToSurface2 progressive frame once a codec context has already been established (keyed by codec_context_id). This aligns the decoder with real-world server behavior and the spec’s “establish once, then reference” model for progressive contexts.
Changes:
- Allow
decode_bitmapto fall back to the previously-established context’suse_reduce_extrapolatevalue when the current frame omitsCONTEXT. - Preserve the existing strict error when the very first frame for a context arrives without a
CONTEXTblock.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1079
to
+1102
| // Extract the band-layout flag from the CONTEXT block when present. | ||
| // Per MS-RDPEGFX 2.2.4.2 the SYNC + CONTEXT blocks establish a codec | ||
| // context once (keyed by `codec_context_id`) and are not required to be | ||
| // repeated on subsequent frames that reference the same context. | ||
| // Real-world servers (xrdp, GNOME Remote Desktop) omit the CONTEXT | ||
| // block on every frame after the first one that established the | ||
| // context. The strict requirement rejected each of those frames with | ||
| // `MissingBlock("CONTEXT")`, freezing the image on the coarse first | ||
| // pass. | ||
| // | ||
| // Fall back to the value stored when the context was first created. | ||
| // Only error when neither source is available, i.e. the very first | ||
| // frame for a context arrived without a CONTEXT block. | ||
| let use_reduce_extrapolate = match blocks.iter().find_map(|block| match block { | ||
| ProgressiveBlock::Context(ctx) => Some(ctx.uses_reduce_extrapolate()), | ||
| _ => None, | ||
| }) { | ||
| Some(v) => v, | ||
| None => self | ||
| .contexts | ||
| .get(&codec_context_id) | ||
| .map(|c| c.surface.use_reduce_extrapolate) | ||
| .ok_or(ProgressiveDecodeError::MissingBlock("CONTEXT"))?, | ||
| }; |
David T. Martel (David-Martel)
added a commit
to David-Martel/IronRDP
that referenced
this pull request
Jul 4, 2026
…render GRD GNOME Remote Desktop 46 delivers graphics as the RemoteFX Progressive codec inside RDPGFX_WIRE_TO_SURFACE_PDU_2, but the client's on_wire_to_surface2 was a no-op, so no pixels ever reached the framebuffer. Decode and composite those frames so a live GRD session renders. Ported (not reimplemented) from upstream Devolutions/IronRDP, which already had this capability the fork was missing. Brought in as file-level `git checkout` (no merge/cherry-pick; the fork reworked egfx/rfx and server-side Devolutions#1197 changes conflict and are not needed for client decode): - ironrdp-pdu rfx/progressive.rs: progressive block-stream parser (SYNC/CONTEXT/FRAME/REGION/TILE_SIMPLE|FIRST|UPGRADE). From Devolutions#1196/Devolutions#1197. - ironrdp-graphics dwt_extrapolate.rs + srl.rs: reduce-extrapolate DWT and SRL primitives for progressive refinement. From Devolutions#1196. - ironrdp-graphics progressive.rs: ProgressiveDecoder with per-codec_context_id tile state; SIMPLE full-quality and FIRST/UPGRADE refinement. From Devolutions#1197. Adapted alloc::collections -> std::collections (fork's graphics is std). Applied the Devolutions#1395 fix: GRD omits the CONTEXT block on every frame after the first, so cache use_reduce_extrapolate per context; without this only the coarse first frame renders and the image freezes (MissingBlock("CONTEXT")). Wired into GraphicsPipelineClient::handle_wire_to_surface2: decode tiles, blit into a persistent per-surface RGBA framebuffer, and deliver the full framebuffer via on_bitmap_updated (the renderer Image path is full-frame, shared with the AVC420/uncompressed paths). Progressive state resets on ResetGraphics; context freed on DeleteEncodingContext. A one-shot framebuffer dump is gated behind IRONRDP_EGFX_DUMP for visual verification. Stripped the `arbitrary` fuzz derives (the fork's pdu crate has no such feature) and satisfied the fork's stricter clippy config. Verified against live GRD 46.3 (100.64.0.3): first frame composited 510 tiles (full 1920x1080), dumped and visually confirmed as the Ubuntu GDM login screen with correct colors across all channels; subsequent 1-tile incremental frames decoded with no CONTEXT block and no errors; session stayed up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
David T. Martel (David-Martel)
added a commit
to David-Martel/IronRDP
that referenced
this pull request
Jul 5, 2026
Document evidence-based dispositions from a targeted re-scan of master..upstream/master (139 commits, upstream HEAD 069786c): - IMPORTED Devolutions#1236 (connector: name actual PDU on unexpected Share Control PDU); gates green (workspace 1348/0, release client build, clippy clean). - AVC444 decode and ARC reconnect-cookie: no upstream source exists (client.rs:720 stubs AVC444; rdp.rs:774 TODOs the cookie) — both stay DEFERRED/from-scratch. Corrected the reconnect-cookie note: the SecurityVerifier is HMAC-MD5(ArcRandomBits, ClientRandom), not a copy; gate any future impl behind an opt-in --auto-reconnect flag. - dirty-rect stays from-scratch/verify-gated (fork's own app.rs/rdp.rs). - gap-4 autodetect: importable part already in the fork; Devolutions#1178 handles a different framing and not the handover re-advertise blocker (no upstream fix) — SKIPPED. - Devolutions#1395 already present in the fork (progressive.rs:835). Devolutions#1132/Devolutions#1174/Devolutions#1305 skipped with reasons. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a server sends a WireToSurface2 PDU for a codec context that's already been established, the CONTEXT block is often omitted. MS-RDPEGFX 2.2.4.2 specifies SYNC + CONTEXT as the establishment sequence for a context (keyed by codec_context_id); frames after that point reference the existing context and don't carry a fresh CONTEXT.
The current decoder treats CONTEXT as mandatory on every decode_bitmap call. Servers that follow the spec and omit it on subsequent frames get rejected with MissingBlock("CONTEXT"), and the image is stuck on the coarse first pass for the rest of the session.
Confirmed against real servers: xrdp and GNOME Remote Desktop both send the CONTEXT block on the first frame only. Reproducing it requires a multi-frame progressive stream from one of those servers, so there's no test in this PR. The fix has been verified end to end against both xrdp and GNOME Remote Desktop.
The change is small. If the current frame has a CONTEXT block, use it (unchanged). If not, fall back to the use_reduce_extrapolate value stored when the context was first created. Only when neither is available, i.e. the very first frame for a context arrived without a CONTEXT block, do we still error out.
Relates to the progressive codec introduced in #1197 / #1198. Open to adding a regression test if there's a preferred way to fixture a multi-frame stream here, or if maintainers want me to generate one from FreeRDP's vectors.