Skip to content

feat(egfx)!: surface total_frames_decoded on the frame-ack callback#1345

Merged
Benoît Cortier (CBenoit) merged 2 commits into
Devolutions:masterfrom
lamco-admin:feat/egfx-on-frame-ack-total-decoded
Jun 1, 2026
Merged

feat(egfx)!: surface total_frames_decoded on the frame-ack callback#1345
Benoît Cortier (CBenoit) merged 2 commits into
Devolutions:masterfrom
lamco-admin:feat/egfx-on-frame-ack-total-decoded

Conversation

@glamberson

Copy link
Copy Markdown
Contributor

Summary

GraphicsPipelineHandler::on_frame_ack already receives frame_id and queue_depth, but handle_frame_acknowledge drops the third field the client sends in FrameAcknowledgePdu (MS-RDPEGFX 2.2.2.13): total_frames_decoded, the client's running decoded-frame count. encoded_count - total_frames_decoded is an instant measure of the client's decode backlog, which a server needs for closed-loop EGFX flow control. Issue #790 (ironrdp-web becomes unresponsive under rapid framebuffer updates) is the user-facing symptom of having no such signal.

This extends on_frame_ack to pass total_frames_decoded through. The field is already decoded; the handler just dropped it before the callback. ironrdp-egfx is unpublished (publish = false), so the breaking signature change has no external SemVer impact; the only in-tree consumers are the trait default and the egfx testsuite handler, both updated here. ironrdp-server inherits the default and is unchanged.

Validation

cargo xtask check fmt/lints/tests/typos/locks all pass.

Notes

Breaking on the GraphicsPipelineHandler trait (hence feat!), scoped to ironrdp-egfx plus its testsuite. This surfaces the field only; it adds no flow-control policy.

GraphicsPipelineHandler::on_frame_ack already receives frame_id and
queue_depth, but handle_frame_acknowledge drops the third field the client
sends in FrameAcknowledgePdu (MS-RDPEGFX 2.2.2.13): total_frames_decoded,
the client's running decoded-frame count. Computing
encoded_count - total_frames_decoded gives an instant measure of the
client's decode backlog, which a server needs for closed-loop EGFX flow
control (see issue Devolutions#790: ironrdp-web becomes unresponsive under rapid
framebuffer updates because nothing throttles to the client decode rate).

Extend on_frame_ack to pass total_frames_decoded through. ironrdp-egfx is
unpublished, so the breaking signature change has no external SemVer
impact; the only in-tree consumers are the default impl and the egfx
testsuite handler, both updated here.
Greg Lamberson (glamberson) added a commit to lamco-admin/IronRDP that referenced this pull request May 31, 2026
Mirror upstream PR Devolutions#1345: extend GraphicsPipelineHandler::on_frame_ack to
pass the client's total_frames_decoded (MS-RDPEGFX 2.2.2.13), and drop the
fork-only on_frame_ack_full method that carried it via a delegating
default. The call-site routes through the extended on_frame_ack directly.
Keeps this fork uniform with the upstream change so they converge on merge.
Comment thread crates/ironrdp-egfx/src/server.rs Outdated

@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.

LGTM

@CBenoit Benoît Cortier (CBenoit) enabled auto-merge (squash) June 1, 2026 15:13
@CBenoit Benoît Cortier (CBenoit) merged commit cf51bdd into Devolutions:master Jun 1, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants