From fdba3fe884d5b43e07d7d49033c83f2f11bf524c Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Fri, 31 Jan 2014 13:12:18 +0100 Subject: [PATCH] Push encoder and decoder handling down into the connection objects This keeps the reader and writer objects clean and simple protocol decoders/encoders. --- common/rfb/CMsgHandler.h | 3 +- common/rfb/CMsgReader.cxx | 24 +----------- common/rfb/CMsgReader.h | 2 - common/rfb/RREEncoder.cxx | 4 +- common/rfb/RREEncoder.h | 4 +- common/rfb/SMsgWriter.cxx | 56 ---------------------------- common/rfb/SMsgWriter.h | 34 +++-------------- common/rfb/VNCSConnectionST.cxx | 66 +++++++++++++++++++++------------ common/rfb/VNCSConnectionST.h | 4 +- vncviewer/CConn.cxx | 32 +++++++++++----- vncviewer/CConn.h | 10 +++-- 11 files changed, 87 insertions(+), 152 deletions(-) diff --git a/common/rfb/CMsgHandler.h b/common/rfb/CMsgHandler.h index d7ffd65b..8b58e0e5 100644 --- a/common/rfb/CMsgHandler.h +++ b/common/rfb/CMsgHandler.h @@ -59,8 +59,7 @@ namespace rfb { virtual void framebufferUpdateStart() = 0; virtual void framebufferUpdateEnd() = 0; - virtual void beginRect(const Rect& r, int encoding) = 0; - virtual void endRect(const Rect& r, int encoding) = 0; + virtual void dataRect(const Rect& r, int encoding) = 0; virtual void setColourMapEntries(int firstColour, int nColours, rdr::U16* rgbs) = 0; diff --git a/common/rfb/CMsgReader.cxx b/common/rfb/CMsgReader.cxx index b39fd099..37612ea3 100644 --- a/common/rfb/CMsgReader.cxx +++ b/common/rfb/CMsgReader.cxx @@ -31,16 +31,10 @@ CMsgReader::CMsgReader(CMsgHandler* handler_, rdr::InStream* is_) : imageBufIdealSize(0), handler(handler_), is(is_), imageBuf(0), imageBufSize(0), nUpdateRectsLeft(0) { - for (int i = 0; i <= encodingMax; i++) { - decoders[i] = 0; - } } CMsgReader::~CMsgReader() { - for (int i = 0; i <= encodingMax; i++) { - delete decoders[i]; - } delete [] imageBuf; } @@ -196,23 +190,7 @@ void CMsgReader::readRect(const Rect& r, int encoding) if (r.is_empty()) fprintf(stderr, "Warning: zero size rect\n"); - handler->beginRect(r, encoding); - - if (!Decoder::supported(encoding)) { - fprintf(stderr, "Unknown rect encoding %d\n", encoding); - throw Exception("Unknown rect encoding"); - } - - if (!decoders[encoding]) { - decoders[encoding] = Decoder::createDecoder(encoding, this); - if (!decoders[encoding]) { - fprintf(stderr, "Unknown rect encoding %d\n", encoding); - throw Exception("Unknown rect encoding"); - } - } - decoders[encoding]->readRect(r, handler); - - handler->endRect(r, encoding); + handler->dataRect(r, encoding); } void CMsgReader::readSetCursor(int width, int height, const Point& hotspot) diff --git a/common/rfb/CMsgReader.h b/common/rfb/CMsgReader.h index 12846678..97b57ce7 100644 --- a/common/rfb/CMsgReader.h +++ b/common/rfb/CMsgReader.h @@ -33,7 +33,6 @@ namespace rdr { class InStream; } namespace rfb { class CMsgHandler; - class Decoder; struct Rect; class CMsgReader { @@ -69,7 +68,6 @@ namespace rfb { CMsgHandler* handler; rdr::InStream* is; - Decoder* decoders[encodingMax+1]; rdr::U8* imageBuf; int imageBufSize; int nUpdateRectsLeft; diff --git a/common/rfb/RREEncoder.cxx b/common/rfb/RREEncoder.cxx index b37b7588..dc2d74ed 100644 --- a/common/rfb/RREEncoder.cxx +++ b/common/rfb/RREEncoder.cxx @@ -33,7 +33,7 @@ using namespace rfb; #include #undef BPP -RREEncoder::RREEncoder(SMsgWriter* writer) : Encoder(writer) +RREEncoder::RREEncoder(SMsgWriter* writer) : RawEncoder(writer) { } @@ -58,7 +58,7 @@ void RREEncoder::writeRect(const Rect& r, TransImageGetter* ig) } if (nSubrects < 0) { - writer->writeRect(r, encodingRaw, ig); + RawEncoder::writeRect(r, ig); return; } diff --git a/common/rfb/RREEncoder.h b/common/rfb/RREEncoder.h index c5e65569..b263967e 100644 --- a/common/rfb/RREEncoder.h +++ b/common/rfb/RREEncoder.h @@ -19,11 +19,11 @@ #define __RFB_RREENCODER_H__ #include -#include +#include namespace rfb { - class RREEncoder : public Encoder { + class RREEncoder : public RawEncoder { public: RREEncoder(SMsgWriter* writer); virtual ~RREEncoder(); diff --git a/common/rfb/SMsgWriter.cxx b/common/rfb/SMsgWriter.cxx index 5dc7c22f..67590fe6 100644 --- a/common/rfb/SMsgWriter.cxx +++ b/common/rfb/SMsgWriter.cxx @@ -42,7 +42,6 @@ SMsgWriter::SMsgWriter(ConnParams* cp_, rdr::OutStream* os_) imageBuf(0), imageBufSize(0) { for (int i = 0; i <= encodingMax; i++) { - encoders[i] = 0; bytesSent[i] = 0; rectsSent[i] = 0; } @@ -53,7 +52,6 @@ SMsgWriter::~SMsgWriter() vlog.info("framebuffer updates %d",updatesSent); int bytes = 0; for (int i = 0; i <= encodingMax; i++) { - delete encoders[i]; if (i != encodingCopyRect) bytes += bytesSent[i]; if (rectsSent[i]) @@ -135,32 +133,6 @@ void SMsgWriter::writeEndOfContinuousUpdates() endMsg(); } -void SMsgWriter::setupCurrentEncoder() -{ - int encoding = cp->currentEncoding(); - - // FIXME: Code duplication, see writeRect(). - if (!encoders[encoding]) { - encoders[encoding] = Encoder::createEncoder(encoding, this); - assert(encoders[encoding]); - } - - encoders[encoding]->setCompressLevel(cp->compressLevel); - encoders[encoding]->setQualityLevel(cp->qualityLevel); - encoders[encoding]->setFineQualityLevel(cp->fineQualityLevel, - cp->subsampling); -} - -int SMsgWriter::getNumRects(const Rect &r) -{ - int encoding = cp->currentEncoding(); - - if (!encoders[encoding]) - setupCurrentEncoder(); - - return encoders[encoding]->getNumRects(r); -} - bool SMsgWriter::writeSetDesktopSize() { if (!cp->supportsDesktopResize) return false; @@ -290,20 +262,6 @@ void SMsgWriter::writeNoDataUpdate() writeFramebufferUpdateEnd(); } -void SMsgWriter::writeRects(const UpdateInfo& ui, TransImageGetter* ig) -{ - std::vector rects; - std::vector::const_iterator i; - - ui.copied.get_rects(&rects, ui.copy_delta.x <= 0, ui.copy_delta.y <= 0); - for (i = rects.begin(); i != rects.end(); i++) - writeCopyRect(*i, i->tl.x - ui.copy_delta.x, i->tl.y - ui.copy_delta.y); - - ui.changed.get_rects(&rects); - for (i = rects.begin(); i != rects.end(); i++) - writeRect(*i, ig); -} - void SMsgWriter::writeFramebufferUpdateStart(int nRects) { startMsg(msgTypeFramebufferUpdate); @@ -346,20 +304,6 @@ void SMsgWriter::writeFramebufferUpdateEnd() endMsg(); } -void SMsgWriter::writeRect(const Rect& r, TransImageGetter* ig) -{ - writeRect(r, cp->currentEncoding(), ig); -} - -void SMsgWriter::writeRect(const Rect& r, int encoding, TransImageGetter* ig) -{ - if (!encoders[encoding]) { - encoders[encoding] = Encoder::createEncoder(encoding, this); - assert(encoders[encoding]); - } - encoders[encoding]->writeRect(r, ig); -} - void SMsgWriter::writeCopyRect(const Rect& r, int srcX, int srcY) { startRect(r,encodingCopyRect); diff --git a/common/rfb/SMsgWriter.h b/common/rfb/SMsgWriter.h index ee59eb36..ccc8f803 100644 --- a/common/rfb/SMsgWriter.h +++ b/common/rfb/SMsgWriter.h @@ -32,10 +32,6 @@ namespace rdr { class OutStream; } namespace rfb { class ConnParams; - class TransImageGetter; - class Region; - class UpdateInfo; - class Encoder; class ScreenSet; class WriteSetCursorCallback { @@ -72,14 +68,6 @@ namespace rfb { // updates mode. void writeEndOfContinuousUpdates(); - // setupCurrentEncoder() should be called before each framebuffer update, - // prior to calling getNumRects() or writeFramebufferUpdateStart(). - void setupCurrentEncoder(); - - // getNumRects() computes the number of sub-rectangles that will compose a - // given rectangle, for current encoder. - int getNumRects(const Rect &r); - // writeSetDesktopSize() won't actually write immediately, but will // write the relevant pseudo-rectangle as part of the next update. bool writeSetDesktopSize(); @@ -118,26 +106,17 @@ namespace rfb { // pseudo-rectangles. void writeNoDataUpdate(); - // writeRects() accepts an UpdateInfo (changed & copied regions) and an - // ImageGetter to fetch pixels from. It then calls writeCopyRect() and - // writeRect() as appropriate. writeFramebufferUpdateStart() must be used - // before the first writeRects() call and writeFrameBufferUpdateEnd() after - // the last one. - void writeRects(const UpdateInfo& update, TransImageGetter* ig); - - // To construct a framebuffer update you can call - // writeFramebufferUpdateStart(), followed by a number of writeCopyRect()s - // and writeRect()s, finishing with writeFramebufferUpdateEnd(). + // writeFramebufferUpdateStart() initiates an update which you can fill + // in using writeCopyRect() and encoders. Finishing the update by calling + // writeFramebufferUpdateEnd(). void writeFramebufferUpdateStart(int nRects); void writeFramebufferUpdateEnd(); - // writeRect() writers the given rectangle using either the preferred - // encoder, or the one explicitly given. - void writeRect(const Rect& r, TransImageGetter* ig); - void writeRect(const Rect& r, int encoding, TransImageGetter* ig); - + // There is no explicit encoder for CopyRect rects. void writeCopyRect(const Rect& r, int srcX, int srcY); + // Encoders should call these to mark the start and stop of individual + // rects. void startRect(const Rect& r, int enc); void endRect(); @@ -169,7 +148,6 @@ namespace rfb { ConnParams* cp; rdr::OutStream* os; - Encoder* encoders[encodingMax+1]; int currentEncoding; int nRectsInUpdate; diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx index 452f7246..0000c1ea 100644 --- a/common/rfb/VNCSConnectionST.cxx +++ b/common/rfb/VNCSConnectionST.cxx @@ -37,6 +37,7 @@ #include #include #include +#include #define XK_MISCELLANY #define XK_XKB_KEYS #include @@ -81,6 +82,8 @@ VNCSConnectionST::VNCSConnectionST(VNCServerST* server_, network::Socket *s, peerEndpoint.buf = sock->getPeerEndpoint(); VNCServerST::connectionsLog.write(1,"accepted: %s", peerEndpoint.buf); + memset(encoders, 0, sizeof(encoders)); + // Configure the socket setSocketTimeouts(); lastEventTime = time(0); @@ -106,6 +109,9 @@ VNCSConnectionST::~VNCSConnectionST() // Remove this client from the server server->clients.remove(this); + for (int i = 0; i <= encodingMax; i++) + delete encoders[i]; + delete [] fenceData; } @@ -1060,18 +1066,30 @@ void VNCSConnectionST::writeFramebufferUpdate() } if (!ui.is_empty() || writer()->needFakeUpdate() || drawRenderedCursor) { + std::vector rects; + std::vector::const_iterator i; + int encoding; + + // Make sure the encoder has the latest settings + encoding = cp.currentEncoding(); + + if (!encoders[encoding]) + encoders[encoding] = Encoder::createEncoder(encoding, writer()); + + encoders[encoding]->setCompressLevel(cp.compressLevel); + encoders[encoding]->setQualityLevel(cp.qualityLevel); + encoders[encoding]->setFineQualityLevel(cp.fineQualityLevel, + cp.subsampling); + // Compute the number of rectangles. Tight encoder makes the things more // complicated as compared to the original VNC4. - writer()->setupCurrentEncoder(); int nRects = (ui.copied.numRects() + (drawRenderedCursor ? 1 : 0)); - std::vector rects; - std::vector::const_iterator i; ui.changed.get_rects(&rects); for (i = rects.begin(); i != rects.end(); i++) { if (i->width() && i->height()) { - int nUpdateRects = writer()->getNumRects(*i); + int nUpdateRects = encoders[encoding]->getNumRects(*i); if (nUpdateRects == 0 && cp.currentEncoding() == encodingTight) { // With Tight encoding and LastRect support, the client does not // care about the number of rectangles in the update - it will @@ -1088,17 +1106,33 @@ void VNCSConnectionST::writeFramebufferUpdate() writer()->writeFramebufferUpdateStart(nRects); - writer()->writeRects(ui, &image_getter); - updates.clear(); + ui.copied.get_rects(&rects); + for (i = rects.begin(); i != rects.end(); i++) + writer()->writeCopyRect(*i, i->tl.x - ui.copy_delta.x, + i->tl.y - ui.copy_delta.y); + + ui.changed.get_rects(&rects); + for (i = rects.begin(); i != rects.end(); i++) + encoders[encoding]->writeRect(*i, &image_getter); - if (drawRenderedCursor) - writeRenderedCursorRect(); + if (drawRenderedCursor) { + image_getter.setPixelBuffer(&server->renderedCursor); + image_getter.setOffset(server->renderedCursorTL); + + encoders[encoding]->writeRect(renderedCursorRect, &image_getter); + + image_getter.setPixelBuffer(server->pb); + image_getter.setOffset(Point(0,0)); + + drawRenderedCursor = false; + } writer()->writeFramebufferUpdateEnd(); writeRTTPing(); requested.clear(); + updates.clear(); } out: @@ -1106,22 +1140,6 @@ out: } -// writeRenderedCursorRect() writes a single rectangle drawing the rendered -// cursor on the client. - -void VNCSConnectionST::writeRenderedCursorRect() -{ - image_getter.setPixelBuffer(&server->renderedCursor); - image_getter.setOffset(server->renderedCursorTL); - - writer()->writeRect(renderedCursorRect, &image_getter); - - image_getter.setPixelBuffer(server->pb); - image_getter.setOffset(Point(0,0)); - - drawRenderedCursor = false; -} - void VNCSConnectionST::screenLayoutChange(rdr::U16 reason) { if (!authenticated()) diff --git a/common/rfb/VNCSConnectionST.h b/common/rfb/VNCSConnectionST.h index 3c7672f4..ca5c4f01 100644 --- a/common/rfb/VNCSConnectionST.h +++ b/common/rfb/VNCSConnectionST.h @@ -37,6 +37,8 @@ struct RTTInfo; namespace rfb { + class Encoder; + class VNCSConnectionST : public SConnection, public WriteSetCursorCallback, public Timer::Callback { @@ -169,7 +171,6 @@ namespace rfb { void writeFramebufferUpdate(); - void writeRenderedCursorRect(); void screenLayoutChange(rdr::U16 reason); void setCursor(); void setDesktopName(const char *name); @@ -202,6 +203,7 @@ namespace rfb { Rect renderedCursorRect; bool continuousUpdates; Region cuRegion; + Encoder* encoders[encodingMax+1]; Timer updateTimer; diff --git a/vncviewer/CConn.cxx b/vncviewer/CConn.cxx index d7f57b5f..f5420629 100644 --- a/vncviewer/CConn.cxx +++ b/vncviewer/CConn.cxx @@ -1,6 +1,6 @@ /* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. - * Copyright 2009-2011 Pierre Ossman for Cendio AB * Copyright (C) 2011 D. R. Commander. All Rights Reserved. + * Copyright 2009-2014 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 @@ -29,6 +29,7 @@ #include #include +#include #include #include #include @@ -80,6 +81,8 @@ CConn::CConn(const char* vncServerName, network::Socket* socket=NULL) setShared(::shared); sock = socket; + memset(decoders, 0, sizeof(decoders)); + int encNum = encodingNum(preferredEncoding); if (encNum != -1) currentEncoding = encNum; @@ -131,6 +134,9 @@ CConn::~CConn() { OptionsDialog::removeCallback(handleOptions); + for (int i = 0; i < sizeof(decoders)/sizeof(decoders[0]); i++) + delete decoders[i]; + if (desktop) delete desktop; @@ -380,19 +386,27 @@ void CConn::serverCutText(const char* str, rdr::U32 len) delete [] buffer; } -// We start timing on beginRect and stop timing on endRect, to -// avoid skewing the bandwidth estimation as a result of the server -// being slow or the network having high latency -void CConn::beginRect(const Rect& r, int encoding) +void CConn::dataRect(const Rect& r, int encoding) { sock->inStream().startTiming(); - if (encoding != encodingCopyRect) { + + if (encoding != encodingCopyRect) lastServerEncoding = encoding; + + if (!Decoder::supported(encoding)) { + fprintf(stderr, "Unknown rect encoding %d\n", encoding); + throw Exception("Unknown rect encoding"); } -} -void CConn::endRect(const Rect& r, int encoding) -{ + if (!decoders[encoding]) { + decoders[encoding] = Decoder::createDecoder(encoding, reader()); + if (!decoders[encoding]) { + fprintf(stderr, "Unknown rect encoding %d\n", encoding); + throw Exception("Unknown rect encoding"); + } + } + decoders[encoding]->readRect(r, this); + sock->inStream().stopTiming(); } diff --git a/vncviewer/CConn.h b/vncviewer/CConn.h index 286a09ed..f7f560bc 100644 --- a/vncviewer/CConn.h +++ b/vncviewer/CConn.h @@ -1,5 +1,5 @@ /* Copyright (C) 2002-2005 RealVNC Ltd. All Rights Reserved. - * Copyright 2009-2011 Pierre Ossman for Cendio AB + * Copyright 2009-2014 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,8 +23,11 @@ #include #include +#include #include +namespace rfb { class Decoder; } + class DesktopWindow; class CConn : public rfb::CConnection, @@ -62,8 +65,7 @@ public: void framebufferUpdateStart(); void framebufferUpdateEnd(); - void beginRect(const rfb::Rect& r, int encoding); - void endRect(const rfb::Rect& r, int encoding); + void dataRect(const rfb::Rect& r, int encoding); void fillRect(const rfb::Rect& r, rfb::Pixel p); void imageRect(const rfb::Rect& r, void* p); @@ -102,6 +104,8 @@ private: bool pendingPFChange; rfb::PixelFormat pendingPF; + rfb::Decoder *decoders[rfb::encodingMax+1]; + int currentEncoding, lastServerEncoding; bool formatChange; -- 2.39.5