search

LibVNC / libvncserver

LibVNCServer/LibVNCClient are cross-platform C libraries that allow you to easily implement VNC server or client functionality in your program.
GNU General Public License v2.0
1.1k stars 482 forks source link

Please support larger screens #521

Closed rrthomas closed 2 years ago

rrthomas commented 2 years ago

Is your feature request related to a problem? Please describe. I have a two-monitor setup with 2× 4k screens. This requires a line buffer of 30720 bytes, but in rfb.h UPDATE_BUF_SIZE is defined as 30000. When I try to serve this setup I get the error:

rfbSendRectEncodingRaw: send buffer too small for 30720 bytes

It might be reasonable to use a much bigger value, e.g. at least 61440, to allow setups with 2× 8k monitors.

Describe the solution you'd like Please increase the value of UPDATE_BUF_SIZE. This doesn't seem to be a big deal, since there's only one buffer of this size per struct _rfbClientRec, and even (say) doubling that again would only be 128KB per client.

Thanks for LibVNC!

bk138 commented 2 years ago

Yeah, sounds reasonable!

sunweaver commented 2 years ago

@bk138 Any chance to get an upstream patch any time soonish which I then could cherry-pick into libvncserver in Debian prior to an upstream release?

bk138 commented 2 years ago

@bk138 Any chance to get an upstream patch any time soonish which I then could cherry-pick into libvncserver in Debian prior to an upstream release?

Yeah I can look into this this or next week, as it's probably a fairly small change.

bk138 commented 2 years ago

@rrthomas Two questions:

rrthomas commented 2 years ago

@rrthomas Two questions:

* How do you actually end up with 30720 bytes for a 2 x 4096 x 4 setup?

Horizontal resolution for 4k is (at least in my case) 3840: 3840×4×2 = 30720.

* Are you in control of the server code, i.e. would a configurable buffer size work for you?

It would work for me, but why make it complicated when raising the limit would use very little memory? At least raise it to 30,720 (an extra 720 bytes), which would work for up to 2×4k monitors or 1×8k monitor.

If raising a static allocation is seriously not an option, could that memory be dynamically allocated?

sunweaver commented 2 years ago

X11 max resolution is 32768x32768, so maybe simply put the X11 max values?

rrthomas commented 2 years ago

@sunweaver, that would be 262,144 bytes. Also, good point about the max size, I was wondering how many screens horizontally is "reasonable", but this is independent of that.

sunweaver commented 2 years ago

it is actually not 32768x32768, but MAXSHORTxMAXSHORT whereas MAXSHORT is SHRT_MAX set to 32767 in C's limits.h.

bk138 commented 2 years ago

@rrthomas I'm mainly thinking about existing users with embedded use case where a doubling of the static allocation per client might already have serious effects.

@sunweaver using X11 max values is an understandable idea, the use case of the lib is not limited to X11 though, so this would be arbitrary as well.

I'll simply raise the default to 32768 which is not an awful lot of an increase and should not hurt the embedded guys.

If someone needs more, we can still go down the malloc() route, but I'll leave this to the users to request and will KISS for now.