Handling surface update commands on large screens with RFX

[matt335672]

In the space of investigating #2068, a problem was found with the existing interface to the RFX encoder when processing surface commands.

On a large screen (i.e. 4K), using RFX the number of rectangles passed in to the encoder can be too large after processing to fit in to a single TS_FP_SURFCMDS PDU. At present this condition isn't detected, and the result is (often) an xrdp crash, although it also seems possible we send the client an over-large PDU, which may well have the same effect.

It's a simple matter to detect and prevent the overflow - there's a patch attached to #2068 which does this - but then the question arises as to what to do. The current patch simply results in all the surface commands being dropped. Also, at present, the error condition isn't logged which means it isn't obvious what's happened to the frame data.

A complete solution to this problem requires (I think) the rectangles to be distributed over multiple PDUs. This is going to need quite a bit of change to the code however, and it's not clear to me that the benefit we'd achieve would be that great for the time it would take.

Other factors at play here are Microsoft moving away from RemoteFX and the work we're putting on to GFX.

Other than a costly re-implementation of RemoteFX, ways of addressing this could be:-

1. Drop the surface updates, and log a message
2. Truncate the surface updates and log a message

Are either of these sensible things to do at this point?

[matt335672]

I'm replying to my own post here. After reading some more of the RemoteFX spec and looking at the FreeRDP code there's a relatively simple solution to the buffer size at least, which is to ask the client to support a larger buffer.

A lot of the rest of the above probably isn't relevant.

[metalefty]

I just looked through this.

Hmm, Windows 7 is the last Windows that uses classic RemoteFX. 4K screen was not so popular Windows 7 era. Maybe that's why this problem didn't show up in Windows 7 at the time.

Unfortunately, I'm still not sure which way to go.