fix(server): emit RGB-channel QOI for opaque captures so ironrdp-session can decode#1335
Conversation
…ion can decode
The server's `qoi_encode` mapped 4-byte pixel formats with explicit
alpha (`ARgb32`, `ABgr32`, `BgrA32`, `RgbA32`) to the matching `*a`
variant of `qoi::RawChannels`, which in `qoicoubeh` produces a QOI
header with `channels = Rgba`. The client-side decoder in
`ironrdp-session/src/fast_path.rs::qoi_apply` only supports
`Channels::Rgb` — the `Rgba` arm is literally
`warn!("Unsupported RGBA QOI data")` and drops the frame. The result
was that any time the QOI codec was actually negotiated between an
`ironrdp-server`-based server and an `ironrdp-client`-based client,
the server emitted a header the client refused, and the client
rendered a blank window with one "Unsupported RGBA QOI data"
warning per frame.
Reproduced over a loopback session:
- Server advertised `[NsCodec, RemoteFx, ImageRemoteFx, Qoi, QoiZ]`.
- Client (`ironrdp-viewer --features qoi,qoiz`) confirmed
`[RemoteFx, Qoi, QoiZ]`.
- Server picked QOI per its codec priority.
- Client emitted 412 `Unsupported RGBA QOI data` warnings over ~12s
and never rendered.
Fix: map every 4-byte input format — alpha or filler — to the `*x`
sibling of `RawChannels`. The qoi crate selects
`Channels::Rgb` for `*x`/`*r`/`*g`/`*b` variants and `Channels::Rgba`
only for `*a` variants, so this forces 3-channel QOI output that the
existing decoder accepts. Server-side bitmap captures are
functionally opaque (the alpha byte from screen capture is meaningless
— either premultiplied to 0 or always 0xFF), so discarding the alpha
byte at encode time is consistent with how the rest of the legacy
bitmap path already handles these formats.
After the fix the same loopback session emits 0 RGBA warnings and the
viewer renders normally.
Discussion: Devolutions#1322
…OI output
The vendored `qoi_encode` mapped 4-byte `*A32` pixel formats to the
matching `*a` `qoi::RawChannels`, which produces a QOI header with
`channels = Rgba`. The decoder in `ironrdp-session::fast_path::qoi_apply`
only supports `Channels::Rgb` — the `Rgba` arm is literally
`warn!("Unsupported RGBA QOI data")` and drops the frame.
So whenever a QOI-capable IronRDP client (like `ironrdp-viewer`) connects
to macrdp, the codec negotiates fine but every encoded frame is dropped
by the client. Loopback repro: 412 `Unsupported RGBA QOI data` warnings
in ~12s, viewer window stays blank.
Force each 4-byte input — alpha or filler — to its `*x` sibling so QOI
emits 3-channel `Channels::Rgb` output that the decoder accepts.
Screen-capture alpha is meaningless anyway (always 0xFF / premultiplied),
so dropping the byte at encode time is consistent with how the legacy
bitmap path already handles these formats.
Verified end-to-end: 0 warnings, viewer renders normally.
Upstreamed as Devolutions/IronRDP#1335. CLAUDE.md
updated with divergence (7) under the vendor/ironrdp-server note, and
the "keep until upstreamed AND released" sentence extended through (7).
|
thanks for the investigation and the fix! If the server is correctly encoding RGBA bitmap, why not fix the client instead? |
|
Thanks for the review! Fair point: both fixes are reasonable, and I think they're complementary rather than alternatives. The server-side change here is partly a bandwidth optimization: every screen-capture *A32 frame has an alpha byte that's always 0xFF for opaque content, so encoding it costs one extra byte per QOI_OP_RGB/QOI_OP_RGBA literal chunk for zero information gain. There's actually existing precedent in the same function; the four *X32 variants already map to the *x RawChannels for exactly this reason (the X filler byte is dropped). Extending the same treatment to *A32 keeps the function consistent. Long story short, the server can actually emit either format. But you're right that the client should also handle RGBA QOI. Today it can't decode legitimate RGBA frames (the Channels::Rgba arm is a warn! + drop), which is a real gap whenever a server has a reason to emit them (layered composition, premultiplied alpha preservation, etc.). I'd be happy to open a follow-up adding the RGBA decode path on the client side, it's a few lines either widening apply_rgb24 or stripping alpha before the existing call. They're independent PRs and can land in either order. This is good for future divergence (or third-party RDP servers with real alpha channels). If you'd prefer to close this in favor of just the client-side fix, I'm fine with that too — but I'd argue the server side is worth keeping as a bandwidth optimization regardless, since screen captures don't carry useful alpha — every consumer I know of feeds opaque content. |
|
Marc-Andre Lureau (@elmarco) — opened #1341 as the companion client-side fix. TL;DR: I think both changes have independent value (server is a bandwidth optimization on opaque captures; client is correctness against any server that legitimately emits |
fair enough, I don't think we care enough about rgba support at this point. We can revisit later. |
Benoît Cortier (CBenoit)
left a comment
There was a problem hiding this comment.
LGTM! If Marc-Andre Lureau (@elmarco) is good with the changes too, we can merge
Fixes the broken QOI codec path between
ironrdp-serverandironrdp-client/ironrdp-session. Found while answering Marc-Andre Lureau (@elmarco)'s "did you try [QOI]?" question on #1322.What's broken
In
crates/ironrdp-server/src/encoder/mod.rs::qoi_encode, the four*A32PixelFormatvariants (ARgb32,ABgr32,BgrA32,RgbA32) map to the matching*avariant ofqoi::RawChannels. Inqoicoubeh, those variants produce a QOI header withchannels = Rgba. The decoder inironrdp-session/src/fast_path.rs::qoi_applyonly supportsChannels::Rgb; theChannels::Rgbaarm is literallywarn!("Unsupported RGBA QOI data")and drops the frame.So the moment QOI actually gets negotiated between an
ironrdp-server-based server and anironrdp-client/ironrdp-session-based client, the server emits a header the client refuses, and the client renders a blank window plus one warning per frame.Reproduction (before the fix)
Loopback against
macrdp0.5.49 (which usesironrdp-serverfromDevolutions/IronRDPrev879ffed8):From the connector trace, the server advertised
[NsCodec, RemoteFx, ImageRemoteFx, Qoi, QoiZ], the client confirmed[RemoteFx, Qoi, QoiZ], and the server picked QOI per its codec selection ladder. Every encoded frame was emitted as RGBA.Fix
Map every 4-byte input format — whether it nominally has an alpha byte or an "X" filler — to the
*xsibling ofRawChannels. The qoi crate picksChannels::Rgbfor*x/*r/*g/*bandChannels::Rgbaonly for*a, so this forces 3-channel output that the existing decoder accepts.Server-side bitmap captures are functionally opaque (the alpha byte from a screen capture is either premultiplied to 0 or set to 0xFF), so discarding it at encode time is consistent with how the rest of the legacy bitmap path already handles these formats.
After the fix
Same loopback session:
Unsupported RGBA QOI datawarningsTest plan
cargo build -p ironrdp-server --features qoi— cleancargo build -p ironrdp-server(default) — cleancargo clippy -p ironrdp-server --features qoi,qoiz,helper,__bench --all-targets -- -D warnings— cleancargo fmt --check— cleanironrdp-viewer --features qoi,qoizagainstmacrdp(aironrdp-serverconsumer) — 412 → 0 RGBA warningsNotes
*X32variants are unchanged; they already mapped to the*xRawChannelsand produced correct RGB output.Channels::Rgbadecode path inqoi_apply(just ignore the alpha plane), which would let clients consume RGBA QOI from third-party encoders if any exist. Out of scope here.