feat(server): add EGFX server integration with DVC bridge#1099
Open
glamberson wants to merge 2 commits intoDevolutions:masterfrom
Open
feat(server): add EGFX server integration with DVC bridge#1099glamberson wants to merge 2 commits intoDevolutions:masterfrom
glamberson wants to merge 2 commits intoDevolutions:masterfrom
Conversation
Contributor
Author
|
I nearly forgot about thsi one... |
There was a problem hiding this comment.
Pull request overview
This PR integrates the EGFX (Graphics Pipeline Extension) server from ironrdp-egfx into ironrdp-server, enabling H.264 AVC420/AVC444 video streaming to RDP clients via the EGFX dynamic virtual channel. The integration builds on PR #1057 which added the EGFX PDU types and server state machine.
Changes:
- Added ZGFX uncompressed segment wrapping for DVC wire format compliance with MS-RDPEGFX 2.2.5
- Implemented auto-send of ResetGraphics PDU before first CreateSurface per MS-RDPEGFX requirements
- Created GfxServerFactory trait and GfxDvcBridge for integrating EGFX with the server DVC infrastructure
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ironrdp-egfx/src/lib.rs | Made CHANNEL_NAME public for cross-crate DVC registration |
| crates/ironrdp-egfx/src/server.rs | Added ZGFX wrapping, auto-ResetGraphics, channel_id tracking, and rewrote drain_output() |
| crates/ironrdp-server/src/lib.rs | Exposed gfx module under egfx feature flag |
| crates/ironrdp-server/src/gfx.rs | New module with GfxServerFactory trait and GfxDvcBridge DVC processor |
| crates/ironrdp-server/src/builder.rs | Added with_gfx_factory() builder method |
| crates/ironrdp-server/src/server.rs | Integrated GfxServerFactory, added ServerEvent::Egfx variant, wired DVC bridge |
| crates/ironrdp-server/Cargo.toml | Added egfx optional feature and ironrdp-egfx dependency |
| crates/ironrdp-testsuite-core/tests/egfx/server.rs | Updated test to expect auto-sent ResetGraphics PDU |
| Cargo.lock | Added ironrdp-egfx dependency to ironrdp-server |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Wire ironrdp-egfx's GraphicsPipelineServer into ironrdp-server, enabling H.264 AVC420/AVC444 video streaming to RDP clients. Changes to ironrdp-egfx: - Make CHANNEL_NAME public for cross-crate DVC registration - Add ZGFX uncompressed segment wrapping for DVC wire format - Auto-send ResetGraphics before first CreateSurface (MS-RDPEGFX) - Track DVC channel_id from start() for proactive frame encoding - Rewrite drain_output() to ZGFX-wrap PDUs for DVC transmission New ironrdp-server gfx module (behind "egfx" feature): - GfxServerFactory trait following CliprdrServerFactory pattern - GfxDvcBridge: Arc<Mutex> wrapper enabling shared access between DVC message processing and proactive frame submission - ServerEvent::Egfx variant for routing EGFX PDUs to the wire - Builder integration with with_gfx_factory()
- Remove unused channel_id from EgfxServerMessage::SendMessages - Make GfxServerFactory extend ServerEventSender for event loop signaling - Store GfxServerHandle in RdpServer instead of discarding it - Replace local wrap_zgfx_uncompressed with ironrdp_graphics::zgfx::wrap_uncompressed
797ee1f to
96a8c7d
Compare
Contributor
Author
|
Great. issues resolved and rebased. Thanks. |
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.
Summary
Wire
ironrdp-egfx'sGraphicsPipelineServerintoironrdp-server, enabling H.264 AVC420/AVC444 video streaming to RDP clients via the EGFX dynamic virtual channel.This builds on #1057 (merged) which added the EGFX PDU types and server state machine. This PR adds the integration layer that connects it to the server connection lifecycle.
Changes to ironrdp-egfx
CHANNEL_NAMEpublic for cross-crate DVC registrationResetGraphicsbefore firstCreateSurfaceper MS-RDPEGFX requirementstart()for proactive frame encodingdrain_output()to ZGFX-wrap PDUs for DVC transmissionNew ironrdp-server gfx module (behind
egfxfeature)GfxServerFactorytrait following theCliprdrServerFactory/SoundServerFactorypatternGfxDvcBridge:Arc<Mutex>wrapper enabling shared access between DVC message processing and proactive frame submission from display handlersServerEvent::Egfxvariant for routing EGFX PDUs to the wirewith_gfx_factory()Design notes
std::sync::Mutex(not tokio) becauseDvcProcessortrait methods are synchronousegfxfeature is optional and off by default, so no impact on existing usersTest plan
cargo fmt --all -- --check: cleancargo clippy --workspace --all-targets --features helper,__bench -- -D warnings: cleancargo clippy -p ironrdp-egfx -p ironrdp-server --features egfx -- -D warnings: cleancargo test --workspace: 1058 passed, 0 failedtest_surface_lifecycleto account for auto-ResetGraphics PDU