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

@@ -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.
+

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_MTU=590
-CONFIG_NET_ETH_TCP_RECVWNDO=1454
+CONFIG_NET_ETH_TCP_RECVWNDO=536
 CONFIG_NET_GUARDSIZE=2
 
 #

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;
 }

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;

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
  *

graphics/vnc/server/vnc_updater.c

@@ -770,9 +770,26 @@ 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 */
+  /* 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?
+   */
+
+  while (session->state == VNCSERVER_RUNNING)
+    {
+      /* Get the next queued rectangle update.  This call will block until an
+       * upate is available for the case where the update queue is empty.
+       */
+
+      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.
+       */
 
-  update        = (FAR struct rfb_framebufferupdate_s *)session->outbuf;
       bytesperpixel = (session->bpp + 7) >> 3;
       maxwidth      = CONFIG_VNCSERVER_UPDATE_BUFSIZE / bytesperpixel;
 
@@ -805,21 +822,6 @@ static FAR void *vnc_updater(FAR void *arg)
             goto errout;
         }
 
-  /* Then 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?
-   */
-
-  while (session->state == VNCSERVER_RUNNING)
-    {
-      /* Get the next queued rectangle update.  This call will block until an
-       * upate is available for the case where the update queue is empty.
-       */
-
-      srcrect = vnc_remove_queue(session);
-      DEBUGASSERT(srcrect != NULL);
-
       /* 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);
             }
         }