Skip to content

fix(session): decode RGBA QOI bitmaps instead of dropping the frame#1341

Merged
Benoît Cortier (CBenoit) merged 2 commits into
Devolutions:masterfrom
clintcan:feat-client-rgba-qoi
Jun 1, 2026
Merged

fix(session): decode RGBA QOI bitmaps instead of dropping the frame#1341
Benoît Cortier (CBenoit) merged 2 commits into
Devolutions:masterfrom
clintcan:feat-client-rgba-qoi

Conversation

@clintcan
Copy link
Copy Markdown
Contributor

Companion to #1335 on the client side. Addresses Marc-Andre Lureau (@elmarco)'s review comment on that PR: "if the server is correctly encoding RGBA bitmap, why not fix the client instead?" — short answer: doing both, because they fix different things and reinforce each other (server-side is also a bandwidth optimization, client-side is correctness against third-party RDP servers).

What's broken

In crates/ironrdp-session/src/fast_path.rs::qoi_apply, the Channels::Rgba arm is literally:

qoi::Channels::Rgba => {
    warn!("Unsupported RGBA QOI data");
}

So any server that emits an *a-channel QOI header — which is the natural mapping from a BgrA32 / ARgb32 / etc. screen capture in ironrdp-server::encoder::qoi_encode today — gets one warning per frame and a blank window on the viewer. #1335 fixes the server side of that for the in-tree ironrdp-server. But:

  1. There's nothing in the QOI codec spec that forbids Channels::Rgba. A third-party RDP server that genuinely cares about alpha (layered composition, premultiplied content from a different capture path, etc.) is well within its rights to emit it.
  2. Until fix(server): emit RGB-channel QOI for opaque captures so ironrdp-session can decode #1335 lands and ships in a published ironrdp-server release, every other ironrdp-server consumer in the wild keeps emitting RGBA QOI. Clients carrying this patch render those sessions correctly; clients without it don't.

So the client-side fix is the durable one. The server-side fix in #1335 stands on its own merits (it's a bandwidth optimization — opaque-capture alpha is always 0xFF and costs one byte per QOI literal chunk for zero information).

Fix

Mirror the existing apply_rgb24 / apply_rgb24_iter pair with a 4-bytes-per-pixel apply_rgba32 / apply_rgba32_iter that passes the alpha byte through to the framebuffer (instead of forcing 0xFF the way the rgb24 path does), and call it from the QOI Rgba arm. The Rgb arm is unchanged.

The screen framebuffer is a flat RGBA blit target with no blending, so for an opaque-capture server (everything carrying #1335, plus all current in-tree servers) the only observable effect is that the warning is gone and the frame renders. For a server that legitimately encodes translucent content, the alpha byte is now preserved end-to-end.

Reproduction (before the fix)

Built ironrdp-viewer with --features qoi and pointed it at macrdp 0.5.50 with its vendored QOI Rgb-only workaround disabled (so it emits the natural RGBA mapping):

$ ironrdp-viewer 127.0.0.1:3390 -u $USER -p ...
WARN ironrdp_session::fast_path: Unsupported RGBA QOI data
WARN ironrdp_session::fast_path: Unsupported RGBA QOI data
... (412 warnings in ~12s; viewer renders nothing)

After the fix

Same loopback session against the same macrdp build, no other changes:

Test plan

  • cargo build -p ironrdp-session --features qoi — clean
  • cargo clippy -p ironrdp-session --features qoi --all-targets -- -D warnings — clean (only pre-existing repo-wide MSRV-mismatch notice)
  • cargo test -p ironrdp-session --features qoi — clean
  • cargo fmt --check — clean
  • Loopback ironrdp-viewer --features qoi against a macrdp build emitting RGBA QOI: desktop renders, zero RGBA warnings

Notes

  • apply_rgba32 takes the same flip parameter shape as apply_rgb24 for symmetry; QOI calls it with flip = false.
  • No format/codec advertisement changes — the client already accepts QOI without distinguishing Rgb vs. Rgba in the capability set. Only the apply path changes.
  • If fix(server): emit RGB-channel QOI for opaque captures so ironrdp-session can decode #1335 lands first, this PR still has value as the durable correctness fix described above.

@elmarco
Copy link
Copy Markdown
Contributor

thanks! lgtm

@clintcan clintcan force-pushed the feat-client-rgba-qoi branch from 3425c87 to ffa8855 Compare May 28, 2026 23:45
Copy link
Copy Markdown
Member

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

Choose a reason for hiding this comment

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

LGTM!

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the client-side QOI decode path in ironrdp-session so RGBA-channel QOI frames are decoded and applied to the framebuffer instead of being dropped, improving interoperability with third-party RDP servers and older ironrdp-server builds that emit RGBA QOI.

Changes:

  • Add DecodedImage::apply_rgba32 / apply_rgba32_iter to blit RGBA32 pixel data (including alpha) into the session framebuffer.
  • Update qoi_apply to apply decoded QOI frames for both qoi::Channels::Rgb and qoi::Channels::Rgba (removing the “Unsupported RGBA QOI data” drop path).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
crates/ironrdp-session/src/image.rs Adds an RGBA32 apply path mirroring the existing RGB24 path, preserving the alpha byte.
crates/ironrdp-session/src/fast_path.rs Routes QOI RGBA frames to the new RGBA32 apply function instead of warning + dropping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/ironrdp-session/src/fast_path.rs
Clint Christopher Canada added 2 commits June 2, 2026 01:39
`qoi_apply` previously dropped `Channels::Rgba` frames with
`warn!("Unsupported RGBA QOI data")`, leaving any server that emits
`*A32`-sourced QOI (the natural mapping from a BGRA / ARGB capture)
rendering blank.

Mirror the existing `apply_rgb24` pair with a 4-bytes-per-pixel
`apply_rgba32` / `apply_rgba32_iter` that copies the alpha byte
through to the framebuffer instead of forcing it to `0xFF`, and call
it from the QOI Rgba arm. The Rgb arm is unchanged.

The screen framebuffer doesn't blend, so on opaque captures the only
observable difference is that the warning is gone and the frame is
applied. Servers carrying real translucent content (layered
compositing, third-party RDP impls) now also work.
qoi_apply fed the decoded buffer straight into apply_rgb24/apply_rgba32,
which derive the row count from the decoded length. The only downstream
bounds check (rect_fits) validates the rectangle against the image, not
the buffer against the rectangle, so an oversized/malformed QOI payload
could drive the per-row write index past self.data and panic
(client-side DoS). Reject any decode whose length != width*height*channels
before applying. Guards both the new RGBA path and the existing RGB path.
@clintcan clintcan force-pushed the feat-client-rgba-qoi branch from ffa8855 to 7c892a0 Compare June 1, 2026 17:41
@CBenoit Benoît Cortier (CBenoit) merged commit ef20ea4 into Devolutions:master Jun 1, 2026
21 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.

4 participants