From 54cf82da514a20330867aee4401d16dedc443ff9 Mon Sep 17 00:00:00 2001 From: DRC Date: Tue, 24 Sep 2024 16:58:58 -0400 Subject: [PATCH] Server: Don't use Tight w/o Zlib unless advertised Since TurboVNC v0.5, the Lossless Tight encoding method has used an unofficial Tight encoding extension that bypasses zlib. That extension was innocuous when using the TurboVNC Viewer with a non-TurboVNC server, but when using the TurboVNC Server with a non-TurboVNC viewer, selecting Compression Level 0 without JPEG caused a fatal error in the viewer. The extension has been officially registered with IANA as an RFB pseudo-encoding (0xFFFFFEC3 or -317), so this commit modifies the server so that it no longer uses the extension unless a viewer advertises support for it. (The TurboVNC Viewer has been modified in the TurboVNC 3.1.x and prior code bases so that it advertises support for the extension.) This commit also disallows more than 4 Tight encoding threads. Since 9116e397da289f334784ce1694281fb00db10001 (TurboVNC 2.1.2), the TurboVNC Server hasn't enabled more than 4 Tight encoding threads by default, because doing so would reduce the compression ratio significantly and create compatibility issues with non-TurboVNC viewers (due to the unofficial use of the aforementioned extension.) Low-level tests show an incremental speedup (about 36% on my Dell Precision 5820) when using Tight + Perceptually Lossless JPEG with 8 threads vs. 4 threads. However, since Tight encoding is limited to 4 zlib streams, using more than 4 threads requires bypassing zlib. Thus, when using Tight + Perceptually Lossless JPEG, the compression ratio is about 15-20% worse with 8 threads vs. 4 threads. (The compression ratio is much worse when using Lossless Tight + Zlib with 8 threads vs. 4 threads.) Also, the speedup does not bear out in real-world tests, since the viewer or the network tends to be more of a bottleneck. In order to continue supporting more than 4 Tight encoding threads, it would be necessary to make the maximum thread count contingent upon whether the viewer supports the (now official) Tight Encoding Without Zlib extension, and that would be confusing to end users. My experience is that a feature that requires more than one apology is a feature that has probably outlived its usefulness. --- ChangeLog.md | 6 +++ common/rfb/rfbproto.h | 3 ++ doc/compatibility.txt | 35 +++++++-------- doc/config.txt | 2 +- doc/index.html | 45 +++++++++---------- doc/performance.txt | 24 ++++++---- unix/Xvnc/programs/Xserver/hw/vnc/rfb.h | 4 +- unix/Xvnc/programs/Xserver/hw/vnc/rfbserver.c | 6 +++ unix/Xvnc/programs/Xserver/hw/vnc/tight.c | 25 +++++------ 9 files changed, 84 insertions(+), 66 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index d6009974f..0e59afc07 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -50,6 +50,12 @@ parameter (`ServerDir`.) `turbovnc.via` and `turbovnc.tunnel` Java system properties have been deprecated and replaced with a new advanced parameter (`ExtSSHTemplate`.) +6. The TurboVNC Server no longer enables the "Tight Encoding Without Zlib" RFB +extension unless the VNC viewer advertises support for it. This prevents a +fatal error that occurred in TightVNC-compatible VNC viewers (other than the +TurboVNC Viewer) when attempting to select Compression Level 0 without JPEG +while connected to a TurboVNC session. + 3.1.3 ===== diff --git a/common/rfb/rfbproto.h b/common/rfb/rfbproto.h index 0c9f87943..7ec45ab2b 100644 --- a/common/rfb/rfbproto.h +++ b/common/rfb/rfbproto.h @@ -506,6 +506,7 @@ typedef struct _rfbInteractionCapsMsg { * -768 .. -763 * 0xFFFFFE00 .. 0xFFFFFE64 -- fine-grained quality level (0-100 scale) * -512 .. -412 + * 0xFFFFFEC3 -- Tight encoding without zlib * 0xFFFFFEC7 .. 0xFFFFFEC8 -- flow control extensions * -313 .. -312 * 0xFFFFFECC -- extended desktop size @@ -543,6 +544,8 @@ typedef struct _rfbInteractionCapsMsg { #define rfbEncodingFineQualityLevel0 0xFFFFFE00 /* -512 */ #define rfbEncodingFineQualityLevel100 0xFFFFFE64 /* -412 */ +#define rfbEncodingTightWithoutZlib 0xFFFFFEC3 /* -317 */ + #define rfbEncodingContinuousUpdates 0xFFFFFEC7 /* -313 */ #define rfbEncodingFence 0xFFFFFEC8 /* -312 */ diff --git a/doc/compatibility.txt b/doc/compatibility.txt index 797bf7457..76ba29500 100644 --- a/doc/compatibility.txt +++ b/doc/compatibility.txt @@ -67,14 +67,14 @@ TurboVNC with other VNC flavors. Compression Level 5. {nl}{nl} - * TurboVNC supports an extension to Tight encoding that allows the - server to tell the viewer to bypass zlib when decompressing a particular - subrectangle. Zlib introduces a tremendous amount of performance overhead, - even when zlib compression level 0 (no compression) is used. Thus, when a - TurboVNC viewer requests Compression Level 0 from the TurboVNC Server, - the TurboVNC Server bypasses zlib altogether. TightVNC and TigerVNC - servers do not support this extension, so they will still use zlib to - "compress" framebuffer updates even if you request Compression Level 0. + * Zlib introduces a significant amount of performance overhead, even when + zlib compression level 0 (no compression) is used, so TurboVNC supports a + Tight encoding extension that allows the server to bypass zlib when + encoding a particular subrectangle. The extension is enabled when a VNC + viewer advertises support for it and requests Compression Level 0. As of + this writing, TightVNC and TigerVNC do not support the extension, so the + TightVNC and TigerVNC servers will use zlib to "compress" framebuffer + updates if you request Compression Level 0 using the TurboVNC Viewer. {nl}{nl} * When properly configured, version 1.2 and later (except versions @@ -95,17 +95,14 @@ TurboVNC with other VNC flavors. specified in {ref prefix="Section ": TigerVNC_JPEG_Qual}. {nl}{nl} - * When either a TightVNC or TigerVNC viewer is connected to a TurboVNC - session and JPEG subencoding is disabled, setting the compression level to - 0 in the viewer will cause the connection to abort with a "bad subencoding - value" error. This is because the TurboVNC Server attempts to send - framebuffer updates with no zlib compression, and the TightVNC and TigerVNC - viewers don't support that. - {nl}{nl} - A similar issue occurs when using more than four encoding threads in the - TurboVNC session. Since the Tight encoding type is limited to four zlib - streams, any encoding threads beyond the first four cannot use zlib - compression. + * Zlib introduces a significant amount of performance overhead, even when + zlib compression level 0 (no compression) is used, so TurboVNC supports a + Tight encoding extension that allows the server to bypass zlib when + encoding a particular subrectangle. The extension is enabled when a VNC + viewer advertises support for it and requests Compression Level 0. As of + this writing, TightVNC and TigerVNC do not support the extension, so the + TurboVNC Server will use zlib to "compress" framebuffer updates if you + request Compression Level 0 using the TightVNC or TigerVNC Viewer. {nl}{nl} * Refer to {ref prefix="Section ": AdvancedCompression} for a description of diff --git a/doc/config.txt b/doc/config.txt index 71e92e89f..c8c994027 100644 --- a/doc/config.txt +++ b/doc/config.txt @@ -119,7 +119,7 @@ compare full rectangles, as TurboVNC 1.2.x did. | Description :: See {ref prefix="Section ": Multithreading} | Environment Variable | {pcode: TVNC_NTHREADS = __{n}__} | -| Summary | Use __''{n}''__ threads (1 <\= __''{n}''__ <\= 8) to perform image \ +| Summary | Use __''{n}''__ threads (1 <\= __''{n}''__ <\= 4) to perform image \ encoding | | Default Value | __''{n}''__ = the number of CPU cores in the system, up to \ a maximum of 4 | diff --git a/doc/index.html b/doc/index.html index 35cafafd1..ba4093530 100644 --- a/doc/index.html +++ b/doc/index.html @@ -3,7 +3,7 @@ - + User’s Guide for TurboVNC 3.2 @@ -1299,7 +1299,7 @@

7 Performance and Image Quality

N/A N/A 0 - This encoding method uses indexed color subencoding for subrectangles that have a low number of unique colors, but it otherwise does not perform any image compression at all. Lossless Tight is thus suitable only for gigabit and faster networks. This encoding method uses significantly less CPU time than any of the JPEG-based encoding methods. Lossless Tight requires an RFB protocol extension that is, as of this writing, only supported by the TurboVNC Viewer. + This encoding method uses indexed color subencoding for subrectangles that have a low number of unique colors and raw subencoding for subrectangles that have a high number of unique colors. If the VNC viewer supports the “Tight Encoding Without Zlib” RFB extension, then zlib is bypassed, and all subrectangles are sent without compression. Otherwise, all subrectangles are “compressed” using zlib with zlib compression level 0. (Zlib compression level 0 maintains the zlib state but does not perform any actual compression. However, the overhead of maintaining the zlib state reduces overall Tight encoding performance by 10-60% vs. bypassing zlib, depending on the zlib implementation.) Lossless Tight uses significantly less CPU time than any of the JPEG-based encoding methods, but it is suitable only for gigabit and faster networks. “Lossless Tight + Zlib” @@ -1405,7 +1405,7 @@

7.2 Advanced Compression Options

1 24 No - Same as Compression Level 1. Bypassing zlib when JPEG is enabled would only reduce the CPU usage for non-JPEG subrectangles, which is of limited usefulness. Furthermore, bypassing zlib requires an RFB protocol extension that is not supported by non-TurboVNC viewers. It is presumed that, if one wants to reduce the CPU usage, then one wants to do so for all subrectangles, so CL 0 without JPEG (AKA “Lossless Tight”) should be used. + Same as Compression Level 1. Bypassing zlib when JPEG is enabled would only reduce the CPU usage for non-JPEG subrectangles, which is of limited usefulness. Furthermore, bypassing zlib requires an RFB protocol extension that is not supported by non-TurboVNC viewers (as of this writing.) It is presumed that, if one wants to reduce the CPU usage, then one wants to do so for all subrectangles, so CL 0 without JPEG (AKA “Lossless Tight”) should be used. 1 @@ -2366,15 +2366,15 @@

10.1 TightVNC or TigerVNC Servers

network usage relative to Compression Level 5.

  • - TurboVNC supports an extension to Tight encoding that allows the server - to tell the viewer to bypass zlib when decompressing a particular - subrectangle. Zlib introduces a tremendous amount of performance - overhead, even when zlib compression level 0 (no compression) is used. - Thus, when a TurboVNC viewer requests Compression Level 0 from the - TurboVNC Server, the TurboVNC Server bypasses zlib altogether. TightVNC - and TigerVNC servers do not support this extension, so they will still - use zlib to “compress” framebuffer updates even if you - request Compression Level 0.

    + Zlib introduces a significant amount of performance overhead, even when + zlib compression level 0 (no compression) is used, so TurboVNC supports + a Tight encoding extension that allows the server to bypass zlib when + encoding a particular subrectangle. The extension is enabled when a VNC + viewer advertises support for it and requests Compression Level 0. As + of this writing, TightVNC and TigerVNC do not support the extension, so + the TightVNC and TigerVNC servers will use zlib to + “compress” framebuffer updates if you request Compression + Level 0 using the TurboVNC Viewer.

  • When properly configured, version 1.2 and later (except versions 1.4.0 - @@ -2402,16 +2402,15 @@

    10.2 TightVNC or TigerVNC Viewers

    10.1.

  • - When either a TightVNC or TigerVNC viewer is connected to a TurboVNC - session and JPEG subencoding is disabled, setting the compression level - to 0 in the viewer will cause the connection to abort with a “bad - subencoding value” error. This is because the TurboVNC Server - attempts to send framebuffer updates with no zlib compression, and the - TightVNC and TigerVNC viewers don’t support that.

    A - similar issue occurs when using more than four encoding threads in the - TurboVNC session. Since the Tight encoding type is limited to four zlib - streams, any encoding threads beyond the first four cannot use zlib - compression.

    + Zlib introduces a significant amount of performance overhead, even when + zlib compression level 0 (no compression) is used, so TurboVNC supports + a Tight encoding extension that allows the server to bypass zlib when + encoding a particular subrectangle. The extension is enabled when a VNC + viewer advertises support for it and requests Compression Level 0. As + of this writing, TightVNC and TigerVNC do not support the extension, so + the TurboVNC Server will use zlib to “compress” framebuffer + updates if you request Compression Level 0 using the TightVNC or + TigerVNC Viewer.

  • Refer to Section 7.2 for @@ -2701,7 +2700,7 @@

    11.1 Server Settings

    Summary - Use {n} threads (1 <= {n} <= 8) to perform image encoding + Use {n} threads (1 <= {n} <= 4) to perform image encoding Default Value diff --git a/doc/performance.txt b/doc/performance.txt index fc8904fa1..6e8493df8 100644 --- a/doc/performance.txt +++ b/doc/performance.txt @@ -89,12 +89,17 @@ the most useful combinations of the image compression options described above: compression. | | "Lossless Tight" | No | N/A | N/A | 0 | \ This encoding method uses indexed color subencoding for subrectangles that \ - have a low number of unique colors, but it otherwise does not perform any \ - image compression at all. Lossless Tight is thus suitable only for gigabit \ - and faster networks. This encoding method uses significantly less CPU time \ - than any of the JPEG-based encoding methods. Lossless Tight requires an \ - RFB protocol extension that is, as of this writing, only supported by the \ - TurboVNC Viewer. | + have a low number of unique colors and raw subencoding for subrectangles \ + that have a high number of unique colors. If the VNC viewer supports the \ + "Tight Encoding Without Zlib" RFB extension, then zlib is bypassed, and all \ + subrectangles are sent without compression. Otherwise, all subrectangles \ + are "compressed" using zlib with zlib compression level 0. (Zlib \ + compression level 0 maintains the zlib state but does not perform any \ + actual compression. However, the overhead of maintaining the zlib state \ + reduces overall Tight encoding performance by 10-60% vs. bypassing zlib, \ + depending on the zlib implementation.) Lossless Tight uses significantly \ + less CPU time than any of the JPEG-based encoding methods, but it is \ + suitable only for gigabit and faster networks. | | "Lossless Tight + Zlib" | No | N/A | N/A | 6 | \ This encoding method uses indexed color subencoding for subrectangles that \ have a low number of unique colors and raw subencoding for subrectangles \ @@ -178,9 +183,10 @@ has the following effect: Same as Compression Level 1. Bypassing zlib when JPEG is enabled would \ only reduce the CPU usage for non-JPEG subrectangles, which is of limited \ usefulness. Furthermore, bypassing zlib requires an RFB protocol extension \ - that is not supported by non-TurboVNC viewers. It is presumed that, if one \ - wants to reduce the CPU usage, then one wants to do so for all \ - subrectangles, so CL 0 without JPEG (AKA "Lossless Tight") should be used. | + that is not supported by non-TurboVNC viewers (as of this writing.) It is \ + presumed that, if one wants to reduce the CPU usage, then one wants to do \ + so for all subrectangles, so CL 0 without JPEG (AKA "Lossless Tight") \ + should be used. | | 1 | 1 | 24 | No | \ See the description of the "Tight + JPEG" encoding methods above. | | 2 | 3 | 96 | No | \ diff --git a/unix/Xvnc/programs/Xserver/hw/vnc/rfb.h b/unix/Xvnc/programs/Xserver/hw/vnc/rfb.h index f0b4e1fdb..58304dc7d 100644 --- a/unix/Xvnc/programs/Xserver/hw/vnc/rfb.h +++ b/unix/Xvnc/programs/Xserver/hw/vnc/rfb.h @@ -87,7 +87,7 @@ /* Maximum number of threads to use for multithreaded encoding, regardless of the CPU count */ -#define MAX_ENCODING_THREADS 8 +#define MAX_ENCODING_THREADS 4 /* Maximum number of client connections. The default of 100 should be more than enough for most use cases. The ceiling is set to 500 to give us plenty @@ -443,6 +443,8 @@ typedef struct rfbClientRec { extension */ Bool enableVMwareLEDState; /* client supports VMware LED State extension */ + Bool enableTightWithoutZlib; /* client supports Tight Encoding Without + Zlib extension */ Bool useRichCursorEncoding; /* rfbEncodingRichCursor is preferred */ Bool cursorWasChanged; /* cursor shape update should be sent */ Bool cursorWasMoved; /* cursor position update should be sent */ diff --git a/unix/Xvnc/programs/Xserver/hw/vnc/rfbserver.c b/unix/Xvnc/programs/Xserver/hw/vnc/rfbserver.c index abd332438..1f501920d 100644 --- a/unix/Xvnc/programs/Xserver/hw/vnc/rfbserver.c +++ b/unix/Xvnc/programs/Xserver/hw/vnc/rfbserver.c @@ -1138,6 +1138,12 @@ static void rfbProcessClientNormalMessage(rfbClientPtr cl) cl->enableVMwareLEDState = TRUE; } break; + case rfbEncodingTightWithoutZlib: + if (!cl->enableTightWithoutZlib) { + RFBLOGID("Enabling Tight Encoding Without Zlib protocol extension\n"); + cl->enableTightWithoutZlib = TRUE; + } + break; default: if (enc >= (CARD32)rfbEncodingCompressLevel0 && enc <= (CARD32)rfbEncodingCompressLevel9) { diff --git a/unix/Xvnc/programs/Xserver/hw/vnc/tight.c b/unix/Xvnc/programs/Xserver/hw/vnc/tight.c index 87457fafd..64b1eb726 100644 --- a/unix/Xvnc/programs/Xserver/hw/vnc/tight.c +++ b/unix/Xvnc/programs/Xserver/hw/vnc/tight.c @@ -128,7 +128,7 @@ typedef struct PALETTE_s { /* Globals for multi-threading */ static Bool threadInit = FALSE; -static pthread_t thnd[MAX_ENCODING_THREADS] = { 0, 0, 0, 0, 0, 0, 0, 0 }; +static pthread_t thnd[MAX_ENCODING_THREADS] = { 0, 0, 0, 0 }; typedef struct _threadparam { rfbClientPtr cl; @@ -420,6 +420,12 @@ Bool rfbSendRectEncodingTight(rfbClientPtr cl, int x, int y, int w, int h) REGION_INIT(pScreen, &tparam[i].lossyRegion, NullBox, 0); REGION_INIT(pScreen, &tparam[i].losslessRegion, NullBox, 0); } + /* Tight encoding has only a limited number of zlib streams (4). The + streams must all be left open as long as the client is connected, or + performance suffers. Thus, multiple threads can't use the same zlib + stream. We divide the pool of 4 evenly among the available threads (up + to the first 4 threads), and if each thread has more than one stream, it + cycles between them in a round-robin fashion. */ if (i < 4) { int n = min(nt, 4); tparam[i].baseStreamId = 4 / n * i; @@ -964,7 +970,8 @@ static Bool SendMonoRect(threadparam *t, int w, int h) dataLen = (w + 7) / 8; dataLen *= h; - if (tightConf[compressLevel].monoZlibLevel == 0 || t->id > 3) + if (tightConf[compressLevel].monoZlibLevel == 0 && + cl->enableTightWithoutZlib) t->updateBuf[(*t->ublen)++] = (char)((rfbTightNoZlib | rfbTightExplicitFilter) << 4); else @@ -1032,7 +1039,7 @@ static Bool SendIndexedRect(threadparam *t, int w, int h) } /* Prepare tight encoding header. */ - if (tightConf[compressLevel].idxZlibLevel == 0 || t->id > 3) + if (tightConf[compressLevel].idxZlibLevel == 0 && cl->enableTightWithoutZlib) t->updateBuf[(*t->ublen)++] = (char)((rfbTightNoZlib | rfbTightExplicitFilter) << 4); else @@ -1098,7 +1105,7 @@ static Bool SendFullColorRect(threadparam *t, int w, int h) t->streamId = t->baseStreamId; } - if (tightConf[compressLevel].rawZlibLevel == 0 || t->id > 3) + if (tightConf[compressLevel].rawZlibLevel == 0 && cl->enableTightWithoutZlib) t->updateBuf[(*t->ublen)++] = (char)(rfbTightNoZlib << 4); else t->updateBuf[(*t->ublen)++] = streamId << 4; @@ -1130,15 +1137,7 @@ static Bool CompressData(threadparam *t, int streamId, int dataLen, return TRUE; } - /* Tight encoding has only a limited number of Zlib streams (4). The - streams must all be left open as long as the client is connected, or - performance suffers. Thus, multiple threads can't use the same Zlib - stream. We divide the pool of 4 evenly among the available threads (up - to the first 4 threads), and if each thread has more than one stream, it - cycles between them in a round-robin fashion. If we have more than 4 - threads, then threads 5 and beyond must encode their data without Zlib - compression. */ - if (zlibLevel == 0 || t->id > 3) + if (zlibLevel == 0 && cl->enableTightWithoutZlib) return SendCompressedData(t, t->tightBeforeBuf, dataLen); pz = &cl->zsStruct[streamId];