From 7134652fea6ed4673b84bb7d76beed1dbd2db3e0 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Wed, 20 Apr 2016 09:47:02 -0600 Subject: [PATCH] VNC: Fixes from debug. One issues is that VNC client is changing color formats after starting. That is now handled. --- configs/samv71-xult/README.txt | 16 ++++++ configs/samv71-xult/vnc/defconfig | 4 +- graphics/vnc/server/vnc_negotiate.c | 60 ++++++++++++++++---- graphics/vnc/server/vnc_receiver.c | 12 +++- graphics/vnc/server/vnc_server.h | 19 +++++++ graphics/vnc/server/vnc_updater.c | 85 +++++++++++++++-------------- 6 files changed, 141 insertions(+), 55 deletions(-) diff --git a/configs/samv71-xult/README.txt b/configs/samv71-xult/README.txt index 5bab0de4ed..0ada8ad010 100644 --- a/configs/samv71-xult/README.txt +++ b/configs/samv71-xult/README.txt @@ -2323,3 +2323,19 @@ Configuration sub-directories 2. The default (local) framebuffer configuration is 320x240 with 16-bit RGB color. + 3. There are complicated interactions between VNC and the network + configuration. The CONFIG_VNCSERVER_UPDATE_BUFSIZE determines the + size of update messages. That is 1024 bytes in that configuration + (the full message with the header will be a little larger). The + MTU (CONFIG_NET_ETH_MTU) is set to 590 so that a full update will + require several packets.i + + Write buffering also effects network performance. This will break + up the large updates into small (196 byte) groups. When we run out + of read-ahead buffers, then partial updates may be sent causing a + loss of synchronization. + + 4. Hint: If you are debugging using the RealVNC clint, turn off all + mouse/keyboard inputs in the options/input menu. That will make + things a little clearer. + diff --git a/configs/samv71-xult/vnc/defconfig b/configs/samv71-xult/vnc/defconfig index f3bd46b62c..4399be7327 100644 --- a/configs/samv71-xult/vnc/defconfig +++ b/configs/samv71-xult/vnc/defconfig @@ -741,8 +741,8 @@ CONFIG_NET_NOINTS=y # Driver buffer configuration # # CONFIG_NET_MULTIBUFFER is not set -CONFIG_NET_ETH_MTU=1508 -CONFIG_NET_ETH_TCP_RECVWNDO=1454 +CONFIG_NET_ETH_MTU=590 +CONFIG_NET_ETH_TCP_RECVWNDO=536 CONFIG_NET_GUARDSIZE=2 # diff --git a/graphics/vnc/server/vnc_negotiate.c b/graphics/vnc/server/vnc_negotiate.c index d50798f6f1..f543f73efb 100644 --- a/graphics/vnc/server/vnc_negotiate.c +++ b/graphics/vnc/server/vnc_negotiate.c @@ -383,10 +383,51 @@ int vnc_negotiate(FAR struct vnc_session_s *session) return -EPROTO; } - /* Check if the client request format is one that we can handle. */ + /* Instantiate the client pixel format, verifying that the client request format + * is one that we can handle. + */ - pixelfmt = &setformat->format; + ret = vnc_client_pixelformat(session, &setformat->format); + if (ret < 0) + { + /* We do not support this pixel format */ + gdbg("ERROR: PixelFormat not supported\n"); + return ret; + } + + /* Receive supported encoding types from client, but ignore them. + * we will do only raw format. + */ + + gvdbg("Receive encoding types\n"); + + (void)psock_recv(&session->connect, session->inbuf, + CONFIG_VNCSERVER_INBUFFER_SIZE, 0); + + session->state = VNCSERVER_CONFIGURED; + return OK; +} + +/**************************************************************************** + * Name: vnc_client_pixelformat + * + * Description: + * A Client-to-Sever SetPixelFormat message has been received. We need to + * immediately switch the output color format that we generate. + * + * Input Parameters: + * session - An instance of the session structure. + * pixelfmt - The pixel from from the received SetPixelFormat message + * + * Returned Value: + * Returns zero (OK) on success; a negated errno value on failure. + * + ****************************************************************************/ + +int vnc_client_pixelformat(FAR struct vnc_session_s *session, + FAR struct rfb_pixelfmt_s *pixelfmt) +{ if (pixelfmt->truecolor == 0) { /* At present, we support only TrueColor formats */ @@ -397,26 +438,31 @@ int vnc_negotiate(FAR struct vnc_session_s *session) if (pixelfmt->bpp == 8 && pixelfmt->depth == 6) { + gvdbg("Client pixel format: RGB8 2:2:2\n"); session->colorfmt = FB_FMT_RGB8_222; session->bpp = 8; } else if (pixelfmt->bpp == 8 && pixelfmt->depth == 8) { + gvdbg("Client pixel format: RGB8 3:3:2\n"); session->colorfmt = FB_FMT_RGB8_332; session->bpp = 8; } else if (pixelfmt->bpp == 16 && pixelfmt->depth == 15) { + gvdbg("Client pixel format: RGB16 5:5:5\n"); session->colorfmt = FB_FMT_RGB16_555; session->bpp = 16; } else if (pixelfmt->bpp == 16 && pixelfmt->depth == 16) { + gvdbg("Client pixel format: RGB16 5:6:5\n"); session->colorfmt = FB_FMT_RGB16_565; session->bpp = 16; } else if (pixelfmt->bpp == 32 && pixelfmt->depth == 24) { + gvdbg("Client pixel format: RGB32 8:8:8\n"); session->colorfmt = FB_FMT_RGB32; session->bpp = 32; } @@ -434,15 +480,5 @@ int vnc_negotiate(FAR struct vnc_session_s *session) return -ENOSYS; } - /* Receive supported encoding types from client, but ignore them. - * we will do only raw format. - */ - - gvdbg("Receive encoding types\n"); - - (void)psock_recv(&session->connect, session->inbuf, - CONFIG_VNCSERVER_INBUFFER_SIZE, 0); - - session->state = VNCSERVER_CONFIGURED; return OK; } diff --git a/graphics/vnc/server/vnc_receiver.c b/graphics/vnc/server/vnc_receiver.c index 2a9bfd65f1..7d269339af 100644 --- a/graphics/vnc/server/vnc_receiver.c +++ b/graphics/vnc/server/vnc_receiver.c @@ -189,7 +189,17 @@ int vnc_receiver(FAR struct vnc_session_s *session) } else { - /* REVISIT: SetPixelFormat is currently ignored */ + FAR struct rfb_setpixelformat_s *setformat = + (FAR struct rfb_setpixelformat_s *)session->inbuf; + + ret = vnc_client_pixelformat(session, &setformat->format); + if (ret < 0) + { + /* We do not support this pixel format */ + /* REVISIT: We are going to be putting garbage on the RFB */ + + gdbg("ERROR: PixelFormat not supported\n"); + } } } break; diff --git a/graphics/vnc/server/vnc_server.h b/graphics/vnc/server/vnc_server.h index c4bd08ac38..feffc55d63 100644 --- a/graphics/vnc/server/vnc_server.h +++ b/graphics/vnc/server/vnc_server.h @@ -300,6 +300,25 @@ int vnc_server(int argc, FAR char *argv[]); int vnc_negotiate(FAR struct vnc_session_s *session); +/**************************************************************************** + * Name: vnc_client_pixelformat + * + * Description: + * A Client-to-Sever SetPixelFormat message has been received. We need to + * immediately switch the output color format that we generate. + * + * Input Parameters: + * session - An instance of the session structure. + * pixelfmt - The pixel from from the received SetPixelFormat message + * + * Returned Value: + * Returns zero (OK) on success; a negated errno value on failure. + * + ****************************************************************************/ + +int vnc_client_pixelformat(FAR struct vnc_session_s *session, + FAR struct rfb_pixelfmt_s *pixelfmt); + /**************************************************************************** * Name: vnc_start_updater * diff --git a/graphics/vnc/server/vnc_updater.c b/graphics/vnc/server/vnc_updater.c index 986e081d72..5a52b6b86c 100644 --- a/graphics/vnc/server/vnc_updater.c +++ b/graphics/vnc/server/vnc_updater.c @@ -770,42 +770,7 @@ static FAR void *vnc_updater(FAR void *arg) DEBUGASSERT(session != NULL); gvdbg("Updater running for Display %d\n", session->display); - /* Set up some constant pointers and values for convenience */ - - update = (FAR struct rfb_framebufferupdate_s *)session->outbuf; - bytesperpixel = (session->bpp + 7) >> 3; - maxwidth = CONFIG_VNCSERVER_UPDATE_BUFSIZE / bytesperpixel; - - /* Set up the color conversion */ - - switch (session->colorfmt) - { - case FB_FMT_RGB8_222: - convert.bpp8 = vnc_convert_rgb8_222; - break; - - case FB_FMT_RGB8_332: - convert.bpp8 = vnc_convert_rgb8_332; - break; - - case FB_FMT_RGB16_555: - convert.bpp16 = vnc_convert_rgb16_555; - break; - - case FB_FMT_RGB16_565: - convert.bpp16 = vnc_convert_rgb16_565; - break; - - case FB_FMT_RGB32: - convert.bpp32 = vnc_convert_rgb32_888; - break; - - default: - gdbg("ERROR: Unrecognized color format: %d\n", session->colorfmt); - goto errout; - } - - /* Then loop, processing updates until we are asked to stop. + /* Loop, processing updates until we are asked to stop. * REVISIT: Probably need some kind of signal mechanism to wake up * vnc_remove_queue() in order to stop. Or perhaps a special STOP * message in the queue? @@ -820,6 +785,43 @@ static FAR void *vnc_updater(FAR void *arg) srcrect = vnc_remove_queue(session); DEBUGASSERT(srcrect != NULL); + /* Set up characteristics of the client pixel format to use on this + * update. These can change at any time if a SetPixelFormat is + * received asynchronously. + */ + + bytesperpixel = (session->bpp + 7) >> 3; + maxwidth = CONFIG_VNCSERVER_UPDATE_BUFSIZE / bytesperpixel; + + /* Set up the color conversion */ + + switch (session->colorfmt) + { + case FB_FMT_RGB8_222: + convert.bpp8 = vnc_convert_rgb8_222; + break; + + case FB_FMT_RGB8_332: + convert.bpp8 = vnc_convert_rgb8_332; + break; + + case FB_FMT_RGB16_555: + convert.bpp16 = vnc_convert_rgb16_555; + break; + + case FB_FMT_RGB16_565: + convert.bpp16 = vnc_convert_rgb16_565; + break; + + case FB_FMT_RGB32: + convert.bpp32 = vnc_convert_rgb32_888; + break; + + default: + gdbg("ERROR: Unrecognized color format: %d\n", session->colorfmt); + goto errout; + } + /* Get with width and height of the source and destination rectangles. * The source rectangle many be larger than the destination rectangle. * In that case, we will have to emit multiple rectangles. @@ -895,17 +897,17 @@ static FAR void *vnc_updater(FAR void *arg) * performing the necessary color conversions. */ - if (session->bpp == 8) + if (bytesperpixel == 1) { size = vnc_copy8(session, y, x, updheight, updwidth, convert.bpp8); } - else if (session->bpp == 16) + else if (bytesperpixel == 2) { size = vnc_copy16(session, y, x, updheight, updwidth, convert.bpp16); } - else + else /* bytesperpixel == 4 */ { size = vnc_copy32(session, y, x, updheight, updwidth, convert.bpp32); @@ -913,6 +915,7 @@ static FAR void *vnc_updater(FAR void *arg) /* Format the FramebufferUpdate message */ + update = (FAR struct rfb_framebufferupdate_s *)session->outbuf; update->msgtype = RFB_FBUPDATE_MSG; update->padding = 0; rfb_putbe16(update->nrect, 1); @@ -930,7 +933,7 @@ static FAR void *vnc_updater(FAR void *arg) size += SIZEOF_RFB_FRAMEBUFFERUPDATE_S(0); src = session->outbuf; - while (size > 0) + do { nsent = psock_send(&session->connect, src, size, 0); if (nsent < 0) @@ -940,9 +943,11 @@ static FAR void *vnc_updater(FAR void *arg) goto errout; } + DEBUGASSERT(nsent <= size); src += nsent; size -= nsent; } + while (size > 0); } }