Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add QOI compression #670

Draft
wants to merge 33 commits into
base: master
Choose a base branch
from
Draft

Add QOI compression #670

wants to merge 33 commits into from

Conversation

elmarco
Copy link
Contributor

@elmarco elmarco commented Feb 13, 2025

I recently found the QOI image format (https://qoiformat.org/) and was excited by its potential for desktop remoting.

I hacked up a few things to actually benchmark it compared to the other codecs.

Based on a raw recording of a typical Fedora desktop usage (terminal/web), for 730 4k frames:

None: 5s user CPU, 0% compression
Bitmap: 74s user CPU, 92.5% compression
RemoteFx (lossy): 201s user CPU, 96.72% compression
QOI: 10s user CPU, 96.20% compression
UPDATE:
jpeg-turbo 80% 4:2:2: 16s user CPU, 97.92% compression
jpeg-turbo 95% 4:4:4: 22s user CPU, 96.13% compression
jpeg-turbo lossless: 135s user CPU, 86.06% compression
UPDATE2:
tile diff+QOI+zstd stream long window: 11s user CPU, 99.76% compression!!

There is still a lot of various optimizations to be desired regardless of the codecs, but this will hopefully help. Also, we should start thinking how QOI could be adapted for streaming.

@awakecoding
Copy link
Contributor

Custom codecs not supported by the Microsoft RDP implementation have rarely been used, but since we're building both the client and server in IronRDP, we're free to experiment! QOI is an excellent choice, it is very simple, and easier to optimize due to its low complexity. IIRC QOI uses RGB, not YUV like most codecs. Color conversion and chroma subsampling alone can take up a lot of the processing time if not properly optimized.

There is just one custom codec extension I've seen in the past: JPEG in RDP. XRDP supports it in the server, and FreeRDP supports it in the client as an optional build feature. JPEG has many advantages: it is by far the most widely available image codec, but it is also one for which the most effort has been put into optimizing the implementation (think libjpeg-turbo). For an RDP web client, this also makes it possible to leverage built-in browser support for JPEG decoding, which is a huge advantage over porting and optimizing a custom codec in WASM.

In the past, we've designed our own remote desktop protocol (Wayk Now) at Devolutions, and we experimented with many different ways to deal with the codecs more efficiently than RDP. We had GFWX ported in Rust: https://github.com/Devolutions/gfwx-rs

GFWX, unlike QOI, is a simplified wavelet codec, which would make it closer to RemoteFX. It is generic and can accept a wide variety of YUV color formats (RGB wouldn't perform well, you're better off doing the YUV conversion to avoid the color channel correlation issue of RGB). It is one of the rare codecs that would enable really nice things like lossless encoding in which you could trim part of the encoded output for lossy encoding. In our implementation, we've used a reversible color conversion (YCoCg-R) due to its reduced complexity (it's only integer operations, not floating point). Unfortunately, we found out JPEG still performed better than GFWX. In theory GFWX offers ways to beat JPEG, but libjpeg-turbo is just hard to beat.

If you're curious about the color transformations we've experimented with, we still have them here:
https://github.com/Devolutions/cadeau/tree/master/libxpp

I'm open to experimenting with various custom codecs, the ones in RDP are good, but not necessarily special or great. We can definitely beat the standard protocol if we go custom :)

@elmarco
Copy link
Contributor Author

elmarco commented Feb 14, 2025

@awakecoding I added some jpegturbo benchmarks above. For lossless, desktop usage, QOI is very good - jpeg is far behind. Performance with wasm should be ok (rust-qoi compiles for wasm target)

We used to have some heuristics in Spice server to detect video zones, and used jpeg aggressively iirc (when not using whole frame gpu video encoding).

pub(crate) struct UpdateFragmenter<'a> {
code: UpdateCode,
index: usize,
data: &'a [u8],
pub data: &'a [u8],
Copy link
Member

@CBenoit CBenoit Feb 14, 2025

Choose a reason for hiding this comment

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

question: Does this work?

Suggested change
pub data: &'a [u8],
#[cfg_attr(feature = "__bench", visibility::make(pub))]
data: &'a [u8],

I know this is very similar, but it means that under normal builds, while the struct is visible by all the other modules within the crate, the data field remains module-private, while the unconditional pub means data could be accessed by the other modules even under normal builds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, it doesn't work on fields: error: expected non-macro attribute, found attribute macro visibility::make

Copy link
Member

Choose a reason for hiding this comment

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

I see! Then, could you add something like this:

#[doc(hidden)] // not part of the public API, used by benchmarks

Just for easier reviewing in the future

@CBenoit
Copy link
Member

CBenoit commented Feb 14, 2025

I’m impressed by the characteristics of this codec, especially given how simple the compression format is.

As a custom codec, it seems to be a solid alternative to JPEG:

  • Smaller CPU-usage than RemoteFX
  • No dependency on jpeg-turbo, i.e.: smaller binary and less unsafe code
  • Compression level on par with both RemoteFX and JPEG

And the Rust implementation is not even optimized yet.

For the reasons explained by Marc-André, I don’t think we can beat JPEG in the browser (yet?) even with WASM, but I suspect it’s better than the current implementation which does not rely on the native JPEG implementation of the browser anyway (RLE / RDP 6.0 compression codec → RGB → JavaScript Image object). Definitely does not hurt to use QOI. If we really want to squeeze performance using JPEG, we could consider that too in the future though.

Obviously the limitation is that only IronRDP ↔ IronRDP will be supported, but I’m interested in seeing this as an option similar to how JPEG is an option for FreeRDP ↔ XRDP.
I’ll be glad to merge the PR when it’s ready.

By the way, do you have an idea how good it performs on the client side (decoding)?

Good opportunity to get some some basic benchmarks merged too.
Small suggestion: what about a benches/ folder at the root of the repository?

@elmarco elmarco force-pushed the qoi branch 2 times, most recently from 9a3f3ab to a8c9fdd Compare March 26, 2025 19:04
elmarco added 16 commits March 27, 2025 00:12
Give a hint to the backend when the channel is actually connected &
ready to process messages.

Signed-off-by: Marc-André Lureau <[email protected]>
Signed-off-by: Marc-André Lureau <[email protected]>
Dealing with multiple formats is sufficiently annoying, there isn't much
need for awkward image layout. This was done for efficiency reason for
bitmap encoding, but bitmap is really inefficient anyway and very few
servers will actually provide bottom to top images (except with GL/GPU
textures, but this is not in scope yet).

Signed-off-by: Marc-André Lureau <[email protected]>
Signed-off-by: Marc-André Lureau <[email protected]>
This will hold the updated bitmap data for the whole framebuffer. For
now, only a mutable version.

Signed-off-by: Marc-André Lureau <[email protected]>
This is more idiomatic, and thus less confusing.

Signed-off-by: Marc-André Lureau <[email protected]>
Signed-off-by: Marc-André Lureau <[email protected]>
The bitmap encoder dispatching code was becoming convoluted and the same
struct was handling PduEncoding and various bitmap encoding handling.
Instead, split UpdateEncoder in different types and concerns.

Signed-off-by: Marc-André Lureau <[email protected]>
Signed-off-by: Marc-André Lureau <[email protected]>
It should reflect client drawing state.

In following changes, we will fix it to draw bitmap updates on it, to
keep it up to date.

Signed-off-by: Marc-André Lureau <[email protected]>
Signed-off-by: Marc-André Lureau <[email protected]>
elmarco added 8 commits March 27, 2025 00:12
Trying to share a common buffer creates all sort of complicated lifetime
issues when trying to move the encoding to a 'static handler, or when
implementing iterators. It's not worth it.

Signed-off-by: Marc-André Lureau <[email protected]>
FIXME: remotefx renders some line artefacts, likely due to bad
       compression behaviour on the tile edges.

Signed-off-by: Marc-André Lureau <[email protected]>
This will simplify setting up the UpdateEncoder with further codecs.

Signed-off-by: Marc-André Lureau <[email protected]>
Signed-off-by: Marc-André Lureau <[email protected]>
@elmarco elmarco force-pushed the qoi branch 2 times, most recently from 40483fb to 78bd227 Compare March 27, 2025 07:52
elmarco added 8 commits March 27, 2025 13:48
"session" has a fixed set of supported codecs with associated IDs.

"connector" must expose the set of codecs during capabilities exchange.
It currently uses hard-codes codec IDs in different places.

Move the BitmapCodecs set to ironrdp-pdu. Shared code will be used by
the server, so this is a suitable common place.

Signed-off-by: Marc-André Lureau <[email protected]>
Signed-off-by: Marc-André Lureau <[email protected]>
Teach the server to support customizable codecs set. Use the same
logic/parsing as the client codecs configuration.

Signed-off-by: Marc-André Lureau <[email protected]>
If a region is updated more than 5x/seconds, use a video updater for
encoding. For now, this is RemoteFx, but it will allow future tweaking.

Signed-off-by: Marc-André Lureau <[email protected]>
No need to special-case codec_properties_len == 0, defer to the decoding
of the properties instead.

Signed-off-by: Marc-André Lureau <[email protected]>
The Quite OK Image format ([1]) losslessly compresses images to a
similar size of PNG, while offering 20x-50x faster encoding and 3x-4x
faster decoding.

Add a new QOI codec (UUID 4dae9af8-b399-4df6-b43a-662fd9c0f5d6) for
SetSurface command. The PDU data contains the QOI header (14 bytes) +
data "chunks" and the end marker (8 bytes).

Some benchmarks showing interesting results (using ironrdp/perfenc)

Bitmap: 74s user CPU, 92.5% compression
RemoteFx (lossy): 201s user CPU, 96.72% compression
QOI: 10s user CPU, 96.20% compression

[1]: https://qoiformat.org/

Signed-off-by: Marc-André Lureau <[email protected]>
Add a new QOIZ codec (UUID 229cc6dc-a860-4b52-b4d8-053a22b3892b) for
SetSurface command. The PDU data contains the same data as the QOI
codec, with zstd compression.

Some benchmarks showing interesting results (using ironrdp/perfenc)

QOI: 10s user CPU, 96.20% compression
QOIZ: 11s user CPU, 99.76% compression

Signed-off-by: Marc-André Lureau <[email protected]>
@elmarco elmarco changed the title WIP: add QOI compression Add QOI compression Mar 28, 2025
@elmarco
Copy link
Contributor Author

elmarco commented Mar 28, 2025

@CBenoit I think we should be good to start reviewing this series. I can split various preliminary patches in different MR if you prefer?

@CBenoit
Copy link
Member

CBenoit commented Mar 28, 2025

@CBenoit I think we should be good to start reviewing this series. I can split various preliminary patches in different MR if you prefer?

Sounds great! Yes, I would prefer multiple MRs. I’ll do my best to be as responsive as possible!

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.

3 participants