Skip to content

Commit

Permalink
Server: Don't use Tight w/o Zlib unless advertised
Browse files Browse the repository at this point in the history 
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
9116e39 (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.
  • Loading branch information
@dcommander
dcommander committed Sep 25, 2024
1 parent 6d26cd1 commit 54cf82d
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 66 deletions.
6 changes: 6 additions & 0 deletions ChangeLog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
=====
Expand Down
3 changes: 3 additions & 0 deletions common/rfb/rfbproto.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 */

Expand Down
35 changes: 16 additions & 19 deletions doc/compatibility.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion doc/config.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
45 changes: 22 additions & 23 deletions doc/index.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<head>
<meta http-equiv="content-type" content="text/html; charset=ISO-8859-1">
<meta name="language" content="en">
<meta name="date" content="2024-09-25T15:04:57">
<meta name="date" content="2024-09-25T15:05:15">
<meta name="generator" content="deplate.rb 0.8.5">
<title>User&rsquo;s Guide for TurboVNC 3.2</title>
<link rel="start" href="index.html" title="Frontpage">
Expand Down Expand Up @@ -1299,7 +1299,7 @@ <h1 id="hd007"><a name="file007"></a>7&nbsp;Performance and Image Quality</h1>
<td class="standard">N/A</td>
<td class="standard">N/A</td>
<td class="standard">0</td>
<td class="standard">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.</td>
<td class="standard">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 &ldquo;Tight Encoding Without Zlib&rdquo; RFB extension, then zlib is bypassed, and all subrectangles are sent without compression. Otherwise, all subrectangles are &ldquo;compressed&rdquo; 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.</td>
</tr>
<tr class="standard">
<td class="standard">&ldquo;Lossless Tight + Zlib&rdquo;</td>
Expand Down Expand Up @@ -1405,7 +1405,7 @@ <h2 id="hd007002">7.2&nbsp;Advanced Compression Options</h2>
<td class="standard">1</td>
<td class="standard">24</td>
<td class="standard">No</td>
<td class="standard">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 &ldquo;Lossless Tight&rdquo;) should be used.</td>
<td class="standard">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 &ldquo;Lossless Tight&rdquo;) should be used.</td>
</tr>
<tr class="standard">
<td class="standard">1</td>
Expand Down Expand Up @@ -2366,15 +2366,15 @@ <h2 id="hd0010001">10.1&nbsp;TightVNC or TigerVNC Servers</h2>
network usage relative to Compression Level 5. <br /><br />
</li>
<li class="Itemize-1 Itemize asterisk">
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 &ldquo;compress&rdquo; framebuffer updates even if you
request Compression Level 0. <br /><br />
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
&ldquo;compress&rdquo; framebuffer updates if you request Compression
Level 0 using the TurboVNC Viewer. <br /><br />
</li>
<li class="Itemize-1 Itemize asterisk">
When properly configured, version 1.2 and later (except versions 1.4.0 -
Expand Down Expand Up @@ -2402,16 +2402,15 @@ <h2 id="hd0010002">10.2&nbsp;TightVNC or TigerVNC Viewers</h2>
<a href="#TigerVNC_JPEG_Qual" class="ref">10.1</a>. <br /><br />
</li>
<li class="Itemize-1 Itemize asterisk">
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 &ldquo;bad
subencoding value&rdquo; error. This is because the TurboVNC Server
attempts to send framebuffer updates with no zlib compression, and the
TightVNC and TigerVNC viewers don&rsquo;t support that. <br /><br /> 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. <br /><br />
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 &ldquo;compress&rdquo; framebuffer
updates if you request Compression Level 0 using the TightVNC or
TigerVNC Viewer. <br /><br />
</li>
<li class="Itemize-1 Itemize asterisk">
Refer to Section <a href="#AdvancedCompression" class="ref">7.2</a> for
Expand Down Expand Up @@ -2701,7 +2700,7 @@ <h2 id="hd0011001">11.1&nbsp;Server Settings</h2>
</tr>
<tr class="standard">
<td class="high standard">Summary</td>
<td class="standard">Use <em><code>{n}</code></em> threads (1 &lt;= <em><code>{n}</code></em> &lt;= 8) to perform image encoding</td>
<td class="standard">Use <em><code>{n}</code></em> threads (1 &lt;= <em><code>{n}</code></em> &lt;= 4) to perform image encoding</td>
</tr>
<tr class="standard">
<td class="high standard">Default Value</td>
Expand Down
24 changes: 15 additions & 9 deletions doc/performance.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down Expand Up @@ -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 | \
Expand Down
4 changes: 3 additions & 1 deletion unix/Xvnc/programs/Xserver/hw/vnc/rfb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 */
Expand Down
6 changes: 6 additions & 0 deletions unix/Xvnc/programs/Xserver/hw/vnc/rfbserver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
25 changes: 12 additions & 13 deletions unix/Xvnc/programs/Xserver/hw/vnc/tight.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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];
Expand Down

0 comments on commit 54cf82d

Please sign in to comment.