From a2b80d6ae7a4ad2df5db657c7e23aabf3df104e4 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Fri, 23 Mar 2018 09:30:09 +0100 Subject: [PATCH] Limit lossless refresh update to safe size We don't want to waste bandwidth on the lossless refresh if we might need that bandwidth for a normal update. Try to estimate how much data we can safely send without interfering. --- common/rfb/Congestion.cxx | 15 ++++++++++++--- common/rfb/Congestion.h | 8 +++++++- common/rfb/EncodeManager.cxx | 19 +++++++++++-------- common/rfb/EncodeManager.h | 5 +++-- common/rfb/Timer.cxx | 10 ++++++++-- common/rfb/Timer.h | 6 ++++++ common/rfb/VNCSConnectionST.cxx | 16 ++++++++++++++-- common/rfb/VNCServerST.cxx | 11 +++++++++++ common/rfb/VNCServerST.h | 1 + 9 files changed, 73 insertions(+), 18 deletions(-) diff --git a/common/rfb/Congestion.cxx b/common/rfb/Congestion.cxx index a2f7a256..4d36d9f1 100644 --- a/common/rfb/Congestion.cxx +++ b/common/rfb/Congestion.cxx @@ -1,4 +1,4 @@ -/* Copyright 2009-2015 Pierre Ossman for Cendio AB +/* Copyright 2009-2018 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 @@ -73,7 +73,7 @@ static LogWriter vlog("Congestion"); Congestion::Congestion() : lastPosition(0), extraBuffer(0), baseRTT(-1), congWindow(INITIAL_WINDOW), inSlowStart(true), - measurements(0), minRTT(-1), minCongestedRTT(-1) + safeBaseRTT(-1), measurements(0), minRTT(-1), minCongestedRTT(-1) { gettimeofday(&lastUpdate, NULL); gettimeofday(&lastSent, NULL); @@ -170,7 +170,7 @@ void Congestion::gotPong() // Try to estimate wire latency by tracking lowest seen latency if (rtt < baseRTT) - baseRTT = rtt; + safeBaseRTT = baseRTT = rtt; // Pings sent before the last adjustment aren't interesting as they // aren't a measurement of the current congestion window @@ -284,6 +284,15 @@ int Congestion::getUncongestedETA() } } +size_t Congestion::getBandwidth() +{ + // No measurements yet? Guess RTT of 60 ms + if (safeBaseRTT == (unsigned)-1) + return congWindow * 1000 / 60; + + return congWindow * 1000 / safeBaseRTT; +} + void Congestion::debugTrace(const char* filename, int fd) { #ifdef CONGESTION_TRACE diff --git a/common/rfb/Congestion.h b/common/rfb/Congestion.h index fd57c22e..d2935128 100644 --- a/common/rfb/Congestion.h +++ b/common/rfb/Congestion.h @@ -1,4 +1,4 @@ -/* Copyright 2009-2015 Pierre Ossman for Cendio AB +/* Copyright 2009-2018 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 @@ -47,6 +47,10 @@ namespace rfb { // longer be congested. int getUncongestedETA(); + // getBandwidth() returns the current bandwidth estimation in bytes + // per second. + size_t getBandwidth(); + // debugTrace() writes the current congestion window, as well as the // congestion window of the underlying TCP layer, to the specified // file @@ -68,6 +72,8 @@ namespace rfb { unsigned congWindow; bool inSlowStart; + unsigned safeBaseRTT; + struct RTTInfo { struct timeval tv; unsigned pos; diff --git a/common/rfb/EncodeManager.cxx b/common/rfb/EncodeManager.cxx index ca7c7304..0ce611e9 100644 --- a/common/rfb/EncodeManager.cxx +++ b/common/rfb/EncodeManager.cxx @@ -50,8 +50,6 @@ static const int SolidSearchBlock = 16; // Don't bother with blocks smaller than this static const int SolidBlockMinArea = 2048; -static const int LosslessRefreshMaxArea = 4096; - namespace rfb { enum EncoderClass { @@ -267,9 +265,10 @@ void EncodeManager::writeUpdate(const UpdateInfo& ui, const PixelBuffer* pb, } void EncodeManager::writeLosslessRefresh(const Region& req, const PixelBuffer* pb, - const RenderedCursor* renderedCursor) + const RenderedCursor* renderedCursor, + size_t maxUpdateSize) { - doUpdate(false, getLosslessRefresh(req), + doUpdate(false, getLosslessRefresh(req, maxUpdateSize), Region(), Point(), pb, renderedCursor); } @@ -428,12 +427,16 @@ void EncodeManager::prepareEncoders(bool allowLossy) } } -Region EncodeManager::getLosslessRefresh(const Region& req) +Region EncodeManager::getLosslessRefresh(const Region& req, + size_t maxUpdateSize) { std::vector rects; Region refresh; size_t area; + // We make a conservative guess at the compression ratio at 2:1 + maxUpdateSize *= 2; + area = 0; lossyRegion.intersect(req).get_rects(&rects); while (!rects.empty()) { @@ -448,13 +451,13 @@ Region EncodeManager::getLosslessRefresh(const Region& req) // Add rects until we exceed the threshold, then include as much as // possible of the final rect - if ((area + rect.area()) > LosslessRefreshMaxArea) { + if ((area + rect.area()) > maxUpdateSize) { // Use the narrowest axis to avoid getting to thin rects if (rect.width() > rect.height()) { - int width = (LosslessRefreshMaxArea - area) / rect.height(); + int width = (maxUpdateSize - area) / rect.height(); rect.br.x = rect.tl.x + __rfbmax(1, width); } else { - int height = (LosslessRefreshMaxArea - area) / rect.width(); + int height = (maxUpdateSize - area) / rect.width(); rect.br.y = rect.tl.y + __rfbmax(1, height); } refresh.assign_union(Region(rect)); diff --git a/common/rfb/EncodeManager.h b/common/rfb/EncodeManager.h index bdff04b1..a91c544e 100644 --- a/common/rfb/EncodeManager.h +++ b/common/rfb/EncodeManager.h @@ -53,7 +53,8 @@ namespace rfb { const RenderedCursor* renderedCursor); void writeLosslessRefresh(const Region& req, const PixelBuffer* pb, - const RenderedCursor* renderedCursor); + const RenderedCursor* renderedCursor, + size_t maxUpdateSize); protected: void doUpdate(bool allowLossy, const Region& changed, @@ -62,7 +63,7 @@ namespace rfb { const RenderedCursor* renderedCursor); void prepareEncoders(bool allowLossy); - Region getLosslessRefresh(const Region& req); + Region getLosslessRefresh(const Region& req, size_t maxUpdateSize); int computeNumRects(const Region& changed); diff --git a/common/rfb/Timer.cxx b/common/rfb/Timer.cxx index 7179cd87..fd1a87ae 100644 --- a/common/rfb/Timer.cxx +++ b/common/rfb/Timer.cxx @@ -1,5 +1,5 @@ /* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. - * Copyright 2016 Pierre Ossman for Cendio AB + * Copyright 2016-2018 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 @@ -94,7 +94,7 @@ int Timer::checkTimeouts() { int Timer::getNextTimeout() { timeval now; gettimeofday(&now, 0); - int toWait = __rfbmax(1, diffTimeMillis(pending.front()->dueTime, now)); + int toWait = __rfbmax(1, pending.front()->getRemainingMs()); if (toWait > pending.front()->timeoutMs) { if (toWait - pending.front()->timeoutMs < 1000) { vlog.info("gettimeofday is broken..."); @@ -148,6 +148,12 @@ int Timer::getTimeoutMs() { return timeoutMs; } +int Timer::getRemainingMs() { + timeval now; + gettimeofday(&now, 0); + return __rfbmax(0, diffTimeMillis(pending.front()->dueTime, now)); +} + bool Timer::isBefore(timeval other) { return (dueTime.tv_sec < other.tv_sec) || ((dueTime.tv_sec == other.tv_sec) && diff --git a/common/rfb/Timer.h b/common/rfb/Timer.h index 78687d1a..15b5d03d 100644 --- a/common/rfb/Timer.h +++ b/common/rfb/Timer.h @@ -1,4 +1,5 @@ /* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. + * Copyright 2018 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 @@ -85,6 +86,11 @@ namespace rfb { // Usually used with isStarted() to get the _current_ timeout. int getTimeoutMs(); + // getRemainingMs + // Determines how many milliseconds are left before the Timer + // will timeout. Only valid for an active timer. + int getRemainingMs(); + // isBefore // Determine whether the Timer will timeout before the specified time. bool isBefore(timeval other); diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx index 77a6058f..3f92a429 100644 --- a/common/rfb/VNCSConnectionST.cxx +++ b/common/rfb/VNCSConnectionST.cxx @@ -1067,8 +1067,20 @@ void VNCSConnectionST::writeDataUpdate() if (!ui.is_empty()) encodeManager.writeUpdate(ui, server->getPixelBuffer(), cursor); - else - encodeManager.writeLosslessRefresh(req, server->getPixelBuffer(), cursor); + else { + size_t maxUpdateSize; + + // FIXME: If continuous updates aren't used then the client might + // be slower than frameRate in its requests and we could + // afford a larger update size + + // FIXME: Bandwidth estimation without congestion control + maxUpdateSize = congestion.getBandwidth() * + server->msToNextUpdate() / 1000; + + encodeManager.writeLosslessRefresh(req, server->getPixelBuffer(), + cursor, maxUpdateSize); + } writeRTTPing(); diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx index 83f2b7e7..15df71b9 100644 --- a/common/rfb/VNCServerST.cxx +++ b/common/rfb/VNCServerST.cxx @@ -593,6 +593,17 @@ void VNCServerST::stopFrameClock() frameTimer.stop(); } +int VNCServerST::msToNextUpdate() +{ + // FIXME: If the application is updating slower than frameRate then + // we could allow the clients more time here + + if (!frameTimer.isStarted()) + return 1000/rfb::Server::frameRate/2; + else + return frameTimer.getRemainingMs(); +} + // 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 diff --git a/common/rfb/VNCServerST.h b/common/rfb/VNCServerST.h index 584c48c2..9a1a44d1 100644 --- a/common/rfb/VNCServerST.h +++ b/common/rfb/VNCServerST.h @@ -227,6 +227,7 @@ namespace rfb { bool needRenderedCursor(); void startFrameClock(); void stopFrameClock(); + int msToNextUpdate(); void writeUpdate(); bool checkUpdate(); const RenderedCursor* getRenderedCursor(); -- 2.39.5