Skip to content

feat(replay): route captured EGFX output - #1697

Merged
Marc-André Moreau (mamoreau-devolutions) merged 3 commits into
masterfrom
copilot/route-captured-egfx-output
Aug 18, 2026
Merged

feat(replay): route captured EGFX output#1697
Marc-André Moreau (mamoreau-devolutions) merged 3 commits into
masterfrom
copilot/route-captured-egfx-output

Conversation

@mamoreau-devolutions

Copy link
Copy Markdown
Contributor

Attach a passive EGFX client to recorded graphics DVCs so replay can composite completed output into payload-free frame exports.

Preserve captured DVC IDs and names while suppressing replay-side capability advertisements and frame acknowledgements.

Copilot AI balanced review requested due to automatic review settings August 18, 2026 13:05
@github-actions github-actions Bot added kind/protocol Affects RDP or related protocol behavior maintainer-required Maintainer review or intervention is required risk/medium Behavioral change that does not substantially alter a core public API size/L Size: up to 899 counted lines and 20 files; exceeds M in either measure labels Aug 18, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds passive EGFX replay support to composite captured graphics output into exported frames.

Changes:

  • Detects recorded EGFX dynamic channels and preserves their IDs/names.
  • Composites completed EGFX updates while suppressing negotiation and acknowledgements.
  • Adds EGFX replay tests and dependency metadata.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
crates/ironrdp-capture-replay/src/routing.rs Adds passive EGFX routing, compositing, and tests.
crates/ironrdp-capture-replay/Cargo.toml Adds the EGFX dependency.
Cargo.lock Records the dependency update.

Comment thread crates/ironrdp-capture-replay/src/routing.rs Outdated
@github-actions github-actions Bot added risk/low Self-contained change with no cross-crate behavioral effect ai-reviewed/1 One automated review completed and removed risk/medium Behavioral change that does not substantially alter a core public API maintainer-required Maintainer review or intervention is required labels Aug 18, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR wires a passive ironrdp-egfx client into the capture-replay router to composite RDPGFX output into frame exports, reusing the existing DVC-attachment pattern and the tool's explicit-gap convention (adding ReplayGapKind::Unsupported for AVC420). Scope is tight and the new ReplayFramebuffer does careful checked arithmetic. One correctness issue survives review: ReplayFramebuffer::reset reuses the pre-existing MAX_DESKTOP_DIM (8192) primary-desktop bound to validate EGFX ResetGraphics dimensions, but MS-RDPEGFX permits up to 32766, so a legitimate large/multi-monitor capture silently clears the framebuffer and drops every subsequent EGFX frame with no ReplayGap recorded, contradicting the explicit-gap pattern this very PR establishes for AVC420.

Protocol analysis: partially_accepted — Confirmed via ResetGraphicsPdu::decode that width/height are valid up to 32766, while ReplayFramebuffer::reset rejects anything above the reused MAX_DESKTOP_DIM=8192, silently clearing the buffer with no gap recorded. Upgrades the handoff's 'incomplete_evidence' item to a confirmed blocking finding. The second item (mislabeling another channel's failure) is not supported: DVC dispatch routes one Data PDU to exactly one channel per call and propagates its Err immediately, so the flag is only observable in the same call/channel that set it, requiring multiple simultaneous EGFX DVCs that MS-RDPEGFX's single-channel model does not anticipate; downgraded to non-blocking.

  1. non_blocking / low — crates/ironrdp-capture-replay/src/routing.rs
    take_unsupported_egfx_codec() reports true if any tracked EGFX channel's unsupported_avc420 flag is set, without confirming that the channel which just failed stage.process() is the one that raised it. Given the current DVC dispatch (one channel processed per drdynvc Data PDU, error propagated in the same call that sets the flag), this cannot currently misfire in practice, but the check is structurally decoupled from the failing channel_id, so it would silently mislabel a genuine StaticChannel failure as Unsupported if the DVC dispatch or EGFX channel model ever changes to allow overlapping processing.

Comment thread crates/ironrdp-capture-replay/src/routing.rs
@github-actions github-actions Bot added maintainer-required Maintainer review or intervention is required risk/medium Behavioral change that does not substantially alter a core public API and removed risk/low Self-contained change with no cross-crate behavioral effect labels Aug 18, 2026
Attach a passive EGFX client to recorded graphics DVCs so replay can composite completed output into payload-free frame exports.

Preserve captured DVC IDs and names while suppressing replay-side capability advertisements and frame acknowledgements.
Report captured AVC420 graphics as an unsupported replay gap.\n\nAvoid silently omitting frames while keeping H.264 decoding outside replay.
Accept captured EGFX output dimensions through the protocol limit.\n\nRecord an unsupported replay gap when an output snapshot exceeds\nthe compositor-backed memory budget.
@mamoreau-devolutions
Marc-André Moreau (mamoreau-devolutions) merged commit 39cba1e into master Aug 18, 2026
43 checks passed
@mamoreau-devolutions
Marc-André Moreau (mamoreau-devolutions) deleted the copilot/route-captured-egfx-output branch August 18, 2026 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-reviewed/1 One automated review completed kind/protocol Affects RDP or related protocol behavior maintainer-required Maintainer review or intervention is required risk/medium Behavioral change that does not substantially alter a core public API size/L Size: up to 899 counted lines and 20 files; exceeds M in either measure

Development

Successfully merging this pull request may close these issues.

2 participants