Use compressLevel in ZRLEEncoder #1645
Conversation
|
Thanks. There is the "ZlibLevel" setting that also needs to be marked deprecated in some way. The documentation needs to be updated at least, but ideally, there is some warning to users that use this setting. |
adamhalim
force-pushed
the
zrlee-compression-level
branch
2 times, most recently
from
e7d14cc
to
8c9dd96
Compare
|
I've updated it to work in the same way the |
unix/x0vncserver/x0vncserver.cxx
Outdated
| if (compressLevel != -1) { | ||
| vlog.info("ZlibLevel is deprecated, let clients set the compression level instead."); | ||
| } | ||
| } |
There was a problem hiding this comment.
The setting is in the core library, so it is used in all servers. Which means we cannot have the warning just here in x0vncserver.
Perhaps in the constructor for the encoder? It should just be instantiated once per server.
There was a problem hiding this comment.
My mistake, completely forgot about Xvnc.
The encoders are only constructed once a client actually connects to the servers. I figured it would be nice to display a warning message as soon as the server starts instead of when a client connects, so I moved the logging to Configuration.cxx which parses arguments during startup in both x0vncserver and Xvnc.
There was a problem hiding this comment.
Reasonable concern, but I don't think it is worth breaking modularity in Configuration.cxx for. I think this is a very rarely used option. So we can live with the fact that the warning isn't perfect to keep the code simple and clean.
If we get many confused users, then we can always revisit the issue again.
There was a problem hiding this comment.
Fair enough, I've moved the warning to the ZRLEEncoder's constructor.
| @@ -31,7 +31,7 @@ | |||
|
|
|||
| using namespace rfb; | |||
|
|
|||
| IntParameter zlibLevel("ZlibLevel","Zlib compression level",-1); | |||
| IntParameter zlibLevel("ZlibLevel","[DEPRECATED] Zlib compression level",-1); | |||
There was a problem hiding this comment.
We should also stop using it so we get a clear switch to using the normal compression setting.
adamhalim
force-pushed
the
zrlee-compression-level
branch
2 times, most recently
from
e2c4471
to
de1abda
Compare
common/rfb/ZRLEEncoder.cxx
Outdated
| @@ -17,6 +17,7 @@ | |||
| * USA. | |||
| */ | |||
|
|
|||
| #include <cstdio> | |||
There was a problem hiding this comment.
Sorry, I missed this before. This doesn't look like it is used?
There was a problem hiding this comment.
You're correct, removed it.
common/rfb/ZRLEEncoder.cxx
Outdated
| if (zlibLevel != -1) { | ||
| vlog.info("Warning: The ZlibLevel option is deprecated and is " | ||
| "ignored by the server. The compression level can be set " | ||
| "by the client instead."); |
There was a problem hiding this comment.
The indentation looks like it is off here?
adamhalim
force-pushed
the
zrlee-compression-level
branch
from
de1abda
to
ef00173
Compare
This change makes the ZRLEEncoder respect a client's desired compressionLevel. The ZlibLevel option is marked deprecated and removed from the manpages.
adamhalim
force-pushed
the
zrlee-compression-level
branch
from
ef00173
to
cd78d50
Compare
Currently, setting a custom compression level in vncviewer only affects the TightEncoder. This change makes the ZRLEEncoder respect a client's desired compressLevel as well.
Tested using x0vncserver, seems to work.