feat(replay): route captured EGFX output - #1697
feat(replay): route captured EGFX output#1697Marc-André Moreau (mamoreau-devolutions) merged 3 commits into
Conversation
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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.
- 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.
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.
2d6969a to
facc3be
Compare
39cba1e
into
master
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.