feat(server): add ironrdp-server integration for AUDIO_INPUT - #1946
Open
Greg Lamberson (glamberson) wants to merge 2 commits into
Open
Greg Lamberson (glamberson) wants to merge 2 commits into
Greg Lamberson (glamberson) wants to merge 2 commits into
Conversation
Greg Lamberson (glamberson)
temporarily deployed
to
llm-providers
September 10, 2026 18:06 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
github-actions
Bot
added
breaking-change
Contributor
|
Potential duplicate detected: #1945. Candidate Maintainer review is required. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Verified the AUDIO_INPUT server state machine and ironrdp-server wiring against the PR head. The MS-RDPEAI sequencing, the AAC/v1 FormatChange SHOULD-NOT guard, and client format filtering are sound, and the factory/builder/ServerEvent plumbing mirrors existing rdpdr/rdpsnd/rdpei conventions. Eight candidate findings were independently confirmed: malformed AUDIO_INPUT packets and anticipated open()/change_format() state races escalate to connection-fatal errors; a client FormatChange with an out-of-range index permanently disables audio delivery; an OpenReply arriving without the initial FormatChange (which IronRDP's own client sends on Open rejection) permanently wedges the channel; OpenReply success uses S_OK equality instead of HRESULT success semantics; server-initiated DVC data uses empty flags diverging from every sibling path; and two small pieces of redundant state/control flow exist. All eight candidates are accepted with line ranges verified against the head tree.
github-actions
Bot
added
ai-reviewed/1
Depends on the RdpeaiServer orchestration this stacks on. Wires it into ironrdp-server the same way RdpdrServer's follow-up wiring did, matching RdpeiServerFactory's shape: - RdpeaiServerFactory (ironrdp-server/src/rdpeai.rs), a thin factory trait mirroring RdpdrServerFactory/SoundServerFactory: build_backend() returns the RdpeaiServerBackend the embedding application supplies. - attach_channels registers RdpeaiServer on the DVC stack alongside AInput/DisplayControl/Echo/RdpeiServer when a factory is configured. - RdpServerBuilder::with_rdpeai_factory(), following with_rdpdr_factory's shape exactly. - ServerEvent::Rdpeai(RdpeaiServerMessage) lets the embedding application trigger open()/change_format() from outside the DVC message loop, since RdpeaiServer is nested inside DrdynvcServer rather than being its own top-level channel: the dispatch arm reaches it via DrdynvcServer::dvc_by_id_mut, the same path Echo's dispatch uses to reach EchoDvcBridge, then wraps the resulting PDUs with dvc::encode_dvc_messages before writing them. rdpei/rdpeai are legitimately distinct protocols (touch/pen input vs. audio input) whose names happen to be textually close; the constructor's clippy::similar_names is expected and annotated rather than worked around by renaming either.
…y and HRESULT gaps The ServerEvent::Rdpeai dispatch arm escalated open()/change_format() state rejections to a connection-fatal error via map_err_kind, even though open()'s own doc comment documents that callers may legitimately race negotiation completing. Every other unavailability in the same arm (missing drdynvc, missing channel, channel not opened) is drop-and-warn; this now matches, reserving '?' for genuine encode/write failures. handle_open_reply only accepted the PDU in AwaitingOpenReply, but ironrdp-rdpeai's own client rejects Open by replying with OpenReply failure directly, without a preceding FormatChange confirm, whenever initialFormat is out of range or frames_per_packet is rejected. That left the server wedged in AwaitingFormatConfirm forever with no path back to Ready. MS-RDPEAI 3.3.5.1.8 conditions the server's reaction only on the Result field, not on a preceding FormatChange, so the handler now accepts the reply from AwaitingFormatConfirm too. Open Reply success was checked via equality with S_OK rather than proper HRESULT semantics (an HRESULT is an error only when its sign bit is set, per 3.3.5.1.8), so other valid non-negative success codes like S_FALSE took the failure path and silently dropped all subsequent Data PDUs. Also: server-initiated AUDIO_INPUT DVC data now uses ChannelFlags::SHOW_PROTOCOL, matching every sibling send path (Echo, USB, DrdynvcServer itself) instead of an unexplained ChannelFlags::empty(); and the redundant is_channel_opened guard is removed since dvc_by_id_mut already returns None for an unopened channel and that case was already handled. Regression tests added for the OpenReply-before-FormatChange-confirm wedge and the non-S_OK success code; both verified to fail against the prior behavior and pass with the fix.
Greg Lamberson (glamberson)
force-pushed
the
feat/rdpeai-server-wiring
branch
from
September 11, 2026 19:01
a8465c1 to
fe73e18
Compare
Greg Lamberson (glamberson)
deployed
to
llm-providers
September 11, 2026 19:02 — with
GitHub Actions
Active
GitHub Actions
Active
github-actions
Bot
added
risk/high
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
1 participant
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
RdpeaiServerFactory (ironrdp-server/src/rdpeai.rs) is a thin factory trait mirroring RdpdrServerFactory and SoundServerFactory: build_backend() returns the RdpeaiServerBackend the embedding application supplies. attach_channels registers RdpeaiServer on the DVC stack alongside AInput, DisplayControl, Echo, and RdpeiServer when a factory is configured. RdpServerBuilder::with_rdpeai_factory() follows with_rdpdr_factory's shape.
ServerEvent::Rdpeai(RdpeaiServerMessage) lets the embedding application trigger open() and change_format() from outside the DVC message loop, since RdpeaiServer is nested inside DrdynvcServer rather than being its own top-level channel. The dispatch arm reaches it via DrdynvcServer::dvc_by_id_mut, the same path Echo's dispatch uses to reach EchoDvcBridge, then wraps the resulting PDUs with dvc::encode_dvc_messages before writing them.
rdpei and rdpeai are legitimately distinct protocols (touch and pen input versus audio input) whose names happen to be textually close; the constructor's clippy::similar_names is expected and annotated rather than worked around by renaming either.
Review round
Six findings addressed in a follow-up commit, plus two additional stale duplicates of #1945's already-merged fix closed with an explanation: the dispatch arm's open()/change_format() state rejections now drop-and-warn instead of tearing down the connection, handle_open_reply accepts a client's Open-rejection reply even when it skips the FormatChange confirm (matches ironrdp-rdpeai's own client behavior and MS-RDPEAI 3.3.5.1.8), Open Reply success now checks the HRESULT sign bit instead of equality with S_OK, server-initiated AUDIO_INPUT DVC data uses ChannelFlags::SHOW_PROTOCOL matching every sibling arm, and a redundant channel-opened guard was removed. Two regression tests added.