Send updates with a fixed interval

This redesigns the old "deferred updates" mechanism in to a frame
clock that governs how often updates are sent out. The goal is still
the same, to aggregate updates and avoid pointless updates, all in
the name of efficiency. This model should however be more robust
against delays that sometimes causes us to miss the desired rate.

diff --git a/common/rfb/ServerCore.cxx b/common/rfb/ServerCore.cxx
index 6e221d5397855ca1aba1d85d57ea97a5e0c91704..59a7cff36b9b6d9e33d5d8e3af3388d81ba9e4ac 100644 (file)
--- a/common/rfb/ServerCore.cxx
+++ b/common/rfb/ServerCore.cxx
@@ -52,6 +52,10 @@ rfb::IntParameter rfb::Server::compareFB
  "Perform pixel comparison on framebuffer to reduce unnecessary updates "
  "(0: never, 1: always, 2: auto)",
  2);
+rfb::IntParameter rfb::Server::frameRate
+("FrameRate",
+ "The maximum number of updates per second sent to each client",
+ 60);
 rfb::BoolParameter rfb::Server::protocol3_3
 ("Protocol3.3",
  "Always use protocol version 3.3 for backwards compatibility with "

diff --git a/common/rfb/ServerCore.h b/common/rfb/ServerCore.h
index c4d7d5379f87e115e0792a28b6452b1b1acd1eca..37923cc1fcb939bc91cfa933de4a8b8bc3ddb8ee 100644 (file)
--- a/common/rfb/ServerCore.h
+++ b/common/rfb/ServerCore.h
@@ -38,6 +38,7 @@ namespace rfb {
     static IntParameter maxIdleTime;
     static IntParameter clientWaitTimeMillis;
     static IntParameter compareFB;
+    static IntParameter frameRate;
     static BoolParameter protocol3_3;
     static BoolParameter alwaysShared;
     static BoolParameter neverShared;

diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx
index 8d19acb75b1e8c6ce687d4d7b1ec779215c1f945..9b5eaf8984735f304c6da50f44f6b9480c2f65ec 100644 (file)
--- a/common/rfb/VNCSConnectionST.cxx
+++ b/common/rfb/VNCSConnectionST.cxx
@@ -609,7 +609,6 @@ void VNCSConnectionST::framebufferUpdateRequest(const Rect& r,bool incremental)
   if (!incremental) {
     // Non-incremental update - treat as if area requested has changed
     updates.add_changed(reqRgn);
-    server->comparer->add_changed(reqRgn);
 
     // And send the screen layout to the client (which, unlike the
     // framebuffer dimensions, the client doesn't get during init)
@@ -1001,9 +1000,8 @@ void VNCSConnectionST::writeDataUpdate()
 
   updates.enable_copyrect(cp.useCopyRect);
 
-  // Fetch updates from server object, and see if we are allowed to send
-  // anything right now (the framebuffer might have changed in ways we
-  // haven't yet been informed of).
+  // See if we are allowed to send anything right now (the framebuffer
+  // might have changed in ways we haven't yet been informed of).
   if (!server->checkUpdate())
     return;
 

diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx
index 80c79fc3a6bd3b2d8685e8cf4a3077d6fedfed9c..80c992f5c019403133dfeb6ad05734ada2729a44 100644 (file)
--- a/common/rfb/VNCServerST.cxx
+++ b/common/rfb/VNCServerST.cxx
@@ -66,12 +66,6 @@ using namespace rfb;
 static LogWriter slog("VNCServerST");
 LogWriter VNCServerST::connectionsLog("Connections");
 
-rfb::IntParameter deferUpdateTime("DeferUpdate",
-                                  "Time in milliseconds to defer updates",1);
-
-rfb::BoolParameter alwaysSetDeferUpdateTimer("AlwaysSetDeferUpdateTimer",
-                  "Always reset the defer update timer on every change",false);
-
 //
 // -=- VNCServerST Implementation
 //
@@ -85,7 +79,7 @@ VNCServerST::VNCServerST(const char* name_, SDesktop* desktop_)
     renderedCursorInvalid(false),
     queryConnectionHandler(0), keyRemapper(&KeyRemapper::defInstance),
     lastConnectionTime(0), disableclients(false),
-    deferTimer(this), deferPending(false)
+    frameTimer(this)
 {
   lastUserInputTime = lastDisconnectTime = time(0);
   slog.debug("creating single-threaded server %s", name.buf);
@@ -98,6 +92,9 @@ VNCServerST::~VNCServerST()
   // Close any active clients, with appropriate logging & cleanup
   closeClients("Server shutdown");
 
+  // Stop trying to render things
+  stopFrameClock();
+
   // Delete all the clients, and their sockets, and any closing sockets
   //   NB: Deleting a client implicitly removes it from the clients list
   while (!clients.empty()) {
@@ -283,6 +280,8 @@ int VNCServerST::checkTimeouts()
 void VNCServerST::blockUpdates()
 {
   blockCounter++;
+
+  stopFrameClock();
 }
 
 void VNCServerST::unblockUpdates()
@@ -291,9 +290,11 @@ void VNCServerST::unblockUpdates()
 
   blockCounter--;
 
-  // Flush out any updates we might have blocked
-  if (blockCounter == 0)
-    tryUpdate();
+  // Restart the frame clock if we have updates
+  if (blockCounter == 0) {
+    if (!comparer->is_empty())
+      startFrameClock();
+  }
 }
 
 void VNCServerST::setPixelBuffer(PixelBuffer* pb_, const ScreenSet& layout)
@@ -415,8 +416,7 @@ void VNCServerST::add_changed(const Region& region)
     return;
 
   comparer->add_changed(region);
-  startDefer();
-  tryUpdate();
+  startFrameClock();
 }
 
 void VNCServerST::add_copied(const Region& dest, const Point& delta)
@@ -425,8 +425,7 @@ void VNCServerST::add_copied(const Region& dest, const Point& delta)
     return;
 
   comparer->add_copied(dest, delta);
-  startDefer();
-  tryUpdate();
+  startFrameClock();
 }
 
 void VNCServerST::setCursor(int width, int height, const Point& newHotspot,
@@ -508,10 +507,14 @@ SConnection* VNCServerST::getSConnection(network::Socket* sock) {
 
 bool VNCServerST::handleTimeout(Timer* t)
 {
-  if (t != &deferTimer)
-    return false;
+  if (t == &frameTimer) {
+    // We keep running until we go a full interval without any updates
+    if (comparer->is_empty())
+      return false;
 
-  tryUpdate();
+    writeUpdate();
+    return true;
+  }
 
   return false;
 }
@@ -547,77 +550,41 @@ inline bool VNCServerST::needRenderedCursor()
   return false;
 }
 
-inline void VNCServerST::startDefer()
+void VNCServerST::startFrameClock()
 {
-  if (deferUpdateTime == 0)
+  if (frameTimer.isStarted())
     return;
-
-  if (deferPending && !alwaysSetDeferUpdateTimer)
+  if (blockCounter > 0)
     return;
 
-  gettimeofday(&deferStart, NULL);
-  deferTimer.start(deferUpdateTime);
-
-  deferPending = true;
-}
-
-inline bool VNCServerST::checkDefer()
-{
-  if (!deferPending)
-    return true;
-
-  if (msSince(&deferStart) >= (unsigned)deferUpdateTime)
-    return true;
-
-  return false;
+  frameTimer.start(1000/rfb::Server::frameRate);
 }
 
-void VNCServerST::tryUpdate()
+void VNCServerST::stopFrameClock()
 {
-  std::list<VNCSConnectionST*>::iterator ci, ci_next;
-
-  if (blockCounter > 0)
-    return;
-
-  if (!checkDefer())
-    return;
-
-  for (ci = clients.begin(); ci != clients.end(); ci = ci_next) {
-    ci_next = ci; ci_next++;
-    (*ci)->writeFramebufferUpdateOrClose();
-  }
+  frameTimer.stop();
 }
 
-// checkUpdate() is called just before sending an update.  It checks to see
-// what updates are pending and propagates them to the update tracker for each
-// client.  It uses the ComparingUpdateTracker's compare() method to filter out
-// areas of the screen which haven't actually changed.  It also checks the
-// state of the (server-side) rendered cursor, if necessary rendering it again
-// with the correct background.
+// writeUpdate() is called on a regular interval in order to see what
+// updates are pending and propagates them to the update tracker for
+// each client. It uses the ComparingUpdateTracker's compare() method
+// to filter out areas of the screen which haven't actually changed. It
+// also checks the state of the (server-side) rendered cursor, if
+// necessary rendering it again with the correct background.
 
-bool VNCServerST::checkUpdate()
+void VNCServerST::writeUpdate()
 {
   UpdateInfo ui;
-  comparer->getUpdateInfo(&ui, pb->getRect());
-
-  bool renderCursor = needRenderedCursor();
-
-  if (ui.is_empty() && !(renderCursor && renderedCursorInvalid))
-    return true;
+  Region toCheck;
 
-  // Block clients as the frame buffer cannot be safely accessed
-  if (blockCounter > 0)
-    return false;
-
-  // Block client from updating if we are currently deferring updates
-  if (!checkDefer())
-    return false;
+  std::list<VNCSConnectionST*>::iterator ci, ci_next;
 
-  deferPending = false;
+  assert(blockCounter == 0);
 
-  Region toCheck = ui.changed.union_(ui.copied);
+  comparer->getUpdateInfo(&ui, pb->getRect());
+  toCheck = ui.changed.union_(ui.copied);
 
-  if (renderCursor) {
+  if (needRenderedCursor()) {
     Rect clippedCursorRect
       = cursor.getRect(cursorPos.subtract(cursor.hotspot)).intersect(pb->getRect());
 
@@ -635,14 +602,28 @@ bool VNCServerST::checkUpdate()
   if (comparer->compare())
     comparer->getUpdateInfo(&ui, pb->getRect());
 
-  std::list<VNCSConnectionST*>::iterator ci, ci_next;
+  comparer->clear();
+
   for (ci = clients.begin(); ci != clients.end(); ci = ci_next) {
     ci_next = ci; ci_next++;
     (*ci)->add_copied(ui.copied, ui.copy_delta);
     (*ci)->add_changed(ui.changed);
+    (*ci)->writeFramebufferUpdateOrClose();
   }
+}
 
-  comparer->clear();
+// checkUpdate() is called by clients to see if it is safe to read from
+// the framebuffer at this time.
+
+bool VNCServerST::checkUpdate()
+{
+  // Block clients as the frame buffer cannot be safely accessed
+  if (blockCounter > 0)
+    return false;
+
+  // Block client from updating if there are pending updates
+  if (!comparer->is_empty())
+    return false;
 
   return true;
 }

diff --git a/common/rfb/VNCServerST.h b/common/rfb/VNCServerST.h
index 1caea4e139193f7ce7afb76ca4c11b5d03981adc..b5eb4150eb882a2017f56cd2d48187c6561a0150 100644 (file)
--- a/common/rfb/VNCServerST.h
+++ b/common/rfb/VNCServerST.h
@@ -227,9 +227,9 @@ namespace rfb {
     int authClientCount();
 
     bool needRenderedCursor();
-    void startDefer();
-    bool checkDefer();
-    void tryUpdate();
+    void startFrameClock();
+    void stopFrameClock();
+    void writeUpdate();
     bool checkUpdate();
     const RenderedCursor* getRenderedCursor();
 
@@ -246,9 +246,7 @@ namespace rfb {
 
     bool disableclients;
 
-    Timer deferTimer;
-    bool deferPending;
-    struct timeval deferStart;
+    Timer frameTimer;
   };
 
 };