Restoration of Public API Caching Functions and Headers in FreeRDP 3.x

[aleitner]

Is your feature request related to a problem?
With the transition from FreeRDP version 2.x to 3.x, we've encountered a bit of a roadblock. Several functions and cache-related header files that were accessible in the public API of 2.x are now missing/privatized in 3.x. Specifically, API functions like bitmap_cache_register_callbacks and headers such as cache/bitmap.h, cache/brush.h, and others have been removed from the public interface. This change disrupts our current implementation, which relies on these functions for caching mechanisms.

Describe the solution you'd like
We would love it if you brought back public API functionalities related to caching in FreeRDP 3.x.

Describe alternatives you've considered
Initially, I thought about enabling all symbols through the EXPORT_ALL_SYMBOLS flag. Only for a second because that's really not viable and that it diverges from the default behavior of prebuilt FreeRDP packages. As a workaround, I will have to remove our dependency on the FreeRDP cache API. However, this would require significant changes and a move away from using features that have been beneficial to our application's performance and user experience.

Additional context
The migration notes provided for transitioning to FreeRDP 3.x are incomplete and do not provide much clarity (FreeRDP3 migration notes). I've run into a few changes involving setting/getting settings and function name changes that aren't documented within the migration notes. Also, we've noticed a commit which privatizes protocol caches to minimize the public API surface area. This has left us without a transparent upgrade path from 2.x to 3.x. We are looking for guidance, support and ideally a restoration of these caching features in the public API for a better transition for our application.

[akallabeth]

@aleitner we´ve removed the API as there is no sane reason to use it outside but brings a ton of issues if not done properly aside from the maintenance overhead.

so, before considering such a move I´d like to hear some convincing argument to do so:
can you describe a use case that really requires the use of said API outside the core library?

we´re successfully running without this:

• clients
• server implementations
• proxy (client + server)
• protocol analyzers (client + server)

note: for the migration guide we´re happy about an issue with a git bundle with changes (I think the wiki does not support pull requests?) as we´re having issues with wiki vandalism and need some way to limit changes.

[akallabeth]

@aleitner also note: the caching mechanism are initialized automatically depending on your rdpSettings configuration, no need to do it in client side code anymore.
so, the most likely scenario is you need to just remove that code from your implementation and it will work.

[mike-jumper]

@akallabeth Awareness of caching at the API level is required for Apache Guacamole to be able to forward that caching at the protocol level (ie: to allow Guacamole to also cache the bitmaps that the RDP server wants to cache, so that RDP drawing operations that recall bitmaps from that cache can be retransmitted as Guacamole drawing operations that do the same).

Lacking that, we would be forced to resend all cached drawing operations as normal draws without any caching at all. Removing this from the FreeRDP API would effectively remove our ability to keep parity with RDP's caching.

I can't speak to whether there is any other software that leverages FreeRDP to implement this sort of proxying, but removing this API puts an upper bound on the fidelity that can be achieved by any proxy leveraging FreeRDP and limits optimization decisions made by such proxies.

[mike-jumper]

@akallabeth For example, here is how a series of drawing operations would currently map when using FreeRDP 2.x (we use lazy caching to avoid exactly copying Windows' sometimes frivolous use of the cache):

Sequence	RDP Operation	Guacamole Operation
1	Cache bitmap "X"	(do nothing)
2	Draw bitmap "X" using cache	Draw bitmap "X" (without cache)
3	Draw bitmap "X" using cache	Cache bitmap "X", Draw bitmap "X" (using cache)
4	Draw bitmap "X" using cache	Draw bitmap "X" (using cache)
5	Draw bitmap "X" using cache	Draw bitmap "X" (using cache)

And here's how things would map using FreeRDP 3.x without public access to the cache API:

Sequence	RDP Operation	Guacamole Operation
1	Cache bitmap "X"	(do nothing)
2	Draw bitmap "X" using cache	Draw bitmap "X" (without cache)
3	Draw bitmap "X" using cache	Draw bitmap "X" (without cache)
4	Draw bitmap "X" using cache	Draw bitmap "X" (without cache)
5	Draw bitmap "X" using cache	Draw bitmap "X" (without cache)

Awareness of cache operations at the API level allows us to also make use of caching. Lack of awareness of cache operations forces us to always resend image data even when it could have been cached.

[akallabeth]

@mike-jumper I see, the we optimize for windows XP area.
well, I don´t see why you need more low level access to internal caching for that.

you can just hook the Bitmap::New, Bitmap::Free et al.
these are still public (see graphics_register_bitmap et al) so you know when a new bitmap/glyph/whatever is available

[mike-jumper]

@akallabeth That is indeed what we are already doing:

/* Set up bitmap handling */
rdpBitmap bitmap = *graphics->Bitmap_Prototype;
bitmap.size = sizeof(guac_rdp_bitmap);
bitmap.New = guac_rdp_bitmap_new;
bitmap.Free = guac_rdp_bitmap_free;
bitmap.Paint = guac_rdp_bitmap_paint;
bitmap.SetSurface = guac_rdp_bitmap_setsurface;
graphics_register_bitmap(graphics, &bitmap);

@aleitner Perhaps this is a red herring and those explicit function calls to register cache callbacks are not needed?

[akallabeth]

@mike-jumper @aleitner I think you hook in at the wrong level (you get the ROP operations with a specific bitmap and you can hook the bitmap operations as you just quoted above.

I can have a look, but will be afk next week.

[akallabeth]

also, if it is just about these functions: https://github.com/apache/guacamole-server/blob/ad0b4401a55f00f22e343a07d95d6bef94a9ccf3/src/protocols/rdp/rdp.c#L202C5-L202C37

they are now automatically called before the PostConnect callback in the library itself.

[akallabeth]

@mike-jumper so, did you have time to check if these functions are really still required in your case?

[akallabeth]

@aleitner oh, and sorry, pinging you too.
is this now something you still require or can it be done without exposing this?

[aleitner]

@aleitner oh, and sorry, pinging you too. is this now something you still require or can it be done without exposing this?

I took a stab at updating our code to also support FreeRDP3 and removing those cache callback registration functions as well. I don't see any change in performance so I'm pretty sure we didn't need to be calling them

apache/guacamole-server#517