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.11k stars 484 forks source link

Artifacts when connecting with version 0.9.12 as opposed to 0.9.11 [regression] #335

Closed swamp-fish closed 5 years ago

swamp-fish commented 5 years ago

Describe the bug When you connect to TightVNC Server with KRDC/Remmina that use libvncserver you can see strange blocky artifacts all over the screen that aren't present when you connect with older 0.9.11 version or TigerVNC(its actually alot faster)

To Reproduce Steps to reproduce the behavior:

  1. Use 0.9.12 version.
  2. Connect to someone remotely.
  3. Press some menu and drag windows
  4. Observe artifacts.

Expected Behavior Crisp and clear picture with no artifacts when you click on things or drag windows

Your environment (please complete the following information):

Additional context In some cases artifacts are more visible than the others. Also one of the machines has no artifacts when i connect to it with 0.9.12 version. In some cases artifacts are present right away once you connect in most cases though only after you press some button or dragged something

bk138 commented 5 years ago

Thanks for reporting! Can you describe how to build/get the different viewer versions?

swamp-fish commented 5 years ago

I didn't build anything, i was using packages provided by Manjaro distro maintainers. To install 0.9.12 version you can simply use: sudo pacman -S libvncserver And to try out old one you need to download it and install: wget https://archive.archlinux32.org/packages/l/libvncserver/libvncserver-0.9.11-3.1-i686.pkg.tar.xz && sudo pacman -U libvncserver-0.9.11-3.1-i686.pkg.tar.xz

From then on you can use krdc or remmina to connect to someone else: sudo pacman -S krdc remmina

bk138 commented 5 years ago

Theory: different RFB encodings used. Can you please post the log outputs of both cases (same client, which one doesn't matter...)

swamp-fish commented 5 years ago

There are no log outputs for either krdc or remmina and if i use terminal there is no useful data.

bk138 commented 5 years ago

You might be able to Google how to produce logs for said clients.

Another option is git-bisecting on libvncserver source and trying those builds with these clients.

swamp-fish commented 5 years ago

Did the bisection process: libvnc-bisect.log

bk138 commented 5 years ago

Did the bisection process: libvnc-bisect.log

Thanks! Which is one is the last bad commit?

swamp-fish commented 5 years ago

I've started out with master branch: 42649b018744eda57b37bebeb3aeae7d4d3553b8 being bad and 8415ff4c3517c6697d53e1a17bba35284f480891 being a good one. I guess this one: # bad: [436a047f56cf5f5c92d946faa6b08e3ed7aa2309] SDLvncviewer: add a very simple GetCredentials callback should be the last bad by date

bk138 commented 5 years ago

436a047f56cf5f5c92d946faa6b08e3ed7aa2309 can't be the cause. Did yout distro's build settings maybe change?

bk138 commented 5 years ago

Did the bisection process: libvnc-bisect.log

Actually, your bisect log should contain a message 'NNN is the first bad commit'. This log kinda looks incomplete :-/

joakim-tjernlund commented 5 years ago

This might be same/related to https://gitlab.com/Remmina/Remmina/issues/1824

That issue mentions https://github.com/LibVNC/libvncserver/commit/d7b14624cbb9ed7b9df3532658e1edba8da606a6 as first bad commit

bk138 commented 5 years ago

would be helpful to get some log output to find out the encoding used 🙂

swamp-fish commented 5 years ago

Did the bisection process: libvnc-bisect.log

Actually, your bisect log should contain a message 'NNN is the first bad commit'. This log kinda looks incomplete :-/ I had no such lines. But if you meant the last bad commit bisect did and not the one by date than its this one d34c7b07aca7a8c7cd0ea0fb41971992e5dc4670

swamp-fish commented 5 years ago

This might be same/related to https://gitlab.com/Remmina/Remmina/issues/1824

That issue mentions d7b1462 as first bad commit The screenshot looks awfully similar to what i get. And he solved it the same way.

swamp-fish commented 5 years ago

Hi, i've redone the test and this is what it wrote at the end:

git bisect bad
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[d7b14624cbb9ed7b9df3532658e1edba8da606a6] Include Tight decoding optimizations from TurboVNC

Apperantly i overlooked it becase in the log it doesn't mention it. Sorry for confusion. I guess this one is a bad one and there is one other person that confirms it.

Oh i got it, i should've mark it as bad then it wrotes: d7b14624cbb9ed7b9df3532658e1edba8da606a6 is the first bad commit Again, sorry.

bk138 commented 5 years ago

Nice! What happens if you revert this commit and rebuild?

swamp-fish commented 5 years ago

Yes, after reverting this single commit everything works!

joakim-tjernlund commented 5 years ago

That is one big commit though so not easy to to troubleshoot. For me I could workaround the problem using the highest quality settings Any other quality setting broke

swamp-fish commented 5 years ago

Oh thanks for the finding, i can confirm this, highest quality settings do work.

bk138 commented 5 years ago

@dcommander are you able to chime in on this and put out some theories of what's happening?

bk138 commented 5 years ago

@swamp-fish is your libvncclient linked to libjpeg-turbo or plain old libjepg?

swamp-fish commented 5 years ago

libjpeg-turbo 2.0.3

joakim-tjernlund commented 5 years ago

I recall trying libjpeg-turbo-1.5.3, 2.0.1, mater and media-libs/jpeg (http://jpegclub.org) All had this problem.

dcommander commented 5 years ago

@bk138 I'll see if I can reproduce it.

joakim-tjernlund commented 5 years ago

@dcommander , any luck reproducing it ?

dcommander commented 5 years ago

Sorry, I got distracted this week with a high-priority VirtualGL issue. I will look at this on Monday.

dcommander commented 5 years ago

I can't reproduce it, sorry. I've tried the ancient TightVNC Server v1.3.10 (on CentOS 5, because that was the only distro I could find that would run it.) I've also tried Vino. I've tried both Remmina and the SDLvncviewer example in LibVNCServer. There is apparently some specific setting or something else about the OP's configuration that I haven't reproduced yet, and I can't spend any more time hunting in the dark. Maybe this is specific to x32?

joakim-tjernlund commented 5 years ago

Well, for me, it is Remmina to a Vino server using low quality settings. Highest quality works fine.

joakim-tjernlund commented 5 years ago

Using amd64 arch as well

joakim-tjernlund commented 5 years ago

Wonder if it is connected to how turbo is buid:

multilib_src_configure() {
    if multilib_is_native_abi && use java ; then
        export JAVACFLAGS="$(java-pkg_javac-args)"
        export JNI_CFLAGS="$(java-pkg_get-jni-cflags)"
    fi

    local mycmakeargs=(
        -DCMAKE_INSTALL_DEFAULT_DOCDIR="${EPREFIX}/usr/share/doc/${PF}"
        -DENABLE_STATIC="$(usex static-libs)"
        -DWITH_JAVA="$(multilib_native_usex java)"
        -DWITH_MEM_SRCDST=ON
    )
    [[ ${ABI} == "x32" ]] && mycmakeargs+=( -DREQUIRE_SIMD=OFF ) #420239
    cmake-utils_src_configure
}

Java is on but static libs is off

dcommander commented 5 years ago

@joakim-tjernlund Static libs are only relevant for development purposes, not run-time purposes. The only relevant difference there is the fact that, for x32 builds, the libjpeg-turbo SIMD extensions are disabled (libjpeg-turbo won't have proper x32 SIMD support until version 2.1.) However, apart from slow performance, I don't see how that would matter in the context of this issue. The libjpeg-turbo unit tests, which are run as part of the continuous integration build process every time a new commit is pushed, ensure that both SIMD and non-SIMD builds maintain mathematical correctness, as well as mathematical compatibility with jpeg-6b.

dcommander commented 5 years ago

Is there any way to confirm whether this issue is specific to x32? I unfortunately can't test that.

joakim-tjernlund commented 5 years ago

Here is configure for libvnc, if that helps:

ebuild /usr/portage/net-libs/libvncserver/libvncserver-0.9.12-r2.ebuild configure
 * LibVNCServer-0.9.12.tar.gz BLAKE2B SHA512 size ;-) ...                [ ok ]
 * checking ebuild checksums ;-) ...                                     [ ok ]
 * checking auxfile checksums ;-) ...                                    [ ok ]
 * checking miscfile checksums ;-) ...                                   [ ok ]
>>> Unpacking source...
>>> Unpacking LibVNCServer-0.9.12.tar.gz to /var/tmp/portage/net-libs/libvncserver-0.9.12-r2/work
>>> Source unpacked in /var/tmp/portage/net-libs/libvncserver-0.9.12-r2/work
>>> Preparing source in /var/tmp/portage/net-libs/libvncserver-0.9.12-r2/work/libvncserver-LibVNCServer-0.9.12 ...
 * Applying libvncserver-0.9.12-cmake-libdir.patch ...                   [ ok ]
 * Applying libvncserver-0.9.12-libgcrypt.patch ...                      [ ok ]
 * Applying libvncserver-0.9.12-sparc-unaligned.patch ...                [ ok ]
>>> Source prepared.
>>> Configuring source in /var/tmp/portage/net-libs/libvncserver-0.9.12-r2/work/libvncserver-LibVNCServer-0.9.12 ...
>>> Working in BUILD_DIR: "/var/tmp/portage/net-libs/libvncserver-0.9.12-r2/work/libvncserver-0.9.12_build"
cmake -C /var/tmp/portage/net-libs/libvncserver-0.9.12-r2/work/libvncserver-0.9.12_build/gentoo_common_config.cmake -G Ninja -DCMAKE_INSTALL_PREFIX=/usr -DWITH_ZLIB=ON -DWITH_LZO=OFF -DWITH_JPEG=ON -DWITH_PNG=ON -DWITH_THREADS=ON -DWITH_GNUTLS=ON -DWITH_OPENSSL=OFF -DWITH_GCRYPT=ON -DWITH_SYSTEMD=OFF -DWITH_FFMPEG=OFF -DWITH_24BPP=ON -DWITH_IPv6=ON -DWITH_SASL=ON -DCMAKE_BUILD_TYPE=Gentoo -DCMAKE_TOOLCHAIN_FILE=/var/tmp/portage/net-libs/libvncserver-0.9.12-r2/work/libvncserver-0.9.12_build/gentoo_toolchain.cmake  /var/tmp/portage/net-libs/libvncserver-0.9.12-r2/work/libvncserver-LibVNCServer-0.9.12
loading initial cache file /var/tmp/portage/net-libs/libvncserver-0.9.12-r2/work/libvncserver-0.9.12_build/gentoo_common_config.cmake
CMake Warning (dev) at gentoo_common_config.cmake:8 (SET):
  implicitly converting 'BOOLEAN' to 'STRING' type.
This warning is for project developers.  Use -Wno-dev to suppress it.

-- The C compiler identification is GNU 8.3.0
-- The CXX compiler identification is GNU 8.3.0
-- Check for working C compiler: /usr/bin/x86_64-pc-linux-gnu-gcc
-- Check for working C compiler: /usr/bin/x86_64-pc-linux-gnu-gcc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/x86_64-pc-linux-gnu-g++
-- Check for working CXX compiler: /usr/bin/x86_64-pc-linux-gnu-g++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found ZLIB: /usr/lib/libz.so (found version "1.2.11") 
-- Found JPEG: /usr/lib/libjpeg.so (found version "62") 
-- Performing Test FOUND_LIBJPEG_TURBO
-- Performing Test FOUND_LIBJPEG_TURBO - Success
-- Found PNG: /usr/lib/libpng.so (found version "1.6.37+apng") 
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Looking for pthread_create
-- Looking for pthread_create - not found
-- Looking for pthread_create in pthreads
-- Looking for pthread_create in pthreads - not found
-- Looking for pthread_create in pthread
-- Looking for pthread_create in pthread - found
-- Found Threads: TRUE  
-- Found SDL2: /usr/lib/libSDL2.so;-lpthread  
-- Found GnuTLS: /usr/lib64/libgnutls.so (found version "3.6.7") 
-- Looking for endian.h
-- Looking for endian.h - found
-- Looking for fcntl.h
-- Looking for fcntl.h - found
-- Looking for netinet/in.h
-- Looking for netinet/in.h - found
-- Looking for sys/endian.h
-- Looking for sys/endian.h - not found
-- Looking for sys/socket.h
-- Looking for sys/socket.h - found
-- Looking for sys/stat.h
-- Looking for sys/stat.h - found
-- Looking for sys/time.h
-- Looking for sys/time.h - found
-- Looking for sys/types.h
-- Looking for sys/types.h - found
-- Looking for sys/wait.h
-- Looking for sys/wait.h - found
-- Looking for unistd.h
-- Looking for unistd.h - found
-- Looking for sys/uio.h
-- Looking for sys/uio.h - found
-- Looking for vfork.h
-- Looking for vfork.h - not found
-- Looking for ws2tcpip.h
-- Looking for ws2tcpip.h - not found
-- Looking for arpa/inet.h
-- Looking for arpa/inet.h - found
-- Looking for stdint.h
-- Looking for stdint.h - found
-- Looking for stddef.h
-- Looking for stddef.h - found
-- Looking for sys/types.h
-- Looking for sys/types.h - found
-- Looking for gettimeofday
-- Looking for gettimeofday - found
-- Looking for vfork
-- Looking for vfork - found
-- Looking for vprintf
-- Looking for vprintf - found
-- Looking for mmap
-- Looking for mmap - found
-- Looking for fork
-- Looking for fork - found
-- Looking for ftime
-- Looking for ftime - found
-- Looking for gethostbyname
-- Looking for gethostbyname - found
-- Looking for gethostname
-- Looking for gethostname - found
-- Looking for inet_ntoa
-- Looking for inet_ntoa - found
-- Looking for memmove
-- Looking for memmove - found
-- Looking for memset
-- Looking for memset - found
-- Looking for mkfifo
-- Looking for mkfifo - found
-- Looking for select
-- Looking for select - found
-- Looking for socket
-- Looking for socket - found
-- Looking for strchr
-- Looking for strchr - found
-- Looking for strcspn
-- Looking for strcspn - found
-- Looking for strdup
-- Looking for strdup - found
-- Looking for strerror
-- Looking for strerror - found
-- Looking for strstr
-- Looking for strstr - found
-- Looking for htobe64
-- Looking for htobe64 - found
-- Looking for OSSwapHostToBigInt64
-- Looking for OSSwapHostToBigInt64 - not found
-- Building crypto with Libgcrypt
-- Found libgcrypt: /usr/lib/libgcrypt.so
-- Check size of pid_t
-- Check size of pid_t - done
-- Check size of size_t
-- Check size of size_t - done
-- Check size of socklen_t
-- Check size of socklen_t - done
-- Check size of in_addr_t
-- Check size of in_addr_t - done
-- Check if the system is big endian
-- Searching 16 bit integer
-- Check size of unsigned short
-- Check size of unsigned short - done
-- Using unsigned short
-- Check if the system is big endian - little endian
-- Building with SASL: /usr/lib/libsasl2.so and /usr/include
-- <<< Gentoo configuration >>>
Build type      Gentoo
Install path    /usr
Compiler flags:
C               -O2 -pipe
C++             -O2 -pipe
Linker flags:
Executable      -Wl,-O1 -Wl,--as-needed
Module          -Wl,-O1 -Wl,--as-needed
Shared          -Wl,-O1 -Wl,--as-needed

-- Configuring done
-- Generating done
-- Build files have been written to: /var/tmp/portage/net-libs/libvncserver-0.9.12-r2/work/libvncserver-0.9.12_build
>>> Source configured.

Not sure why you thinking x32 ? I am using amd64

dcommander commented 5 years ago

Sorry, I didn't notice that detail, because the OP said x32.

dcommander commented 5 years ago

@joakim-tjernlund Can you provide specifics regarding your client and server environment? Which Linux distribution (and version) are you running on both ends? Which window manager are you running on the server?

joakim-tjernlund commented 5 years ago

Running Gentoo in both ends using MATE DE Gentoo is a rolling distro and I keep both ends up to date

dcommander commented 5 years ago

@joakim-tjernlund I don't have Gentoo, but I am using distributions of similar vintage with MATE. I don't know what else to do. Sorry.

joakim-tjernlund commented 5 years ago

And there is nothing different with how libvnc is built in Gentoo vs. your own build? I am also out of ideas now ...

dcommander commented 5 years ago

Sorry, still can't reproduce. I built LibVNCServer exactly as the Gentoo ebuild does. I tried every combination of color depth and quality. I tried the TightVNC Server (which I was finally able to make work on a more modern platform) and the TurboVNC Server and Vino (although it doesn't appear as if my Vino setup is ever using JPEG encoding-- is there a way to enable that?) I tried the Remmina v1.3.1 and v1.3.6 source code.

It would be helpful if someone could narrow down the exact Remmina settings (color depth and quality) that reproduce the issue, as well as determine whether the issue can be reproduced with the TigerVNC Server as well as Vino. That should reduce the number of variables and hopefully make it easier for me to reproduce the issue. The issue described at https://gitlab.com/Remmina/Remmina/issues/1824 appears to be a bug in the row alignment of VNC rectangles or subrectangles, possibly caused by a mismatch in pixel sizes. This could be a bug in the implementation of a particular Tight subencoding type (such as JPEG or Color Index or even the obsolete Gradient subencoding), so it's possible that my configuration just isn't hitting that bug because that particular subencoding isn't being transmitted for some reason. I'm hoping that more information on your end might help me narrow down the location of the bug so I can reproduce it myself.

antenore commented 5 years ago

Hi all.

Remmina dev/maintainer. First of all thanks a lot for taking a look at this. If I can help somehow let me know.

The Remmina VNC plugin is very poorly implemented and I'd like to fix this before the next release.

If you guys can advice on which functions I should add some logging and error checking to get start it would be very nice of you.

Thank you!!!

joakim-tjernlund commented 5 years ago

Sorry, still can't reproduce. I built LibVNCServer exactly as the Gentoo ebuild does. I tried every combination of color depth and quality. I tried the TightVNC Server (which I was finally able to make work on a more modern platform) and the TurboVNC Server and Vino (although it doesn't appear as if my Vino setup is ever using JPEG encoding-- is there a way to enable that?) I tried the Remmina v1.3.1 and v1.3.6 source code.

My Vino is using jpeg(figure that is important), here is my configure:

>>> Configuring source in /var/tmp/portage/net-misc/vino-3.22.0/work/vino-3.22.0 ...
 * econf: updating vino-3.22.0/build-aux/config.guess with /usr/share/gnuconfig/config.guess
 * econf: updating vino-3.22.0/build-aux/config.sub with /usr/share/gnuconfig/config.sub
./configure --prefix=/usr --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc --localstatedir=/var/lib --disable-dependency-tracking --disable-silent-rules --docdir=/usr/share/doc/vino-3.22.0 --htmldir=/usr/share/doc/vino-3.22.0/html --libdir=/usr/lib64 --disable-maintainer-mode --disable-schemas-compile --enable-compile-warnings=minimum --enable-ipv6 --with-gcrypt --with-secret --with-jpeg --with-gnutls --without-telepathy --without-avahi --with-zlib --with-systemduserunitdir=/usr/lib/systemd/user
configure: loading site script /usr/share/config.site
checking for a BSD-compatible install... /usr/lib/portage/python3.6/ebuild-helpers/xattr/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking whether make supports nested variables... (cached) yes
checking for x86_64-pc-linux-gnu-gcc... x86_64-pc-linux-gnu-gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether x86_64-pc-linux-gnu-gcc accepts -g... yes
checking for x86_64-pc-linux-gnu-gcc option to accept ISO C89... none needed
checking whether x86_64-pc-linux-gnu-gcc understands -c and -o together... yes
checking for style of include used by make... GNU
checking dependency style of x86_64-pc-linux-gnu-gcc... none
checking whether to enable maintainer-specific portions of Makefiles... no
checking whether gcc understands -Wall... yes
checking what warning flags to pass to the C compiler...  -Wall
checking what language compliance flags to pass to the C compiler... 
checking whether to enable debugging... no
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking how to print strings... printf
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by x86_64-pc-linux-gnu-gcc... /usr/x86_64-pc-linux-gnu/bin/ld
checking if the linker (/usr/x86_64-pc-linux-gnu/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/x86_64-pc-linux-gnu-nm -B
checking the name lister (/usr/bin/x86_64-pc-linux-gnu-nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking how to convert x86_64-pc-linux-gnu file names to x86_64-pc-linux-gnu format... func_convert_file_noop
checking how to convert x86_64-pc-linux-gnu file names to toolchain format... func_convert_file_noop
checking for /usr/x86_64-pc-linux-gnu/bin/ld option to reload object files... -r
checking for x86_64-pc-linux-gnu-objdump... x86_64-pc-linux-gnu-objdump
checking how to recognize dependent libraries... pass_all
checking for x86_64-pc-linux-gnu-dlltool... no
checking for dlltool... no
checking how to associate runtime and link libraries... printf %s\n
checking for x86_64-pc-linux-gnu-ar... x86_64-pc-linux-gnu-ar
checking for archiver @FILE support... @
checking for x86_64-pc-linux-gnu-strip... x86_64-pc-linux-gnu-strip
checking for x86_64-pc-linux-gnu-ranlib... x86_64-pc-linux-gnu-ranlib
checking command to parse /usr/bin/x86_64-pc-linux-gnu-nm -B output from x86_64-pc-linux-gnu-gcc object... ok
checking for sysroot... no
checking for a working dd... /bin/dd
checking how to truncate binary pipes... /bin/dd bs=4096 count=1
checking for x86_64-pc-linux-gnu-mt... no
checking for mt... no
checking if : is a manifest tool... no
checking how to run the C preprocessor... x86_64-pc-linux-gnu-gcc -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking for dlfcn.h... yes
checking for objdir... .libs
checking if x86_64-pc-linux-gnu-gcc supports -fno-rtti -fno-exceptions... no
checking for x86_64-pc-linux-gnu-gcc option to produce PIC... -fPIC -DPIC
checking if x86_64-pc-linux-gnu-gcc PIC flag -fPIC -DPIC works... yes
checking if x86_64-pc-linux-gnu-gcc static flag -static works... yes
checking if x86_64-pc-linux-gnu-gcc supports -c -o file.o... yes
checking if x86_64-pc-linux-gnu-gcc supports -c -o file.o... (cached) yes
checking whether the x86_64-pc-linux-gnu-gcc linker (/usr/x86_64-pc-linux-gnu/bin/ld -m elf_x86_64) supports shared libraries... yes
checking whether -lc should be explicitly linked in... no
checking dynamic linker characteristics... GNU/Linux ld.so
checking how to hardcode library paths into programs... immediate
checking whether stripping libraries is possible... yes
checking if libtool supports shared libraries... yes
checking whether to build shared libraries... yes
checking whether to build static libraries... no
checking for x86_64-pc-linux-gnu-pkg-config... /usr/bin/x86_64-pc-linux-gnu-pkg-config
checking pkg-config is at least version 0.16... yes
checking for a sed that does not truncate output... (cached) /bin/sed
checking for glib-genmarshal script... /usr/bin/glib-genmarshal
checking for glib-mkenums script... /usr/bin/glib-mkenums
checking for X... libraries , headers 
checking for gethostbyname... yes
checking for connect... yes
checking for remove... yes
checking for shmat... yes
checking for IceConnectionNumber in -lICE... yes
checking for x86_64-pc-linux-gnu-libgcrypt-config... /usr/bin/x86_64-pc-linux-gnu-libgcrypt-config
checking for LIBGCRYPT - version >= 1.1.90... yes (1.8.3)
checking zlib.h usability... yes
checking zlib.h presence... yes
checking for zlib.h... yes
checking for deflate in -lz... yes
checking jpeglib.h usability... yes
checking jpeglib.h presence... yes
checking for jpeglib.h... yes
checking for jpeg_CreateCompress in -ljpeg... yes
checking whether NLS is requested... yes
checking for intltool >= 0.50.0... 0.51.0 found
checking for intltool-update... /usr/bin/intltool-update
checking for intltool-merge... /usr/bin/intltool-merge
checking for intltool-extract... /usr/bin/intltool-extract
checking for xgettext... /usr/bin/xgettext
checking for msgmerge... /usr/bin/msgmerge
checking for msgfmt... /usr/bin/msgfmt
checking for gmsgfmt... /usr/bin/gmsgfmt
checking for perl... /usr/bin/perl
checking for perl >= 5.8.1... 5.28.2
checking for XML::Parser... ok
checking for X11/extensions/Xdamage.h... yes
checking for XDamageQueryExtension in -lXdamage... yes
checking for X11/extensions/XShm.h... yes
checking for XShmQueryExtension in -lXext... yes
checking X11/extensions/XTest.h usability... yes
checking X11/extensions/XTest.h presence... yes
checking for X11/extensions/XTest.h... yes
checking for XTestQueryExtension in -lXtst... yes
checking X11/XKBlib.h usability... yes
checking X11/XKBlib.h presence... yes
checking for X11/XKBlib.h... yes
checking for XkbQueryExtension in -lX11... yes
checking netinet/in.h usability... yes
checking netinet/in.h presence... yes
checking for netinet/in.h... yes
checking sys/time.h usability... yes
checking sys/time.h presence... yes
checking for sys/time.h... yes
checking fcntl.h usability... yes
checking fcntl.h presence... yes
checking for fcntl.h... yes
checking for unistd.h... (cached) yes
checking sys/socket.h usability... yes
checking sys/socket.h presence... yes
checking for sys/socket.h... yes
checking ifaddrs.h usability... yes
checking ifaddrs.h presence... yes
checking for ifaddrs.h... yes
checking whether byte ordering is bigendian... no
checking for inline... inline
checking for herror in -lresolv... yes
checking for library containing strerror... none required
checking for gettimeofday... yes
checking checking for IPv6 support... yes
checking for VINO_SERVER... yes
checking for EGG_SMCLIENT... yes
checking for x86_64-pc-linux-gnu-pkg-config... (cached) /usr/bin/x86_64-pc-linux-gnu-pkg-config
checking pkg-config is at least version 0.16... yes
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating Makefile
config.status: creating po/Makefile.in
config.status: creating config.h
config.status: executing depfiles commands
config.status: executing libtool commands
config.status: executing po/stamp-it commands
>>> Source configured.

It would be helpful if someone could narrow down the exact Remmina settings (color depth and quality) that reproduce the issue, as well as determine whether the issue can be reproduced with the TigerVNC Server as well as Vino. That should reduce the number of variables and hopefully make it easier for me to reproduce the issue. The issue described at https://gitlab.com/Remmina/Remmina/issues/1824 appears to be a bug in the row alignment of VNC rectangles or subrectangles, possibly caused by a mismatch in pixel sizes. This could be a bug in the implementation of a particular Tight subencoding type (such as JPEG or Color Index or even the obsolete Gradient subencoding), so it's possible that my configuration just isn't hitting that bug because that particular subencoding isn't being transmitted for some reason. I'm hoping that more information on your end might help me narrow down the location of the bug so I can reproduce it myself.

For me color 16 bpp and any Quality less than Best will show the problem. What the Quality mens for VNC I have no idea but I figure @antenore can tell you that.

antenore commented 5 years ago

As I said probably it's an issue on our side, I'll have a look at the code later, but a review from the VNC specialists (@all ) would be much much appreciated. ;-)

https://gitlab.com/Remmina/Remmina/tree/master/plugins/vnc/vnc_plugin.c

antenore commented 5 years ago

@dcommander

Here are the bits of interest:

/* Array of key/value pairs for color depths */
static gpointer colordepth_list[] =
{
    "8",  N_("256 colors (8 bpp)"),
    "15", N_("High color (15 bpp)"),
    "16", N_("High color (16 bpp)"),
    "24", N_("True color (24 bpp)"),
    "32", N_("True color (32 bpp)"),
    NULL
};

/* Array of key/value pairs for quality selection */
static gpointer quality_list[] =
{
    "0", N_("Poor (fastest)"),
    "1", N_("Medium"),
    "2", N_("Good"),
    "9", N_("Best (slowest)"),
    NULL
};
"
// ...
// ...
    switch (quality) {
    case 9:
        cl->appData.useBGR233 = 0;
        cl->appData.encodingsString = "copyrect hextile raw";
        cl->appData.compressLevel = 0;
        cl->appData.qualityLevel = 9;
        break;
    case 2:
        cl->appData.useBGR233 = 0;
        cl->appData.encodingsString = "tight zrle ultra copyrect hextile zlib corre rre raw";
        cl->appData.compressLevel = 3;
        cl->appData.qualityLevel = 7;
        break;
    case 1:
        cl->appData.useBGR233 = 0;
        cl->appData.encodingsString = "tight zrle ultra copyrect hextile zlib corre rre raw";
        cl->appData.compressLevel = 5;
        cl->appData.qualityLevel = 5;
        break;
    case 0:
    default:
        cl->appData.useBGR233 = 1;
        cl->appData.encodingsString = "tight zrle ultra copyrect hextile zlib corre rre raw";
        cl->appData.compressLevel = 9;
        cl->appData.qualityLevel = 0;
        break;
    }
// ...
// ...
    switch (colordepth) {
    case 8:
        cl->format.depth = 8;
        cl->format.bitsPerPixel = 8;
        cl->format.blueMax = 3;
        cl->format.blueShift = 6;
        cl->format.greenMax = 7;
        cl->format.greenShift = 3;
        cl->format.redMax = 7;
        cl->format.redShift = 0;
        break;
    case 16:
        cl->format.depth = 16;
        cl->format.bitsPerPixel = 16;
        cl->format.blueMax = 0x1f;
        cl->format.blueShift = 0;
        cl->format.greenMax = 0x3f;
        cl->format.greenShift = 5;
        cl->format.redMax = 0x1f;
        cl->format.redShift = 11;
        break;
    case 15:
        cl->format.depth = 15;
        cl->format.bitsPerPixel = 16;
        cl->format.blueMax = 0x1f;
        cl->format.blueShift = 0;
        cl->format.greenMax = 0x1f;
        cl->format.greenShift = 5;
        cl->format.redMax = 0x1f;
        cl->format.redShift = 10;
        break;
    case 24:
    case 32:
    default:
        cl->format.depth = 24;
        cl->format.bitsPerPixel = 32;
        cl->format.blueShift = 0;
        cl->format.redShift = 16;
        cl->format.greenShift = 8;
        cl->format.blueMax = 0xff;
        cl->format.redMax = 0xff;
        cl->format.greenMax = 0xff;
        break;
    }

Does it make sense this code?

antenore commented 5 years ago

uh... I think this is the problematic line:

cl = rfbGetClient(8, 3, 4);

This is for the 32-bit, I should differentiate whenever I use a different amount of bits.

i.e for 16 bit

rfbGetClient(5,3,2);

Can you please help me with the others (the one that makes sense, i.e. 8 bits)

antenore commented 5 years ago

I think now I've done it right:

image

Please review everything and let me know, in the meantime I'm going to continue my hacking ;-P

antenore commented 5 years ago

This is the log I get if I set 16 bpp (best quality)

[VNC]  32 bits per pixel.
[VNC]  Least significant byte first in each pixel.
[VNC]  TRUE colour: max red 255 green 255 blue 255, shift red 16 green 8 blue 0
[VNC]Unknown message type 56 from VNC server

Why 32 bits? Is there the problem?

joakim-tjernlund commented 5 years ago

This is the log I get if I set 16 bpp (best quality)

[VNC]  32 bits per pixel.
[VNC]  Least significant byte first in each pixel.
[VNC]  TRUE colour: max red 255 green 255 blue 255, shift red 16 green 8 blue 0
[VNC]Unknown message type 56 from VNC server

Why 32 bits? Is there the problem?

I don't think so, I think Best Q don't use jpeg as cl->appData.encodingsString = "copyrect hextile raw"; doesn't specify jpeg and that is why Best Q works and the others don't

But if the VNC server doesn't offer jpeg then all Q modes will work.

antenore commented 5 years ago

@joakim-tjernlund but if I understand well:

  1. To work in lower quality I need jpeg
  2. On the client side there's nothing I can do as it's how RFB works by design

Right?

antenore commented 5 years ago

Who would like to help I've created a branch and maybe we can continue the discussion using a different channel till we will sort this out?

https://gitlab.com/Remmina/Remmina/tree/vncfixes

dcommander commented 5 years ago

@joakim-tjernlund OK, that's good information, because it means that the issue is apparently within the JPEG subencoding, but I'm still clueless as to why it's occurring. Can you determine whether it occurs only with a particular color depth?

@antenore As far as which quality levels make sense, that's a bit complicated. The old TightVNC Server for Un*x (which is long obsolete, despite the fact that some Linux distros still ship it) used the following JPEG quality and subsampling levels for the various RFB quality levels:

RFB Quality Level JPEG Quality JPEG Chroma Subsampling
0 5 4:2:0
1 10 4:2:0
2 15 4:2:0
3 25 4:2:0
4 37 4:2:0
5 50 4:2:0
6 60 4:2:0
7 70 4:2:0
8 75 4:2:0
9 80 4:2:0

The TigerVNC Server and TurboVNC Server, successors to the TightVNC Server for Un*x, map the various RFB quality levels to JPEG quality and subsampling levels that are based roughly on compression ratio across a particular set of test images:

RFB Quality Level JPEG Quality JPEG Chroma Subsampling Approximate Compression Ratio
0 15 4:2:0 100:1
1 29 4:2:0 80:1
2 41 4:2:0 70:1
3 42 4:2:2 60:1
4 62 4:2:2 50:1
5 77 4:2:2 40:1
6 79 4:4:4 30:1
7 86 4:4:4 25:1
8 92 4:4:4 20:1
9 100 4:4:4 10:1

LibVNCServer v0.9.9 and later uses the TurboVNC encoder, so it maps the RFB quality levels as indicated above. LibVNCServer prior to v0.9.9 uses the TightVNC encoder, so it maps the RFB quality levels as indicated previously. It appears that Vino is including an older version of LibVNCServer in its source tree, so it will have the same behavior (and poor performance) of the obsolete TightVNC Server for Un*x.

Quality Level 0 never makes sense, because it will produce JPEG images with such severe artifacts that text will be unreadable. I would suggest Quality Level 1 for "poor". Using 5, 7, and 9 for medium, good, and best, as you currently do, is reasonable. However, I would also suggest adding "tight" to the encodings used with "best" quality. On modern servers-- TigerVNC, TurboVNC, LibVNCServer >= 0.9.9-- this will produce perceptually lossless JPEG quality but will use much less bandwidth than Raw and produce much greater performance than Hextile.

As far as compression levels, that's also complicated, because different implementations of the Tight encoding do different things with those compression levels. TigerVNC and TightVNC map them to various zlib compression levels, but my own research (conducted in the context of integrating the TurboVNC encoder into LibVNCServer) indicated that Compression Levels 6-9 in the TigerVNC Server and TightVNC Server for Un*x are useless. Those higher compression levels do nothing but increase CPU usage for no significant gain in compression ratio. Also, CL 0 isn't meaningful in conjunction with JPEG subencoding, and most of the gain in compression ratio comes between CL 1 and CL 3 (reference: https://turbovnc.org/pmwiki/uploads/About/turbototiger.pdf). Thus, CL 9 in the TurboVNC encoder (including LibVNCServer v0.9.9+) is roughly equivalent to CL 5 in the TigerVNC and TightVNC Server, CL 0 maps to CL 1 if JPEG subencoding is enabled, and CL 3 maps to CL 2. Thus, I'd suggest using CL 1 for "best", CL 2 for "good", CL 3 for "medium", and CL 9 for "poor." That will cause "poor" to activate the highest compression in all server solutions (TightVNC, TigerVNC, TurboVNC, LibVNCServer.) "Medium" and "good" will activate the compression levels with the best "bang for the buck" in all server solutions (CL 2 and 3 are the same thing in the TurboVNC encoder and LibVNCServer v0.9.9+.) "Best" will activate minimal zlib compression.