VNC: More updates from testing. There are issues with high rate, large TCP transfers.

configs/samv71-xult/README.txt

@@ -2317,13 +2317,9 @@ Configuration sub-directories
 
     NOTES:
 
-    1. The RAMLOG is enabled so all debug output will go to the RAMLOG and
+    1. Network configuration:  IP address 10.0.0.2.  The is easily changed
-       can be view using the NSH dmesg command.  No debug output is enabled
-       in the default configuration, however.
-
-    2. Network confiration:  IP address 10.0.0.2.  The is easily changed
        via 'make menuconfig'.  The VNC server address is 10.0.0.2:5900.
 
-    3. The default (local) framebuffer configuration is 320x240 with 16-bit
+    2. The default (local) framebuffer configuration is 320x240 with 16-bit
        RGB color.
 

configs/samv71-xult/vnc/defconfig

@@ -725,12 +725,7 @@ CONFIG_UART3_2STOP=0
 #
 # System Logging
 #
-CONFIG_RAMLOG=y
+# CONFIG_RAMLOG is not set
-CONFIG_RAMLOG_SYSLOG=y
-# CONFIG_RAMLOG_CONSOLE is not set
-CONFIG_RAMLOG_BUFSIZE=1024
-# CONFIG_RAMLOG_CRLF is not set
-CONFIG_RAMLOG_NONBLOCKING=y
 # CONFIG_SYSLOG_CONSOLE is not set
 
 #
@@ -746,8 +741,8 @@ CONFIG_NET_NOINTS=y
 # Driver buffer configuration
 #
 # CONFIG_NET_MULTIBUFFER is not set
-CONFIG_NET_ETH_MTU=590
+CONFIG_NET_ETH_MTU=1508
-CONFIG_NET_ETH_TCP_RECVWNDO=536
+CONFIG_NET_ETH_TCP_RECVWNDO=1454
 CONFIG_NET_GUARDSIZE=2
 
 #
@@ -834,7 +829,7 @@ CONFIG_ARP_SEND_DELAYMSEC=20
 # Network I/O Buffer Support
 #
 CONFIG_NET_IOB=y
-CONFIG_IOB_NBUFFERS=36
+CONFIG_IOB_NBUFFERS=72
 CONFIG_IOB_BUFSIZE=196
 CONFIG_IOB_NCHAINS=8
 CONFIG_IOB_THROTTLE=8
@@ -897,9 +892,8 @@ CONFIG_FS_PROCFS=y
 #
 # System Logging
 #
-CONFIG_SYSLOG=y
+# CONFIG_SYSLOG is not set
 # CONFIG_SYSLOG_TIMESTAMP is not set
-# CONFIG_SYSLOG_CHAR is not set
 
 #
 # Graphics Support
@@ -1003,7 +997,7 @@ CONFIG_VNCSERVER_COLORFMT_RGB16=y
 CONFIG_VNCSERVER_SCREENWIDTH=320
 CONFIG_VNCSERVER_SCREENHEIGHT=240
 CONFIG_VNCSERVER_NUPDATES=48
-CONFIG_VNCSERVER_UPDATE_BUFSIZE=4096
+CONFIG_VNCSERVER_UPDATE_BUFSIZE=1024
 CONFIG_VNCSERVER_INBUFFER_SIZE=80
 # CONFIG_VNCCLIENT is not set
 

graphics/vnc/server/Kconfig

@@ -98,12 +98,15 @@ config VNCSERVER_NUPDATES
 
 config VNCSERVER_UPDATE_BUFSIZE
 	int "Max update buffer size (bytes)"
-	default 4096
+	default 1024
 	---help---
 		A single buffer is pre-allocated for rendering updates.  This
 		setting specifies the maximum in bytes of that update buffer.  For
-		example, an update buffers of 32 pixels at 32-bits per pixel and
+		example, an update buffers of 32 pixels at 8-bits per pixel and
-		32-rows would yield a buffer size of 4096.
+		32-rows would yield a buffer size of 1024!
+
+		Ideally, this buffer should fit in one network packet to avoid
+		accessive re-assembly of partial TCP packets.
 
 config VNCSERVER_KBDENCODE
 	bool "Encode keyboard input"

graphics/vnc/server/vnc_server.c

@@ -90,8 +90,6 @@ FAR struct vnc_session_s *g_vnc_sessions[RFB_MAX_DISPLAYS];
 static void vnc_reset_session(FAR struct vnc_session_s *session,
                               FAR uint8_t *fb, int display)
 {
-  FAR struct vnc_fbupdate_s *curr;
-  FAR struct vnc_fbupdate_s *next;
   int i;
 
   /* Close any open sockets */
@@ -112,22 +110,13 @@ static void vnc_reset_session(FAR struct vnc_session_s *session,
   /* Put all of the pre-allocated update structures into the freelist */
 
   sq_init(&session->updqueue);
+  sq_init(&session->updfree);
 
-  session->updfree.head =
+  for (i = 0; i < CONFIG_VNCSERVER_NUPDATES; i++)
-    (FAR sq_entry_t *)&session->updpool[0];
-  session->updfree.tail =
-    (FAR sq_entry_t *)&session->updpool[CONFIG_VNCSERVER_NUPDATES-1];
-
-  next = &session->updpool[0];
-  for (i = 1; i < CONFIG_VNCSERVER_NUPDATES-1; i++)
     {
-      curr = next;
+      sq_addlast((FAR sq_entry_t *)&session->updpool[i], &session->updfree);
-      next = &session->updpool[i];
-      curr->flink = next;
     }
 
-  next->flink = NULL;
-
   /* Set the INITIALIZED state */
 
   sem_reset(&session->freesem, CONFIG_VNCSERVER_NUPDATES);

graphics/vnc/server/vnc_updater.c

@@ -51,6 +51,12 @@
 
 #include "vnc_server.h"
 
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#undef VNCSERVER_SEM_DEBUG
+
 /****************************************************************************
  * Private Types
  ****************************************************************************/
@@ -69,15 +75,73 @@ typedef CODE uint32_t(*vnc_convert32_t)(uint32_t rgb);
 #  error Unspecified/unsupported color format
 #endif
 
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+#ifdef VNCSERVER_SEM_DEBUG
+static sem_t g_dbgsem = SEM_INITIALIZER(1);
+#endif
+
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
 
+/****************************************************************************
+ * Name: vnc_sem_debug
+ *
+ * Description:
+ *   Dump information about the freesem to verify that it is sync.
+ *
+ * Input Parameters:
+ *   session - A reference to the VNC session structure.
+ *
+ * Returned Value:
+ *   A non-NULL structure pointer should always be returned.  This function
+ *   will wait if no structure is available.
+ *
+ ****************************************************************************/
+
+#ifdef VNCSERVER_SEM_DEBUG
+static void vnc_sem_debug(FAR struct vnc_session_s *session,
+                          FAR const char *msg, unsigned int unattached)
+{
+  FAR struct vnc_fbupdate_s *update;
+  unsigned int nqueued;
+  unsigned int nfree;
+
+  while (sem_wait(&g_dbgsem) < 0)
+    {
+      DEBUGASSERT(get_errno() == EINTR);
+    }
+
+  /* Count structures in the list */
+
+  for (nqueued = 0, update = (FAR struct vnc_fbupdate_s *)session->updqueue.head;
+       update != NULL;
+       nqueued++, update = update->flink);
+
+  for (nfree = 0, update = (FAR struct vnc_fbupdate_s *)session->updfree.head;
+       update != NULL;
+       nfree++, update = update->flink);
+
+  syslog(LOG_INFO, "FREESEM DEBUG: %s\n", msg);
+  syslog(LOG_INFO, "  freesem:    %d\n", session->freesem.semcount);
+  syslog(LOG_INFO, "  queued:     %u\n", nqueued);
+  syslog(LOG_INFO, "  free:       %u\n", nfree);
+  syslog(LOG_INFO, "  unattached: %u\n", unattached);
+
+  sem_post(&g_dbgsem);
+}
+#else
+#  define vnc_sem_debug(s,m,u)
+#endif
+
 /****************************************************************************
  * Name: vnc_alloc_update
  *
  * Description:
- *  Allocate one update structure by taking it from the freelist.
+ *   Allocate one update structure by taking it from the freelist.
  *
  * Input Parameters:
  *   session - A reference to the VNC session structure.
@@ -99,6 +163,8 @@ vnc_alloc_update(FAR struct vnc_session_s *session)
    */
 
   sched_lock();
+  vnc_sem_debug(session, "Before alloc", 0);
+
   while (sem_wait(&session->freesem) < 0)
     {
       DEBUGASSERT(get_errno() == EINTR);
@@ -107,6 +173,8 @@ vnc_alloc_update(FAR struct vnc_session_s *session)
   /* It is reserved.. go get it */
 
   update = (FAR struct vnc_fbupdate_s *)sq_remfirst(&session->updfree);
+
+  vnc_sem_debug(session, "After alloc", 1);
   sched_unlock();
 
   DEBUGASSERT(update != NULL);
@@ -135,6 +203,7 @@ static void vnc_free_update(FAR struct vnc_session_s *session,
    */
 
   sched_lock();
+  vnc_sem_debug(session, "Before free", 1);
 
   /* Put the entry into the free list */
 
@@ -143,6 +212,8 @@ static void vnc_free_update(FAR struct vnc_session_s *session,
   /* Post the semaphore to indicate the availability of one more update */
 
   sem_post(&session->freesem);
+
+  vnc_sem_debug(session, "After free", 0);
   DEBUGASSERT(session->freesem.semcount <= CONFIG_VNCSERVER_NUPDATES);
 
   sched_unlock();
@@ -176,6 +247,8 @@ vnc_remove_queue(FAR struct vnc_session_s *session)
    */
 
   sched_lock();
+  vnc_sem_debug(session, "Before remove", 0);
+
   while (sem_wait(&session->queuesem) < 0)
     {
       DEBUGASSERT(get_errno() == EINTR);
@@ -184,6 +257,8 @@ vnc_remove_queue(FAR struct vnc_session_s *session)
   /* It is reserved.. go get it */
 
   rect = (FAR struct vnc_fbupdate_s *)sq_remfirst(&session->updqueue);
+
+  vnc_sem_debug(session, "After remove", 0);
   sched_unlock();
 
   DEBUGASSERT(rect != NULL);
@@ -213,6 +288,7 @@ static void vnc_add_queue(FAR struct vnc_session_s *session,
    */
 
   sched_lock();
+  vnc_sem_debug(session, "Before add", 1);
 
   /* Put the entry into the list of queued rectangles. */
 
@@ -223,6 +299,8 @@ static void vnc_add_queue(FAR struct vnc_session_s *session,
    */
 
   sem_post(&session->queuesem);
+
+  vnc_sem_debug(session, "After add", 0);
   DEBUGASSERT(session->queuesem.semcount <= CONFIG_VNCSERVER_NUPDATES);
 
   sched_unlock();
@@ -666,6 +744,7 @@ static FAR void *vnc_updater(FAR void *arg)
   FAR struct vnc_session_s *session = (FAR struct vnc_session_s *)arg;
   FAR struct rfb_framebufferupdate_s *update;
   FAR struct vnc_fbupdate_s *srcrect;
+  FAR const uint8_t *src;
   nxgl_coord_t srcwidth;
   nxgl_coord_t srcheight;
   nxgl_coord_t destwidth;
@@ -849,15 +928,21 @@ static FAR void *vnc_updater(FAR void *arg)
               /* Then send the update packet to the VNC client */
 
               size += SIZEOF_RFB_FRAMEBUFFERUPDATE_S(0);
-              nsent = psock_send(&session->connect, session->outbuf, size, 0);
+              src   = session->outbuf;
-              if (nsent < 0)
-                {
-                  gdbg("ERROR: Send FrameBufferUpdate failed: %d\n",
-                       get_errno());
-                  goto errout;
-                }
 
-              DEBUGASSERT(nsent == size);
+              while (size > 0)
+                {
+                  nsent = psock_send(&session->connect, src, size, 0);
+                  if (nsent < 0)
+                    {
+                       gdbg("ERROR: Send FrameBufferUpdate failed: %d\n",
+                            get_errno());
+                      goto errout;
+                    }
+
+                  src  += nsent;
+                  size -= nsent;
+                }
             }
         }