Here is the promised patch to "fix" the latency issue in VNC. The basic idea is to let the server start sending updates without requests, but doing so in a way that doesn't overload the client or the network.
WARNING! This patch uses protocol numbers that aren't properly registered and should therefore not be deployed anywhere! Rgds -- Pierre Ossman OpenSource-based Thin Client Technology System Developer Telephone: +46-13-21 46 00 Cendio AB Web: http://www.cendio.com A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing?
Index: common/rdr/FdOutStream.h =================================================================== --- common/rdr/FdOutStream.h (revision 23318) +++ common/rdr/FdOutStream.h (working copy) @@ -1,4 +1,5 @@ /* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. + * Copyright 2011 Pierre Ossman for Cendio AB * * This is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -23,6 +24,8 @@ #ifndef __RDR_FDOUTSTREAM_H__ #define __RDR_FDOUTSTREAM_H__ +#include <sys/time.h> + #include <rdr/OutStream.h> namespace rdr { @@ -43,6 +46,8 @@ int bufferUsage(); + unsigned getIdleTime(); + private: int overrun(int itemSize, int nItems); int writeWithTimeout(const void* data, int length, int timeoutms); @@ -53,6 +58,7 @@ int offset; U8* start; U8* sentUpTo; + struct timeval lastWrite; }; } Index: common/rdr/FdOutStream.cxx =================================================================== --- common/rdr/FdOutStream.cxx (revision 23318) +++ common/rdr/FdOutStream.cxx (working copy) @@ -1,4 +1,5 @@ /* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. + * Copyright 2011 Pierre Ossman for Cendio AB * * This is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -44,6 +45,7 @@ #include <rdr/FdOutStream.h> #include <rdr/Exception.h> +#include <rfb/util.h> using namespace rdr; @@ -56,6 +58,8 @@ { ptr = start = sentUpTo = new U8[bufSize]; end = start + bufSize; + + gettimeofday(&lastWrite, NULL); } FdOutStream::~FdOutStream() @@ -86,6 +90,11 @@ return ptr - sentUpTo; } +unsigned FdOutStream::getIdleTime() +{ + return rfb::msSince(&lastWrite); +} + void FdOutStream::flush() { int timeoutms_; @@ -218,5 +227,7 @@ if (n < 0) throw SystemException("write",errno); + gettimeofday(&lastWrite, NULL); + return n; } Index: common/rfb/ConnParams.h =================================================================== --- common/rfb/ConnParams.h (revision 23318) +++ common/rfb/ConnParams.h (working copy) @@ -80,6 +80,8 @@ bool supportsLastRect; bool supportsSetDesktopSize; + bool supportsFence; + bool supportsContinuousUpdates; bool customCompressLevel; int compressLevel; Index: common/rfb/CMsgWriterV3.cxx =================================================================== --- common/rfb/CMsgWriterV3.cxx (revision 23318) +++ common/rfb/CMsgWriterV3.cxx (working copy) @@ -17,6 +17,7 @@ */ #include <rdr/OutStream.h> #include <rfb/msgTypes.h> +#include <rfb/fenceTypes.h> #include <rfb/Exception.h> #include <rfb/ConnParams.h> #include <rfb/CMsgWriterV3.h> @@ -75,3 +76,41 @@ endMsg(); } + +void CMsgWriterV3::writeFence(rdr::U32 flags, unsigned len, const char data[]) +{ + if (!cp->supportsFence) + throw Exception("Server does not support fences"); + if (len > 64) + throw Exception("Too large fence payload"); + if ((flags & ~fenceFlagsSupported) != 0) + throw Exception("Unknown fence flags"); + + startMsg(msgTypeClientFence); + os->pad(3); + + os->writeU32(flags); + + os->writeU8(len); + os->writeBytes(data, len); + + endMsg(); +} + +void CMsgWriterV3::writeEnableContinuousUpdates(bool enable, + int x, int y, int w, int h) +{ + if (!cp->supportsContinuousUpdates) + throw Exception("Server does not support continuous updates"); + + startMsg(msgTypeEnableContinuousUpdates); + + os->writeU8(!!enable); + + os->writeU16(x); + os->writeU16(y); + os->writeU16(w); + os->writeU16(h); + + endMsg(); +} Index: common/rfb/CMsgReaderV3.h =================================================================== --- common/rfb/CMsgReaderV3.h (revision 23318) +++ common/rfb/CMsgReaderV3.h (working copy) @@ -32,6 +32,8 @@ virtual void readFramebufferUpdate(); virtual void readSetDesktopName(int x, int y, int w, int h); virtual void readExtendedDesktopSize(int x, int y, int w, int h); + virtual void readFence(); + virtual void readEndOfContinuousUpdates(); int nUpdateRectsLeft; }; } Index: common/rfb/SMsgWriterV3.cxx =================================================================== --- common/rfb/SMsgWriterV3.cxx (revision 23318) +++ common/rfb/SMsgWriterV3.cxx (working copy) @@ -21,6 +21,7 @@ #include <rdr/MemOutStream.h> #include <rfb/msgTypes.h> #include <rfb/screenTypes.h> +#include <rfb/fenceTypes.h> #include <rfb/Exception.h> #include <rfb/ConnParams.h> #include <rfb/SMsgWriterV3.h> @@ -61,6 +62,35 @@ os->flush(); } +void SMsgWriterV3::writeFence(rdr::U32 flags, unsigned len, const char data[]) +{ + if (!cp->supportsFence) + throw Exception("Client does not support fences"); + if (len > 64) + throw Exception("Too large fence payload"); + if ((flags & ~fenceFlagsSupported) != 0) + throw Exception("Unknown fence flags"); + + startMsg(msgTypeServerFence); + os->pad(3); + + os->writeU32(flags); + + os->writeU8(len); + os->writeBytes(data, len); + + endMsg(); +} + +void SMsgWriterV3::writeEndOfContinuousUpdates() +{ + if (!cp->supportsContinuousUpdates) + throw Exception("Client does not support continuous updates"); + + startMsg(msgTypeEndOfContinuousUpdates); + endMsg(); +} + bool SMsgWriterV3::writeSetDesktopSize() { if (!cp->supportsDesktopResize) return false; needSetDesktopSize = true; Index: common/rfb/SMsgReaderV3.h =================================================================== --- common/rfb/SMsgReaderV3.h (revision 23318) +++ common/rfb/SMsgReaderV3.h (working copy) @@ -30,6 +30,8 @@ virtual void readMsg(); protected: virtual void readSetDesktopSize(); + virtual void readFence(); + virtual void readEnableContinuousUpdates(); }; } #endif Index: common/rfb/encodings.h =================================================================== --- common/rfb/encodings.h (revision 23318) +++ common/rfb/encodings.h (working copy) @@ -36,6 +36,8 @@ const int pseudoEncodingDesktopSize = -223; const int pseudoEncodingExtendedDesktopSize = -308; const int pseudoEncodingDesktopName = -307; + const int pseudoEncodingFence = -321; + const int pseudoEncodingContinuousUpdates = -320; // TightVNC-specific const int pseudoEncodingLastRect = -224; Index: common/rfb/fenceTypes.h =================================================================== --- common/rfb/fenceTypes.h (revision 0) +++ common/rfb/fenceTypes.h (revision 0) @@ -0,0 +1,36 @@ +/* Copyright 2011 Pierre Ossman for Cendio AB + * + * This is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this software; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, + * USA. + */ +#ifndef __RFB_FENCETYPES_H__ +#define __RFB_FENCETYPES_H__ + +#include <rdr/types.h> + +namespace rfb { + const rdr::U32 fenceFlagBlockBefore = 1<<0; + const rdr::U32 fenceFlagBlockAfter = 1<<1; + const rdr::U32 fenceFlagSyncNext = 1<<2; + + const rdr::U32 fenceFlagRequest = 1<<31; + + const rdr::U32 fenceFlagsSupported = (fenceFlagBlockBefore | + fenceFlagBlockAfter | + fenceFlagSyncNext | + fenceFlagRequest); +} + +#endif Index: common/rfb/VNCSConnectionST.h =================================================================== --- common/rfb/VNCSConnectionST.h (revision 23318) +++ common/rfb/VNCSConnectionST.h (working copy) @@ -34,6 +34,8 @@ #include <rfb/VNCServerST.h> #include <rfb/Timer.h> +struct RTTInfo; + namespace rfb { class VNCSConnectionST : public SConnection, public WriteSetCursorCallback, @@ -133,7 +135,12 @@ virtual void setDesktopSize(int fb_width, int fb_height, const ScreenSet& layout); virtual void setInitialColourMap(); + virtual void fence(rdr::U32 flags, unsigned len, const char data[]); + virtual void enableContinuousUpdates(bool enable, + int x, int y, int w, int h); virtual void supportsLocalCursor(); + virtual void supportsFence(); + virtual void supportsContinuousUpdates(); // setAccessRights() allows a security package to limit the access rights // of a VNCSConnectioST to the server. These access rights are applied @@ -149,7 +156,11 @@ // Internal methods + // Congestion control + void writeRTTPing(); + void handleRTTPong(const struct RTTInfo &rttInfo); bool isCongested(); + void updateCongestion(); // writeFramebufferUpdate() attempts to write a framebuffer update to the // client. @@ -168,12 +179,28 @@ bool inProcessMessages; + bool syncFence; + rdr::U32 fenceFlags; + unsigned fenceDataLen; + char *fenceData; + + unsigned baseRTT; + unsigned congWindow; + int ackedOffset, sentOffset; + + unsigned minRTT; + bool seenCongestion; + unsigned pingCounter; + Timer congestionTimer; + VNCServerST* server; SimpleUpdateTracker updates; TransImageGetter image_getter; Region requested; bool drawRenderedCursor, removeRenderedCursor; Rect renderedCursorRect; + bool continuousUpdates; + Region cuRegion; Timer updateTimer; Index: common/rfb/CConnection.cxx =================================================================== --- common/rfb/CConnection.cxx (revision 23318) +++ common/rfb/CConnection.cxx (working copy) @@ -18,6 +18,7 @@ #include <stdio.h> #include <string.h> #include <rfb/Exception.h> +#include <rfb/fenceTypes.h> #include <rfb/CMsgReaderV3.h> #include <rfb/CMsgWriterV3.h> #include <rfb/CSecurity.h> @@ -269,3 +270,16 @@ state_ = RFBSTATE_NORMAL; vlog.debug("initialisation done"); } + +void CConnection::fence(rdr::U32 flags, unsigned len, const char data[]) +{ + CMsgHandler::fence(flags, len, data); + + if (!(flags & fenceFlagRequest)) + return; + + // We cannot guarantee any synchronisation at this level + flags = 0; + + writer()->writeFence(flags, len, data); +} Index: common/rfb/SConnection.cxx =================================================================== --- common/rfb/SConnection.cxx (revision 23318) +++ common/rfb/SConnection.cxx (working copy) @@ -20,6 +20,7 @@ #include <rfb/Exception.h> #include <rfb/Security.h> #include <rfb/msgTypes.h> +#include <rfb/fenceTypes.h> #include <rfb/SMsgReaderV3.h> #include <rfb/SMsgWriterV3.h> #include <rfb/SConnection.h> @@ -329,3 +330,19 @@ } } } + +void SConnection::fence(rdr::U32 flags, unsigned len, const char data[]) +{ + if (!(flags & fenceFlagRequest)) + return; + + // We cannot guarantee any synchronisation at this level + flags = 0; + + writer()->writeFence(flags, len, data); +} + +void SConnection::enableContinuousUpdates(bool enable, + int x, int y, int w, int h) +{ +} Index: common/rfb/CMsgWriter.cxx =================================================================== --- common/rfb/CMsgWriter.cxx (revision 23318) +++ common/rfb/CMsgWriter.cxx (working copy) @@ -60,6 +60,7 @@ { int nEncodings = 0; rdr::U32 encodings[encodingMax+3]; + if (cp->supportsLocalCursor) encodings[nEncodings++] = pseudoEncodingCursor; if (cp->supportsDesktopResize) @@ -68,9 +69,15 @@ encodings[nEncodings++] = pseudoEncodingExtendedDesktopSize; if (cp->supportsDesktopRename) encodings[nEncodings++] = pseudoEncodingDesktopName; + + encodings[nEncodings++] = pseudoEncodingLastRect; + encodings[nEncodings++] = pseudoEncodingContinuousUpdates; + encodings[nEncodings++] = pseudoEncodingFence; + if (Decoder::supported(preferredEncoding)) { encodings[nEncodings++] = preferredEncoding; } + if (useCopyRect) { encodings[nEncodings++] = encodingCopyRect; } @@ -106,7 +113,6 @@ } } - encodings[nEncodings++] = pseudoEncodingLastRect; if (cp->customCompressLevel && cp->compressLevel >= 0 && cp->compressLevel <= 9) encodings[nEncodings++] = pseudoEncodingCompressLevel0 + cp->compressLevel; if (!cp->noJpeg && cp->qualityLevel >= 0 && cp->qualityLevel <= 9) Index: common/rfb/CConnection.h =================================================================== --- common/rfb/CConnection.h (revision 23318) +++ common/rfb/CConnection.h (working copy) @@ -137,6 +137,13 @@ void setState(stateEnum s) { state_ = s; } private: + // This is a default implementation of fences that automatically + // responds to requests, stating no support for synchronisation. + // When overriding, call CMsgHandler::fence() directly in order to + // state correct support for fence flags. + virtual void fence(rdr::U32 flags, unsigned len, const char data[]); + + private: void processVersionMsg(); void processSecurityTypesMsg(); void processSecurityMsg(); Index: common/rfb/SConnection.h =================================================================== --- common/rfb/SConnection.h (revision 23318) +++ common/rfb/SConnection.h (working copy) @@ -106,6 +106,17 @@ // accepts the server's default pixel format and it uses a colour map. virtual void setInitialColourMap(); + // fence() is called when we get a fence request or response. By default + // it responds directly to requests (stating it doesn't support any + // synchronisation) and drops responses. Override to implement more proper + // support. + virtual void fence(rdr::U32 flags, unsigned len, const char data[]); + + // enableContinuousUpdates() is called when the client wants to enable + // or disable continuous updates, or change the active area. + virtual void enableContinuousUpdates(bool enable, + int x, int y, int w, int h); + // setAccessRights() allows a security package to limit the access rights // of a VNCSConnectionST to the server. How the access rights are treated // is up to the derived class. Index: common/rfb/CMsgWriter.h =================================================================== --- common/rfb/CMsgWriter.h (revision 23318) +++ common/rfb/CMsgWriter.h (working copy) @@ -44,6 +44,9 @@ virtual void writeSetDesktopSize(int width, int height, const ScreenSet& layout)=0; + virtual void writeFence(rdr::U32 flags, unsigned len, const char data[])=0; + virtual void writeEnableContinuousUpdates(bool enable, + int x, int y, int w, int h)=0; // CMsgWriter implemented methods virtual void writeSetPixelFormat(const PixelFormat& pf); Index: common/rfb/SMsgWriter.h =================================================================== --- common/rfb/SMsgWriter.h (revision 23318) +++ common/rfb/SMsgWriter.h (working copy) @@ -65,6 +65,13 @@ virtual void writeBell(); virtual void writeServerCutText(const char* str, int len); + // writeFence() sends a new fence request or response to the client. + virtual void writeFence(rdr::U32 flags, unsigned len, const char data[])=0; + + // writeEndOfContinuousUpdates() indicates that we have left continuous + // updates mode. + virtual void writeEndOfContinuousUpdates()=0; + // setupCurrentEncoder() should be called before each framebuffer update, // prior to calling getNumRects() or writeFramebufferUpdateStart(). void setupCurrentEncoder(); Index: common/rfb/msgTypes.h =================================================================== --- common/rfb/msgTypes.h (revision 23318) +++ common/rfb/msgTypes.h (working copy) @@ -26,6 +26,10 @@ const int msgTypeBell = 2; const int msgTypeServerCutText = 3; + const int msgTypeEndOfContinuousUpdates = 150; + + const int msgTypeServerFence = 230; + // client to server const int msgTypeSetPixelFormat = 0; @@ -36,6 +40,10 @@ const int msgTypePointerEvent = 5; const int msgTypeClientCutText = 6; + const int msgTypeEnableContinuousUpdates = 150; + + const int msgTypeClientFence = 230; + const int msgTypeSetDesktopSize = 251; } #endif Index: common/rfb/ConnParams.cxx =================================================================== --- common/rfb/ConnParams.cxx (revision 23318) +++ common/rfb/ConnParams.cxx (working copy) @@ -33,7 +33,8 @@ supportsLocalCursor(false), supportsLocalXCursor(false), supportsDesktopResize(false), supportsExtendedDesktopSize(false), supportsDesktopRename(false), supportsLastRect(false), - supportsSetDesktopSize(false), + supportsSetDesktopSize(false), supportsFence(false), + supportsContinuousUpdates(false), customCompressLevel(false), compressLevel(6), noJpeg(false), qualityLevel(-1), fineQualityLevel(-1), subsampling(SUBSAMP_UNDEFINED), @@ -125,6 +126,10 @@ supportsDesktopRename = true; else if (encodings[i] == pseudoEncodingLastRect) supportsLastRect = true; + else if (encodings[i] == pseudoEncodingFence) + supportsFence = true; + else if (encodings[i] == pseudoEncodingContinuousUpdates) + supportsContinuousUpdates = true; else if (encodings[i] >= pseudoEncodingCompressLevel0 && encodings[i] <= pseudoEncodingCompressLevel9) { customCompressLevel = true; Index: common/rfb/CMsgHandler.h =================================================================== --- common/rfb/CMsgHandler.h (revision 23318) +++ common/rfb/CMsgHandler.h (working copy) @@ -53,6 +53,8 @@ void* data, void* mask) = 0; virtual void setPixelFormat(const PixelFormat& pf); virtual void setName(const char* name); + virtual void fence(rdr::U32 flags, unsigned len, const char data[]); + virtual void endOfContinuousUpdates(); virtual void serverInit() = 0; virtual void framebufferUpdateStart() = 0; Index: common/rfb/CMsgReaderV3.cxx =================================================================== --- common/rfb/CMsgReaderV3.cxx (revision 23318) +++ common/rfb/CMsgReaderV3.cxx (working copy) @@ -60,6 +60,8 @@ case msgTypeSetColourMapEntries: readSetColourMapEntries(); break; case msgTypeBell: readBell(); break; case msgTypeServerCutText: readServerCutText(); break; + case msgTypeEndOfContinuousUpdates: readEndOfContinuousUpdates(); break; + case msgTypeServerFence: readFence(); break; default: fprintf(stderr, "unknown message type %d\n", type); @@ -144,3 +146,29 @@ handler->setExtendedDesktopSize(x, y, w, h, layout); } +void CMsgReaderV3::readFence() +{ + rdr::U32 flags; + rdr::U8 len; + char data[64]; + + is->skip(3); + + flags = is->readU32(); + + len = is->readU8(); + if (len > sizeof(data)) { + fprintf(stderr, "Ignoring fence with too large payload\n"); + is->skip(len); + return; + } + + is->readBytes(data, len); + + handler->fence(flags, len, data); +} + +void CMsgReaderV3::readEndOfContinuousUpdates() +{ + handler->endOfContinuousUpdates(); +} Index: common/rfb/SMsgHandler.h =================================================================== --- common/rfb/SMsgHandler.h (revision 23318) +++ common/rfb/SMsgHandler.h (working copy) @@ -50,6 +50,9 @@ virtual void framebufferUpdateRequest(const Rect& r, bool incremental) = 0; virtual void setDesktopSize(int fb_width, int fb_height, const ScreenSet& layout) = 0; + virtual void fence(rdr::U32 flags, unsigned len, const char data[]) = 0; + virtual void enableContinuousUpdates(bool enable, + int x, int y, int w, int h) = 0; // InputHandler interface // The InputHandler methods will be called for the corresponding messages. @@ -60,6 +63,17 @@ // specially for this purpose. virtual void supportsLocalCursor(); + // supportsFence() is called the first time we detect support for fences + // in the client. A fence message should be sent at this point to notify + // the client of server support. + virtual void supportsFence(); + + // supportsContinuousUpdates() is called the first time we detect that + // the client wants the continuous updates extension. A + // EndOfContinuousUpdates message should be sent back to the client at + // this point if it is supported. + virtual void supportsContinuousUpdates(); + ConnParams cp; }; } Index: common/rfb/VNCSConnectionST.cxx =================================================================== --- common/rfb/VNCSConnectionST.cxx (revision 23318) +++ common/rfb/VNCSConnectionST.cxx (working copy) @@ -17,11 +17,23 @@ * USA. */ +// Debug output on what the congestion control is up to +#define CONGESTION_DEBUG + +#include <sys/time.h> + +#ifdef CONGESTION_DEBUG +#include <sys/socket.h> +#include <netinet/in.h> +#include <netinet/tcp.h> +#endif + #include <network/TcpSocket.h> #include <rfb/VNCSConnectionST.h> #include <rfb/LogWriter.h> #include <rfb/Security.h> #include <rfb/screenTypes.h> +#include <rfb/fenceTypes.h> #include <rfb/ServerCore.h> #include <rfb/ComparingUpdateTracker.h> #include <rfb/KeyRemapper.h> @@ -33,11 +45,35 @@ static LogWriter vlog("VNCSConnST"); +// This window should get us going fairly fast on a decent bandwidth network. +// If it's too high, it will rapidly be reduced and stay low. +static const unsigned INITIAL_WINDOW = 16384; + +// TCP's minimal window is 3*MSS. But since we don't know the MSS, we +// make a guess at 4 KiB (it's probaly a bit higher). +static const unsigned MINIMUM_WINDOW = 4096; + +// The current default maximum window for Linux (4 MiB). Should be a good +// limit for now... +static const unsigned MAXIMUM_WINDOW = 4194304; + +struct RTTInfo { + struct timeval tv; + int offset; + unsigned inFlight; + char marker[4]; +}; + VNCSConnectionST::VNCSConnectionST(VNCServerST* server_, network::Socket *s, bool reverse) - : SConnection(reverse), sock(s), inProcessMessages(false), server(server_), + : SConnection(reverse), sock(s), inProcessMessages(false), + syncFence(false), fenceFlags(0), fenceDataLen(0), fenceData(NULL), + baseRTT(-1), minRTT(-1), + seenCongestion(false), pingCounter(0), ackedOffset(0), sentOffset(0), congWindow(0), + congestionTimer(this), server(server_), updates(false), image_getter(server->useEconomicTranslate), drawRenderedCursor(false), removeRenderedCursor(false), + continuousUpdates(false), updateTimer(this), pointerEventTime(0), accessRights(AccessDefault), startTime(time(0)) { @@ -70,6 +106,7 @@ // Remove this client from the server server->clients.remove(this); + delete [] fenceData; } @@ -121,6 +158,10 @@ while (getInStream()->checkNoWait(1)) { processMsg(); + if (syncFence) { + writer()->writeFence(fenceFlags, fenceDataLen, fenceData); + syncFence = false; + } } // Flush out everything in case we go idle after this. @@ -353,6 +394,10 @@ // - Mark the entire display as "dirty" updates.add_changed(server->pb->getRect()); startTime = time(0); + + // - Bootstrap the congestion control + ackedOffset = sock->outStream().length(); + congWindow = INITIAL_WINDOW; } void VNCSConnectionST::queryConnection(const char* userName) @@ -510,7 +555,8 @@ // Just update the requested region. // Framebuffer update will be sent a bit later, see processMessages(). Region reqRgn(r); - requested.assign_union(reqRgn); + if (!incremental || !continuousUpdates) + requested.assign_union(reqRgn); if (!incremental) { // Non-incremental update - treat as if area requested has changed @@ -566,6 +612,69 @@ setColourMapEntries(0, 0); } +void VNCSConnectionST::fence(rdr::U32 flags, unsigned len, const char data[]) +{ + if (flags & fenceFlagRequest) { + if (flags & fenceFlagSyncNext) { + if (syncFence) + vlog.error("Fence trying to synchronise another fence"); + + syncFence = true; + + fenceFlags = flags & (fenceFlagBlockBefore | fenceFlagBlockAfter | fenceFlagSyncNext); + fenceDataLen = len; + delete [] fenceData; + if (len > 0) { + fenceData = new char[len]; + memcpy(fenceData, data, len); + } + + return; + } + + // We handle everything synchronously so we trivially honor these modes + flags = flags & (fenceFlagBlockBefore | fenceFlagBlockAfter); + + writer()->writeFence(flags, len, data); + return; + } + + struct RTTInfo rttInfo; + + switch (len) { + case 0: + // Initial dummy fence; + break; + case sizeof(struct RTTInfo): + memcpy(&rttInfo, data, sizeof(struct RTTInfo)); + handleRTTPong(rttInfo); + break; + default: + vlog.error("Fence response of unexpected size received"); + } +} + +void VNCSConnectionST::enableContinuousUpdates(bool enable, + int x, int y, int w, int h) +{ + Rect rect; + + if (!cp.supportsFence || !cp.supportsContinuousUpdates) + throw Exception("Client tried to enable continuous updates when not allowed"); + + continuousUpdates = enable; + + rect.setXYWH(x, y, w, h); + cuRegion.reset(rect); + + if (enable) { + requested.clear(); + writeFramebufferUpdate(); + } else { + writer()->writeEndOfContinuousUpdates(); + } +} + // supportsLocalCursor() is called whenever the status of // cp.supportsLocalCursor has changed. If the client does now support local // cursor, we make sure that the old server-side rendered cursor is cleaned up @@ -581,6 +690,21 @@ } } +void VNCSConnectionST::supportsFence() +{ + writer()->writeFence(fenceFlagRequest, 0, NULL); +} + +void VNCSConnectionST::supportsContinuousUpdates() +{ + // We refuse to use continuous updates if we cannot monitor the buffer + // usage using fences. + if (!cp.supportsFence) + return; + + writer()->writeEndOfContinuousUpdates(); +} + void VNCSConnectionST::writeSetCursorCallback() { if (cp.supportsLocalXCursor) { @@ -621,6 +745,8 @@ try { if (t == &updateTimer) writeFramebufferUpdate(); + else if (t == &congestionTimer) + updateCongestion(); } catch (rdr::Exception& e) { close(e.str()); } @@ -629,27 +755,208 @@ } +void VNCSConnectionST::writeRTTPing() +{ + struct RTTInfo rttInfo; + + if (!cp.supportsFence) + return; + + memset(&rttInfo, 0, sizeof(struct RTTInfo)); + + gettimeofday(&rttInfo.tv, NULL); + rttInfo.offset = sock->outStream().length(); + rttInfo.inFlight = rttInfo.offset - ackedOffset; + + memset(&rttInfo.marker, '@', sizeof(rttInfo.marker)); + + // We need to make sure any old update are already processed by the + // time we get the response back. This allows us to reliably throttle + // back on client overload, as well as network overload. + writer()->writeFence(fenceFlagRequest | fenceFlagBlockBefore, + sizeof(struct RTTInfo), (const char*)&rttInfo); + + pingCounter++; + + sentOffset = rttInfo.offset; + + // Let some data flow before we adjust the settings + if (!congestionTimer.isStarted()) + congestionTimer.start(__rfbmin(baseRTT * 2, 100)); +} + +void VNCSConnectionST::handleRTTPong(const struct RTTInfo &rttInfo) +{ + unsigned rtt, delay; + int bdp; + + pingCounter--; + + rtt = msSince(&rttInfo.tv); + if (rtt < 1) + rtt = 1; + + ackedOffset = rttInfo.offset; + + // Try to estimate wire latency by tracking lowest seen latency + if (rtt < baseRTT) + baseRTT = rtt; + + if (rttInfo.inFlight > congWindow) { + seenCongestion = true; + + // Estimate added delay because of overtaxed buffers + delay = (rttInfo.inFlight - congWindow) * baseRTT / congWindow; + + if (delay < rtt) + rtt -= delay; + else + rtt = 1; + + // If we underestimate the congestion window, then we'll get a latency + // that's less than the wire latency, which will confuse other portions + // of the code. + if (rtt < baseRTT) + rtt = baseRTT; + } + + // We only keep track of the minimum latency seen (for a given interval) + // on the basis that we want to avoid continous buffer issue, but don't + // mind (or even approve of) bursts. + if (rtt < minRTT) + minRTT = rtt; +} + bool VNCSConnectionST::isCongested() { + int offset; + + // Stuff still waiting in the send buffer? if (sock->outStream().bufferUsage() > 0) return true; - return false; + if (!cp.supportsFence) + return false; + + // Idle for too long? (and no data on the wire) + // + // FIXME: This should really just be one baseRTT, but we're getting + // problems with triggering the idle timeout on each update. + // Maybe we need to use a moving average for the wire latency + // instead of baseRTT. + if ((sentOffset == ackedOffset) && + (sock->outStream().getIdleTime() > 2 * baseRTT)) { + +#ifdef CONGESTION_DEBUG + if (congWindow > INITIAL_WINDOW) + fprintf(stderr, "Reverting to initial window (%d KiB) after %d ms\n", + INITIAL_WINDOW / 1024, sock->outStream().getIdleTime()); +#endif + + // Close congestion window and allow a transfer + // FIXME: Reset baseRTT like Linux Vegas? + congWindow = __rfbmin(INITIAL_WINDOW, congWindow); + + return false; + } + + offset = sock->outStream().length(); + + // FIXME: Should we compensate for non-update data? + // (i.e. use sentOffset instead of offset) + if ((offset - ackedOffset) < congWindow) + return false; + + // If we just have one outstanding "ping", that means the client has + // started receiving our update. In order to not regress compared to + // before we had congestion avoidance, we allow another update here. + // This could further clog up the tubes, but congestion control isn't + // really working properly right now anyway as the wire would otherwise + // be idle for at least RTT/2. + if (pingCounter == 1) + return false; + + return true; } +void VNCSConnectionST::updateCongestion() +{ + unsigned diff; + + if (!seenCongestion) + return; + + diff = minRTT - baseRTT; + + if (diff > __rfbmin(100, baseRTT)) { + // Way too fast + congWindow = congWindow * baseRTT / minRTT; + } else if (diff > __rfbmin(50, baseRTT/2)) { + // Slightly too fast + congWindow -= 4096; + } else if (diff < 5) { + // Way too slow + congWindow += 8192; + } else if (diff < 25) { + // Too slow + congWindow += 4096; + } + + if (congWindow < MINIMUM_WINDOW) + congWindow = MINIMUM_WINDOW; + if (congWindow > MAXIMUM_WINDOW) + congWindow = MAXIMUM_WINDOW; + +#ifdef CONGESTION_DEBUG + fprintf(stderr, "RTT: %d ms (%d ms), Window: %d KiB, Bandwidth: %g Mbps\n", + minRTT, baseRTT, congWindow / 1024, + congWindow * 8.0 / baseRTT / 1000.0); + +#ifdef TCP_INFO + struct tcp_info tcp_info; + socklen_t tcp_info_length; + + tcp_info_length = sizeof(tcp_info); + if (getsockopt(sock->getFd(), SOL_TCP, TCP_INFO, + (void *)&tcp_info, &tcp_info_length) == 0) { + fprintf(stderr, "Socket: RTT: %d ms (+/- %d ms) Window %d KiB\n", + tcp_info.tcpi_rtt / 1000, tcp_info.tcpi_rttvar / 1000, + tcp_info.tcpi_snd_mss * tcp_info.tcpi_snd_cwnd / 1024); + } +#endif + +#endif + + minRTT = -1; + seenCongestion = false; +} + + void VNCSConnectionST::writeFramebufferUpdate() { + Region req; + UpdateInfo ui; + bool needNewUpdateInfo; + updateTimer.stop(); + // We're in the middle of processing a command that's supposed to be + // synchronised. Allowing an update to slip out right now might violate + // that synchronisation. + if (syncFence) + return; + // We try to aggregate responses, so don't send out anything whilst we // still have incoming messages. processMessages() will give us another // chance to run once things are idle. if (inProcessMessages) return; - if (state() != RFBSTATE_NORMAL || requested.is_empty()) + if (state() != RFBSTATE_NORMAL) return; + if (requested.is_empty() && !continuousUpdates) + return; // Check that we actually have some space on the link and retry in a // bit if things are congested. @@ -658,12 +965,18 @@ return; } + // In continuous mode, we will be outputting at least three distinct + // messages. We need to aggregate these in order to not clog up TCP's + // congestion window. + network::TcpSocket::cork(sock->getFd(), true); + // First take care of any updates that cannot contain framebuffer data // changes. if (writer()->needNoDataUpdate()) { writer()->writeNoDataUpdate(); requested.clear(); - return; + if (!continuousUpdates) + goto out; } updates.enable_copyrect(cp.useCopyRect); @@ -678,10 +991,14 @@ // getUpdateInfo() will normalize the `updates' object such way that its // `changed' and `copied' regions would not intersect. - UpdateInfo ui; - updates.getUpdateInfo(&ui, requested); - bool needNewUpdateInfo = false; + if (continuousUpdates) + req = cuRegion.union_(requested); + else + req = requested; + updates.getUpdateInfo(&ui, req); + needNewUpdateInfo = false; + // If the previous position of the rendered cursor overlaps the source of the // copy, then when the copy happens the corresponding rectangle in the // destination will be wrong, so add it to the changed region. @@ -708,12 +1025,12 @@ // Return if there is nothing to send the client. if (updates.is_empty() && !writer()->needFakeUpdate() && !drawRenderedCursor) - return; + goto out; // The `updates' object could change, make sure we have valid update info. if (needNewUpdateInfo) - updates.getUpdateInfo(&ui, requested); + updates.getUpdateInfo(&ui, req); // If the client needs a server-side rendered cursor, work out the cursor // rectangle. If it's empty then don't bother drawing it, but if it overlaps @@ -723,7 +1040,7 @@ if (needRenderedCursor()) { renderedCursorRect = (server->renderedCursor.getRect(server->renderedCursorTL) - .intersect(requested.get_bounding_rect())); + .intersect(req.get_bounding_rect())); if (renderedCursorRect.is_empty()) { drawRenderedCursor = false; @@ -739,7 +1056,7 @@ //if (drawRenderedCursor) { // updates.subtract(renderedCursorRect); - // updates.getUpdateInfo(&ui, requested); + // updates.getUpdateInfo(&ui, req); //} } @@ -763,16 +1080,27 @@ nRects += nUpdateRects; } } + + writeRTTPing(); writer()->writeFramebufferUpdateStart(nRects); + Region updatedRegion; writer()->writeRects(ui, &image_getter, &updatedRegion); updates.subtract(updatedRegion); + if (drawRenderedCursor) writeRenderedCursorRect(); + writer()->writeFramebufferUpdateEnd(); + + writeRTTPing(); + requested.clear(); } + +out: + network::TcpSocket::cork(sock->getFd(), false); } Index: common/rfb/SMsgReaderV3.cxx =================================================================== --- common/rfb/SMsgReaderV3.cxx (revision 23318) +++ common/rfb/SMsgReaderV3.cxx (working copy) @@ -54,6 +54,8 @@ case msgTypePointerEvent: readPointerEvent(); break; case msgTypeClientCutText: readClientCutText(); break; case msgTypeSetDesktopSize: readSetDesktopSize(); break; + case msgTypeEnableContinuousUpdates: readEnableContinuousUpdates(); break; + case msgTypeClientFence: readFence(); break; default: fprintf(stderr, "unknown message type %d\n", msgType); @@ -91,3 +93,39 @@ handler->setDesktopSize(width, height, layout); } +void SMsgReaderV3::readFence() +{ + rdr::U32 flags; + rdr::U8 len; + char data[64]; + + is->skip(3); + + flags = is->readU32(); + + len = is->readU8(); + if (len > sizeof(data)) { + fprintf(stderr, "Ignoring fence with too large payload\n"); + is->skip(len); + return; + } + + is->readBytes(data, len); + + handler->fence(flags, len, data); +} + +void SMsgReaderV3::readEnableContinuousUpdates() +{ + bool enable; + int x, y, w, h; + + enable = is->readU8(); + + x = is->readU16(); + y = is->readU16(); + w = is->readU16(); + h = is->readU16(); + + handler->enableContinuousUpdates(enable, x, y, w, h); +} Index: common/rfb/CMsgWriterV3.h =================================================================== --- common/rfb/CMsgWriterV3.h (revision 23318) +++ common/rfb/CMsgWriterV3.h (working copy) @@ -32,6 +32,9 @@ virtual void writeSetDesktopSize(int width, int height, const ScreenSet& layout); + virtual void writeFence(rdr::U32 flags, unsigned len, const char data[]); + virtual void writeEnableContinuousUpdates(bool enable, + int x, int y, int w, int h); }; } #endif Index: common/rfb/CMsgHandler.cxx =================================================================== --- common/rfb/CMsgHandler.cxx (revision 23318) +++ common/rfb/CMsgHandler.cxx (working copy) @@ -65,3 +65,12 @@ cp.setName(name); } +void CMsgHandler::fence(rdr::U32 flags, unsigned len, const char data[]) +{ + cp.supportsFence = true; +} + +void CMsgHandler::endOfContinuousUpdates() +{ + cp.supportsContinuousUpdates = true; +} Index: common/rfb/SMsgWriterV3.h =================================================================== --- common/rfb/SMsgWriterV3.h (revision 23318) +++ common/rfb/SMsgWriterV3.h (working copy) @@ -35,6 +35,8 @@ virtual void writeServerInit(); virtual void startMsg(int type); virtual void endMsg(); + virtual void writeFence(rdr::U32 flags, unsigned len, const char data[]); + virtual void writeEndOfContinuousUpdates(); virtual bool writeSetDesktopSize(); virtual bool writeExtendedDesktopSize(); virtual bool writeExtendedDesktopSize(rdr::U16 reason, rdr::U16 result, Index: common/rfb/SMsgHandler.cxx =================================================================== --- common/rfb/SMsgHandler.cxx (revision 23318) +++ common/rfb/SMsgHandler.cxx (working copy) @@ -41,14 +41,33 @@ void SMsgHandler::setEncodings(int nEncodings, rdr::S32* encodings) { + bool firstFence, firstContinuousUpdates; + + firstFence = !cp.supportsFence; + firstContinuousUpdates = !cp.supportsContinuousUpdates; + cp.setEncodings(nEncodings, encodings); + supportsLocalCursor(); + + if (cp.supportsFence && firstFence) + supportsFence(); + if (cp.supportsContinuousUpdates && firstContinuousUpdates) + supportsContinuousUpdates(); } void SMsgHandler::supportsLocalCursor() { } +void SMsgHandler::supportsFence() +{ +} + +void SMsgHandler::supportsContinuousUpdates() +{ +} + void SMsgHandler::setDesktopSize(int fb_width, int fb_height, const ScreenSet& layout) { Index: vncviewer/CConn.h =================================================================== --- vncviewer/CConn.h (revision 23318) +++ vncviewer/CConn.h (working copy) @@ -75,6 +75,8 @@ void setCursor(int width, int height, const rfb::Point& hotspot, void* data, void* mask); + void fence(rdr::U32 flags, unsigned len, const char data[]); + private: void resizeFramebuffer(); @@ -105,8 +107,11 @@ bool firstUpdate; bool pendingUpdate; + bool continuousUpdates; bool forceNonincremental; + + bool supportsSyncFence; }; #endif Index: vncviewer/CConn.cxx =================================================================== --- vncviewer/CConn.cxx (revision 23318) +++ vncviewer/CConn.cxx (working copy) @@ -33,7 +33,10 @@ #include <rfb/LogWriter.h> #include <rfb/util.h> #include <rfb/screenTypes.h> +#include <rfb/fenceTypes.h> #include <rfb/Timer.h> +#include <rdr/MemInStream.h> +#include <rdr/MemOutStream.h> #include <network/TcpSocket.h> #include <FL/Fl.H> @@ -69,8 +72,8 @@ pendingPFChange(false), currentEncoding(encodingTight), lastServerEncoding((unsigned int)-1), formatChange(false), encodingChange(false), - firstUpdate(true), pendingUpdate(false), - forceNonincremental(true) + firstUpdate(true), pendingUpdate(false), continuousUpdates(false), + forceNonincremental(true), supportsSyncFence(false) { setShared(::shared); @@ -130,10 +133,12 @@ void CConn::refreshFramebuffer() { - // FIXME: We cannot safely trigger an update request directly but must - // wait for the next update to arrive. - if (!formatChange) - forceNonincremental = true; + forceNonincremental = true; + + // Without fences, we cannot safely trigger an update request directly + // but must wait for the next update to arrive. + if (supportsSyncFence) + requestNewUpdate(); } const char *CConn::connectionInfo() @@ -297,6 +302,7 @@ // one. void CConn::framebufferUpdateStart() { + // Note: This might not be true if sync fences are supported pendingUpdate = false; requestNewUpdate(); @@ -313,6 +319,11 @@ if (firstUpdate) { int width, height; + // We need fences to make continuous updates "safe". See fence() + // for the next step. + if (cp.supportsFence) + writer()->writeFence(fenceFlagRequest | fenceFlagSyncNext, 0, NULL); + if (cp.supportsSetDesktopSize && sscanf(desktopSize.getValueStr(), "%dx%d", &width, &height) == 2) { ScreenSet layout; @@ -436,6 +447,41 @@ desktop->setCursor(width, height, hotspot, data, mask); } +void CConn::fence(rdr::U32 flags, unsigned len, const char data[]) +{ + CMsgHandler::fence(flags, len, data); + + if (flags & fenceFlagRequest) { + // We handle everything synchronously so we trivially honor these modes + flags = flags & (fenceFlagBlockBefore | fenceFlagBlockAfter); + + writer()->writeFence(flags, len, data); + return; + } + + if (len == 0) { + // Initial probe + if (flags & fenceFlagSyncNext) { + supportsSyncFence = true; + + if (cp.supportsContinuousUpdates) { + vlog.info(_("Enabling continuous updates")); + continuousUpdates = true; + writer()->writeEnableContinuousUpdates(true, 0, 0, cp.width, cp.height); + } + } + } else { + // Pixel format change + rdr::MemInStream memStream(data, len); + PixelFormat pf; + + pf.read(&memStream); + + desktop->setServerPF(pf); + cp.setPF(pf); + } +} + rdr::U8* CConn::getRawPixelsRW(const rfb::Rect& r, int* stride) { return desktop->getPixelsRW(r, stride); } @@ -443,7 +489,6 @@ desktop->damageRect(r); } - ////////////////////// Internal methods ////////////////////// void CConn::resizeFramebuffer() @@ -451,6 +496,9 @@ if (!desktop) return; + if (continuousUpdates) + writer()->writeEnableContinuousUpdates(true, 0, 0, cp.width, cp.height); + desktop->resizeFramebuffer(cp.width, cp.height); } @@ -542,7 +590,7 @@ PixelFormat pf; /* Catch incorrect requestNewUpdate calls */ - assert(pendingUpdate == false); + assert(!pendingUpdate || supportsSyncFence); if (fullColour) { pf = fullColourPF; @@ -555,12 +603,24 @@ pf = mediumColourPF; } - // New requests are sent out at the start of processing the last - // one, so we cannot switch our internal format right now (doing so - // would mean misdecoding the current update). - pendingPFChange = true; - pendingPF = pf; + if (supportsSyncFence) { + // We let the fence carry the pixel format and switch once we + // get the response back. That way we will be synchronised with + // when the server switches. + rdr::MemOutStream memStream; + pf.write(&memStream); + + writer()->writeFence(fenceFlagRequest | fenceFlagSyncNext, + memStream.length(), (const char*)memStream.data()); + } else { + // New requests are sent out at the start of processing the last + // one, so we cannot switch our internal format right now (doing so + // would mean misdecoding the current update). + pendingPFChange = true; + pendingPF = pf; + } + char str[256]; pf.print(str, 256); vlog.info(_("Using pixel format %s"),str); @@ -571,9 +631,11 @@ checkEncodings(); - pendingUpdate = true; - writer()->writeFramebufferUpdateRequest(Rect(0, 0, cp.width, cp.height), - !forceNonincremental); + if (forceNonincremental || !continuousUpdates) { + pendingUpdate = true; + writer()->writeFramebufferUpdateRequest(Rect(0, 0, cp.width, cp.height), + !forceNonincremental); + } forceNonincremental = false; } @@ -620,6 +682,12 @@ pf = mediumColourPF; } - if (!pf.equal(self->cp.pf())) + if (!pf.equal(self->cp.pf())) { self->formatChange = true; + + // Without fences, we cannot safely trigger an update request directly + // but must wait for the next update to arrive. + if (self->supportsSyncFence) + self->requestNewUpdate(); + } }
signature.asc
Description: PGP signature
------------------------------------------------------------------------------ RSA(R) Conference 2012 Save $700 by Nov 18 Register now http://p.sf.net/sfu/rsa-sfdev2dev1
_______________________________________________ Tigervnc-devel mailing list Tigervnc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tigervnc-devel