Skip to content

feat(rdpeai): add RdpeaiServer orchestration for AUDIO_INPUT - #1945

Merged
Benoît Cortier (CBenoit) merged 2 commits into
Devolutions:masterfrom
lamco-admin:feat/rdpeai-server
Sep 11, 2026
Merged

Benoît Cortier (CBenoit) merged 2 commits into
Devolutions:masterfrom
lamco-admin:feat/rdpeai-server

Conversation

@glamberson

@glamberson Greg Lamberson (glamberson) commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

ironrdp-rdpeai only had a client processor (#1645). The PDU layer already decodes and encodes every message in both directions, but nothing drove the server side of the state machine.

RdpeaiServer fills that gap. It mirrors RdpeaiClient's structure and the RdpdrServer precedent (#1783): it sends Version on channel start and Formats once the client replies, automatically, exposes open() and change_format() for the embedding application to call once it wants to record or switch formats, and delivers decoded audio to an injected RdpeaiServerBackend.

Out-of-sequence, unrecognized, and mismatched PDUs are logged and ignored rather than treated as errors, per MS-RDPEAI 3.1.5's explicit MUST-ignore requirement. change_format() honors 3.3.5.3.1's SHOULD NOT for AAC formats when the client only advertised protocol version 1.

14 tests in ironrdp-testsuite-core cover the full handshake, open failure and retry, the format-change round trip and the AAC/version-1 skip rule, out-of-sequence handling, and channel close.

ironrdp-server integration is a separate follow-up, the same split RdpdrServer's PR took.

One PR is stacked on this

#1946 wires this into ironrdp-server (RdpeaiServerFactory, attach_channels, ServerEvent::Rdpeai). It only makes sense once this PR lands.

ironrdp-rdpeai only had a client processor (PR Devolutions#1645); the PDU layer
(pdu.rs) was already bidirectional, but nothing drove the server side
of the MS-RDPEAI state machine. RdpeaiServer fills that gap, mirroring
RdpeaiClient's structure and the RdpdrServer precedent (PR Devolutions#1783):
sends Version on channel start and Formats once the client replies,
automatically; exposes open()/change_format() for the consuming
application to call once it wants to record; and delivers decoded
audio to an injected RdpeaiServerBackend.

Out-of-sequence, unrecognized, and mismatched PDUs are logged and
ignored rather than treated as errors, per MS-RDPEAI 3.1.5's explicit
MUST-ignore requirement. change_format() honors 3.3.5.3.1's SHOULD NOT
for AAC formats when the client only advertised protocol version 1.

14 new tests in ironrdp-testsuite-core cover the full handshake,
open failure/retry, format-change round trip and the AAC/V1 skip
rule, out-of-sequence handling, and channel close.

ironrdp-server integration (a factory trait plus attach_channels
wiring, matching RdpeiServerFactory's shape) is a separate follow-up,
same split RdpdrServer took.
@github-actions github-actions Bot added kind/protocol Affects RDP or related protocol behavior 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 Sep 10, 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 adds RdpeaiServer, filling the server half of AUDIO_INPUT and mirroring RdpeaiClient and the RdpdrServer precedent; handshake sequencing, format-negotiation filtering, open/retry handling, and the AAC/version-1 guard are sound. Two real defects remain: process() propagates decode errors for malformed/unrecognized PDUs, contradicting MS-RDPEAI 3.1.5's MUST-ignore rule and the module's own doc, with the error aborting DRDYNVC SVC processing; and client FormatChange confirmations are applied without range or parity validation, allowing mislabeled or silently dropped audio. Remaining items are minor maintainability nits and one unverified spec interpretation.

Comment thread crates/ironrdp-rdpeai/src/server.rs
Comment thread crates/ironrdp-rdpeai/src/server.rs
Comment thread crates/ironrdp-rdpeai/src/server.rs
Comment thread crates/ironrdp-rdpeai/src/server.rs Outdated
Comment thread crates/ironrdp-rdpeai/src/server.rs Outdated
Comment thread crates/ironrdp-testsuite-core/tests/rdpeai/server.rs Outdated
@github-actions github-actions Bot added the ai-reviewed/1 One automated review completed label Sep 10, 2026
process() propagated a decode failure as an Err through the shared
DRDYNVC SVC processor instead of ignoring it, contradicting the module's
own MS-RDPEAI 3.1.5 MUST-ignore documentation and risking the whole
dynamic-channel loop on a single malformed AUDIO_INPUT PDU. It now warns
and returns an empty message list, matching every other ignore case in
the same function.

handle_format_change also trusted the client's echoed format index even
when it did not match what the server requested, warning but still
applying it. A non-conformant confirm now keeps the server-requested
index instead, since an out-of-range echo silently drops all subsequent
Data PDUs and an in-range-but-wrong one mislabels the format.

Also drops a channel_id field that was set but never read (integrators
recover the id via DrdynvcServer::get_channel_id_by_type, as the stacked
wiring PR already does), folds the start()/close() reset sequence into a
shared helper, and trims three constant-valued parameters plus a stale
doc comment from the negotiate_and_open test helper.
@github-actions github-actions Bot added triage/overlap This issue or pull request already exists or overlaps maintainer-required Maintainer review or intervention is required risk/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny and removed risk/medium Behavioral change that does not substantially alter a core public API labels Sep 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Potential duplicate detected: #1946.

PR #1946 is a stacked follow-up from the same branch line whose cumulative diff includes this exact work (crates/ironrdp-rdpeai/src/server.rs with RdpeaiServer and RdpeaiServerBackend); its body states it is built entirely on this AUDIO_INPUT server crate change (filed as #1945, not in the candidate list), so the same code appears in two open PRs.

Maintainer review is required.

@CBenoit Benoît Cortier (CBenoit) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

@CBenoit
Benoît Cortier (CBenoit) merged commit a1bd456 into Devolutions:master Sep 11, 2026
41 checks passed
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/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny size/L Size: up to 899 counted lines and 20 files; exceeds M in either measure triage/overlap This issue or pull request already exists or overlaps

Development

Successfully merging this pull request may close these issues.

2 participants