fix(egfx): preserve AVC_DISABLED during negotiation#1455
Open
digyear wants to merge 1 commit into
Open
Conversation
Treat AVC_DISABLED as a client-side decoder constraint rather than a mutually supported feature. Preserve it in CapsConfirm for every capability version that defines the flag while retaining intersection semantics for positive flags.
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
Preserve the client-provided
AVC_DISABLEDflag when negotiating EGFX capability sets while continuing to intersect normal positive feature flags.The fix covers every typed capability version that defines
AVC_DISABLED:Why
AVC_DISABLEDis different from ordinary capability bits: it describes a client-side decoder constraint. A client sets it to state that AVC420/AVC444 decoding is unavailable.The current negotiation code treats every flag as a mutually supported positive feature and computes:
A server normally does not advertise that its own AVC decoder is disabled, so this operation removes the client's constraint from
CapabilitiesConfirm. The negotiated state can then report AVC support even though the client explicitly disabled it.This was reproduced with Microsoft Remote Desktop for Android advertising a V10.4 capability set containing
AVC_DISABLED | SMALL_CACHE. After the flag was removed during confirmation, the server and client disagreed about the available codec path. Preserving the constraint keeps the negotiated state faithful to the client and allows the server to select a non-AVC path such as Planar.The relevant capability definitions are documented in MS-RDPEGFX:
Implementation
For capability versions containing
AVC_DISABLED, negotiation now computes:This preserves only the negative client constraint. All other flags retain the existing intersection behavior.
Validation
Added unit tests that verify:
AVC_DISABLEDsurvives negotiation for every supported capability version that defines itCommands run:
cargo test -p ironrdp-egfx capability_negotiation_testscargo check -p ironrdp-egfxcargo fmt --all -- --checkgit diff --checkThe V10.4 behavior was also validated downstream with Microsoft Remote Desktop for Android through
lamco-rdp-server.Relationship to Planar support
This fix is independent of, but complementary to, #1454. That PR exposes a Planar frame sender; this PR ensures an AVC-disabled client remains identified as such after capability negotiation.