base: Redesign internal frame buffer handling

Currently, frame buffer handling in gem5 is quite ad hoc. In practice,
we pass around naked pointers to raw pixel data and expect consumers
to convert frame buffers using the (broken) VideoConverter.

This changeset completely redesigns the way we handle frame buffers
internally. In summary, it fixes several color conversion bugs, adds
support for more color formats (e.g., big endian), and makes the code
base easier to follow.

In the new world, gem5 always represents pixel data using the Pixel
struct when pixels need to be passed between different classes (e.g.,
a display controller and the VNC server). Producers of entire frames
(e.g., display controllers) should use the FrameBuffer class to
represent a frame.

Frame producers are expected to create one instance of the FrameBuffer
class in their constructors and register it with its consumers
once. Consumers are expected to check the dimensions of the frame
buffer when they consume it.

Conversion between the external representation and the internal
representation is supported for all common "true color" RGB formats of
up to 32-bit color depth. The external pixel representation is
expected to be between 1 and 4 bytes in either big endian or little
endian. Color channels are assumed to be contiguous ranges of bits
within each pixel word. The external pixel value is scaled to an 8-bit
internal representation using a floating multiplication to map it to
the entire 8-bit range.

7 files changed, 97 insertions, 449 deletions

diff --git a/src/base/vnc/SConscript b/src/base/vnc/SConscript
index 416743200..271f894ce 100644
--- a/src/base/vnc/SConscript
+++ b/src/base/vnc/SConscript
@@ -39,8 +39,6 @@
 
 Import('*')
 
-Source('convert.cc')
-
 SimObject('Vnc.py')
 Source('vncinput.cc')
 Source('vncserver.cc')
diff --git a/src/base/vnc/convert.cc b/src/base/vnc/convert.cc
deleted file mode 100644
index 2a52ccaec..000000000
--- a/src/base/vnc/convert.cc
+++ /dev/null
@@ -1,183 +0,0 @@
-/*
- * Copyright (c) 2011 ARM Limited
- * All rights reserved
- *
- * The license below extends only to copyright in the software and shall
- * not be construed as granting a license to any other intellectual
- * property including but not limited to intellectual property relating
- * to a hardware implementation of the functionality of the software
- * licensed hereunder.  You may use the software subject to the license
- * terms below provided that you ensure that this notice is replicated
- * unmodified and in its entirety in all distributions of the software,
- * modified or unmodified, in source code or in binary form.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met: redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer;
- * redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution;
- * neither the name of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
- * Authors: Ali Saidi
- *          William Wang
- */
-
-#include <cassert>
-
-#include "base/vnc/convert.hh"
-#include "base/misc.hh"
-
-/** @file
- * This file provides conversion functions for a variety of video modes
- */
-
-VideoConvert::VideoConvert(Mode input_mode, Mode output_mode, int _width,
-        int _height)
-    : inputMode(input_mode), outputMode(output_mode), width(_width),
-    height(_height)
-{
-    if (inputMode != bgr565 && inputMode != rgb565 &&
-        inputMode != bgr8888 && inputMode != bgr888)
-        fatal("Only support converting from bgr565, rdb565, "
-              "bgr8888 and bgr888\n");
-
-    if (outputMode != rgb8888)
-        fatal("Only support converting to rgb8888\n");
-
-    assert(0 < height && height < 4000);
-    assert(0 < width && width < 4000);
-}
-
-VideoConvert::~VideoConvert()
-{
-}
-
-uint8_t*
-VideoConvert::convert(const uint8_t *fb) const
-{
-    switch (inputMode) {
-      case bgr565:
-        return m565rgb8888(fb, true);
-      case rgb565:
-        return m565rgb8888(fb, false);
-      case bgr8888:
-        return bgr8888rgb8888(fb);
-      case bgr888:
-        return bgr888rgb8888(fb);
-      default:
-        panic("Unimplemented Mode\n");
-    }
-}
-
-uint8_t*
-VideoConvert::m565rgb8888(const uint8_t *fb, bool bgr) const
-{
-    uint8_t *out = new uint8_t[area() * sizeof(uint32_t)];
-    uint32_t *out32 = (uint32_t*)out;
-
-    uint16_t *in16 = (uint16_t*)fb;
-
-    for (int x = 0; x < area(); x++) {
-        Bgr565 inpx;
-        Rgb8888 outpx = 0;
-
-        inpx = in16[x];
-
-        if (bgr) {
-            outpx.red = inpx.blue << 3;
-            outpx.green = inpx.green << 2;
-            outpx.blue = inpx.red << 3;
-        } else {
-            outpx.blue = inpx.blue << 3;
-            outpx.green = inpx.green << 2;
-            outpx.red = inpx.red << 3;
-        }
-
-        out32[x] = outpx;
-    }
-
-    return out;
-}
-
-
-uint8_t*
-VideoConvert::bgr8888rgb8888(const uint8_t *fb) const
-{
-    uint8_t *out = new uint8_t[area() * sizeof(uint32_t)];
-    uint32_t *out32 = (uint32_t*)out;
-
-    uint32_t *in32 = (uint32_t*)fb;
-
-    for (int x = 0; x < area(); x++) {
-        Rgb8888 outpx = 0;
-        Bgr8888 inpx;
-
-
-        inpx = in32[x];
-
-        outpx.red = inpx.blue;
-        outpx.green = inpx.green;
-        outpx.blue = inpx.red;
-
-        out32[x] = outpx;
-    }
-
-    return out;
-}
-
-uint8_t*
-VideoConvert::bgr888rgb8888(const uint8_t *fb) const
-{
-    uint8_t *out = new uint8_t[area() * sizeof(uint32_t)];
-    uint32_t *out32 = (uint32_t*)out;
-
-    typedef uint8_t In24[3];
-    const In24 *in24 = (In24 *)fb;
-    for (int x = 0; x < area(); x++) {
-        Rgb8888 outpx = 0;
-
-        outpx.blue = in24[x][0];
-        outpx.green = in24[x][1];
-        outpx.red = in24[x][2];
-        outpx.alpha = 0xFF;
-
-        out32[x] = outpx;
-    }
-
-    return out;
-}
-
-/*
-uint64_t
-VideoConvert::getHash(const uint8_t *fb) const
-{
-    const uint8_t *fb_e = fb + area();
-
-    uint64_t hash = 1;
-    while (fb < fb_e - 8) {
-        hash += *((const uint64_t*)fb);
-        fb += 8;
-    }
-
-    while (fb < fb_e) {
-        hash += *(fb++);
-    }
-
-    return hash;
-}*/
diff --git a/src/base/vnc/convert.hh b/src/base/vnc/convert.hh
deleted file mode 100644
index 592076cbf..000000000
--- a/src/base/vnc/convert.hh
+++ /dev/null
@@ -1,158 +0,0 @@
-/*
- * Copyright (c) 2011 ARM Limited
- * All rights reserved
- *
- * The license below extends only to copyright in the software and shall
- * not be construed as granting a license to any other intellectual
- * property including but not limited to intellectual property relating
- * to a hardware implementation of the functionality of the software
- * licensed hereunder.  You may use the software subject to the license
- * terms below provided that you ensure that this notice is replicated
- * unmodified and in its entirety in all distributions of the software,
- * modified or unmodified, in source code or in binary form.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met: redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer;
- * redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution;
- * neither the name of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
- * Authors: Ali Saidi
- */
-
-/** @file
- * This file provides conversion functions for a variety of video modes
- */
-
-#ifndef __BASE_VNC_CONVERT_HH__
-#define __BASE_VNC_CONVERT_HH__
-
-#include <zlib.h>
-#include "base/bitunion.hh"
-
-class VideoConvert
-{
-  public:
-    enum Mode {
-        UnknownMode,
-        bgr565,
-        rgb565,
-        bgr8888,
-        rgb8888,
-        rgb888,
-        bgr888,
-        bgr444,
-        bgr4444,
-        rgb444,
-        rgb4444
-    };
-
-    // supports bpp32 RGB (bmp) and bpp16 5:6:5 mode BGR (linux)
-    BitUnion32(Rgb8888)
-        Bitfield<7,0> blue;
-        Bitfield<15,8> green;
-        Bitfield<23,16> red;
-        Bitfield<31,24> alpha;
-    EndBitUnion(Rgb8888)
-
-    BitUnion32(Bgr8888)
-        Bitfield<7,0> red;
-        Bitfield<15,8> green;
-        Bitfield<23,16> blue;
-        Bitfield<31,24> alpha;
-    EndBitUnion(Bgr8888)
-
-    BitUnion16(Bgr565)
-        Bitfield<4,0> red;
-        Bitfield<10,5> green;
-        Bitfield<15,11> blue;
-    EndBitUnion(Bgr565)
-
-    BitUnion16(Rgb565)
-        Bitfield<4,0> red;
-        Bitfield<10,5> green;
-        Bitfield<15,11> blue;
-    EndBitUnion(Rgb565)
-
-    /** Setup the converter with the given parameters
-     * @param input_mode type of data that will be provided
-     * @param output_mode type of data that should be output
-     * @param _width width of the frame buffer
-     * @param _height height of the frame buffer
-     */
-    VideoConvert(Mode input_mode, Mode output_mode, int _width, int _height);
-
-    /** Destructor
-     */
-    ~VideoConvert();
-
-    /** Convert the provided frame buffer data into the format specified in the
-     * constructor.
-     * @param fb the frame buffer to convert
-     * @return the converted data (user must free)
-     */
-    uint8_t* convert(const uint8_t *fb) const;
-
-    /** Return the number of pixels that this buffer specifies
-     * @return number of pixels
-     */
-    int area() const { return width * height; }
-
-    /**
-     * Returns a hash on the raw data.
-     *
-     * @return hash of the buffer
-     */
-    inline uint64_t getHash(const uint8_t *fb) const {
-        return adler32(0UL, fb, width * height);
-    }
-
-  private:
-
-    /**
-     * Convert a bgr8888 input to rgb8888.
-     * @param fb the data to convert
-     * @return converted data
-     */
-    uint8_t* bgr8888rgb8888(const uint8_t *fb) const;
-
-    /**
-     * Convert a bgr888 input to rgb8888.
-     * @param fb the data to convert
-     * @return converted data
-     */
-    uint8_t* bgr888rgb8888(const uint8_t *fb) const;
-
-    /**
-     * Convert a bgr565 or rgb565 input to rgb8888.
-     * @param fb the data to convert
-     * @param bgr true if the input data is bgr565
-     * @return converted data
-     */
-    uint8_t* m565rgb8888(const uint8_t *fb, bool bgr) const;
-
-    Mode inputMode;
-    Mode outputMode;
-    int width;
-    int height;
-};
-
-#endif // __BASE_VNC_CONVERT_HH__
-
diff --git a/src/base/vnc/vncinput.cc b/src/base/vnc/vncinput.cc
index 071804583..017fc3876 100644
--- a/src/base/vnc/vncinput.cc
+++ b/src/base/vnc/vncinput.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010 ARM Limited
+ * Copyright (c) 2010, 2015 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -53,9 +53,10 @@ using namespace std;
 
 VncInput::VncInput(const Params *p)
     : SimObject(p), keyboard(NULL), mouse(NULL),
-      vc(NULL), fbPtr(NULL), videoMode(VideoConvert::UnknownMode),
-      _videoWidth(1), _videoHeight(1), captureEnabled(p->frame_capture),
-      captureCurrentFrame(0), captureLastHash(0), captureBitmap(0)
+      fb(&FrameBuffer::dummy),
+      _videoWidth(fb->width()), _videoHeight(fb->height()),
+      captureEnabled(p->frame_capture),
+      captureCurrentFrame(0), captureLastHash(0)
 {
     if (captureEnabled) {
         // remove existing frame output directory if it exists, then create a
@@ -68,33 +69,40 @@ VncInput::VncInput(const Params *p)
 }
 
 void
-VncInput::setFrameBufferParams(VideoConvert::Mode mode, uint16_t width,
-    uint16_t height)
+VncInput::setFrameBuffer(const FrameBuffer *rfb)
 {
-    DPRINTF(VNC, "Updating video params: mode: %d width: %d height: %d\n", mode,
-            width, height);
+    if (!rfb)
+        panic("Trying to VNC frame buffer to NULL!");
 
-    if (mode != videoMode || width != videoWidth() || height != videoHeight()) {
-        videoMode = mode;
-        _videoWidth = width;
-        _videoHeight = height;
+    fb = rfb;
+
+    // create bitmap of the frame with new attributes
+    if (captureEnabled)
+        captureBitmap.reset(new Bitmap(rfb));
+
+    // Setting a new frame buffer means that we need to send an update
+    // to the client. Mark the internal buffers as dirty to do so.
+    setDirty();
+}
 
-        if (vc)
-            delete vc;
+void
+VncInput::setDirty()
+{
+    const unsigned width(fb->width());
+    const unsigned height(fb->height());
 
-        vc = new VideoConvert(mode, VideoConvert::rgb8888, videoWidth(),
-                videoHeight());
+    if (_videoWidth != width || _videoHeight != height) {
+        DPRINTF(VNC, "Updating video params: width: %d height: %d\n",
+                width, height);
 
-        if (captureEnabled) {
-            // create bitmap of the frame with new attributes
-            if (captureBitmap)
-                delete captureBitmap;
+        _videoWidth = width;
+        _videoHeight = height;
 
-            assert(fbPtr);
-            captureBitmap = new Bitmap(videoMode, width, height, fbPtr);
-            assert(captureBitmap);
-        }
+        frameBufferResized();
     }
+
+     if (captureEnabled)
+        captureFrameBuffer();
 }
 
 void
@@ -103,7 +111,7 @@ VncInput::captureFrameBuffer()
     assert(captureBitmap);
 
     // skip identical frames
-    uint64_t new_hash = captureBitmap->getHash();
+    uint64_t new_hash = fb->getHash();
     if (captureLastHash == new_hash)
         return;
     captureLastHash = new_hash;
@@ -116,8 +124,8 @@ VncInput::captureFrameBuffer()
 
     // create the compressed framebuffer file
     ostream *fb_out = simout.create(captureOutputDirectory + frameFilename,
-                    true);
-    captureBitmap->write(fb_out);
+                                    true);
+    captureBitmap->write(*fb_out);
     simout.close(fb_out);
 
     ++captureCurrentFrame;
diff --git a/src/base/vnc/vncinput.hh b/src/base/vnc/vncinput.hh
index 1686e3f25..96235fec7 100644
--- a/src/base/vnc/vncinput.hh
+++ b/src/base/vnc/vncinput.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010 ARM Limited
+ * Copyright (c) 2010, 2015 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -46,8 +46,8 @@
 #define __BASE_VNC_VNC_INPUT_HH__
 
 #include <iostream>
+#include <memory>
 
-#include "base/vnc/convert.hh"
 #include "base/bitmap.hh"
 #include "params/VncInput.hh"
 #include "sim/sim_object.hh"
@@ -160,11 +160,7 @@ class VncInput : public SimObject
      * tell us where the data is instead of constanly copying it around
      * @param rfb frame buffer that we're going to use
      */
-    void
-    setFramebufferAddr(uint8_t* rfb)
-    {
-        fbPtr = rfb;
-    }
+    virtual void setFrameBuffer(const FrameBuffer *rfb);
 
     /** Set up the device that would like to receive notifications when keys are
      * pressed in the vnc client keyboard
@@ -196,32 +192,19 @@ class VncInput : public SimObject
      * the frame buffer has been updated and a new image needs to be sent to the
      * client
      */
-    virtual void setDirty()
-    {
-        if (captureEnabled)
-            captureFrameBuffer();
-    }
-
-    /** Set the mode of the data the frame buffer will be sending us
-     * @param mode the mode
-     */
-    virtual void setFrameBufferParams(VideoConvert::Mode mode, uint16_t width, uint16_t height);
+    virtual void setDirty();
 
   protected:
+    virtual void frameBufferResized() {};
+
     /** The device to notify when we get key events */
     VncKeyboard *keyboard;
 
     /** The device to notify when we get mouse events */
     VncMouse *mouse;
 
-    /** The video converter that transforms data for us */
-    VideoConvert *vc;
-
     /** pointer to the actual data that is stored in the frame buffer device */
-    uint8_t* fbPtr;
-
-    /** The mode of data we're getting frame buffer in */
-    VideoConvert::Mode videoMode;
+    const FrameBuffer *fb;
 
     /** the width of the frame buffer we are sending to the client */
     uint16_t _videoWidth;
@@ -242,7 +225,7 @@ class VncInput : public SimObject
     uint64_t captureLastHash;
 
     /** Cached bitmap object for writing out frame buffers to file */
-    Bitmap *captureBitmap;
+    std::unique_ptr<Bitmap> captureBitmap;
 
     /** Captures the current frame buffer to a file */
     void captureFrameBuffer();
diff --git a/src/base/vnc/vncserver.cc b/src/base/vnc/vncserver.cc
index 6dc2f2f10..20f3bc8fe 100644
--- a/src/base/vnc/vncserver.cc
+++ b/src/base/vnc/vncserver.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010 ARM Limited
+ * Copyright (c) 2010, 2015 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -61,6 +61,7 @@
 
 #include <cerrno>
 #include <cstdio>
+#include <cstddef>
 
 #include "base/atomicio.hh"
 #include "base/bitmap.hh"
@@ -74,6 +75,12 @@
 
 using namespace std;
 
+const PixelConverter VncServer::pixelConverter(
+    4,        // 4 bytes / pixel
+    16, 8, 0, // R in [23, 16], G in [15, 8], B in [7, 0]
+    8, 8, 8,  // 8 bits / channel
+    LittleEndianByteOrder);
+
 /** @file
  * Implementiation of a VNC server
  */
@@ -122,20 +129,19 @@ VncServer::VncServer(const Params *p)
 
     curState = WaitForProtocolVersion;
 
-    // currently we only support this one pixel format
-    // unpacked 32bit rgb (rgb888 + 8 bits of nothing/alpha)
-    // keep it around for telling the client and making
-    // sure the client cooperates
-    pixelFormat.bpp = 32;
-    pixelFormat.depth = 24;
-    pixelFormat.bigendian = 0;
+    // We currently only support one pixel format. Extract the pixel
+    // representation from our PixelConverter instance and keep it
+    // around for telling the client and making sure it cooperates
+    pixelFormat.bpp = 8 * pixelConverter.length;
+    pixelFormat.depth = pixelConverter.depth;
+    pixelFormat.bigendian = pixelConverter.byte_order == BigEndianByteOrder;
     pixelFormat.truecolor = 1;
-    pixelFormat.redmax = 0xff;
-    pixelFormat.greenmax = 0xff;
-    pixelFormat.bluemax = 0xff;
-    pixelFormat.redshift = 16;
-    pixelFormat.greenshift = 8;
-    pixelFormat.blueshift = 0;
+    pixelFormat.redmax = pixelConverter.ch_r.mask;
+    pixelFormat.greenmax = pixelConverter.ch_g.mask;
+    pixelFormat.bluemax = pixelConverter.ch_b.mask;
+    pixelFormat.redshift = pixelConverter.ch_r.offset;
+    pixelFormat.greenshift = pixelConverter.ch_g.offset;
+    pixelFormat.blueshift = pixelConverter.ch_b.offset;
 
     DPRINTF(VNC, "Vnc server created at port %d\n", p->port);
 }
@@ -615,13 +621,11 @@ void
 VncServer::sendFrameBufferUpdate()
 {
 
-    if (!fbPtr || dataFd <= 0 || curState != NormalPhase || !sendUpdate) {
+    if (dataFd <= 0 || curState != NormalPhase || !sendUpdate) {
         DPRINTF(VNC, "NOT sending framebuffer update\n");
         return;
     }
 
-    assert(vc);
-
     // The client will request data constantly, unless we throttle it
     sendUpdate = false;
 
@@ -650,19 +654,25 @@ VncServer::sendFrameBufferUpdate()
     write(&fbu);
     write(&fbr);
 
-    assert(fbPtr);
+    assert(fb);
 
-    uint8_t *tmp = vc->convert(fbPtr);
-    uint64_t num_pixels = videoWidth() * videoHeight();
-    write(tmp, num_pixels * sizeof(uint32_t));
-    delete [] tmp;
+    std::vector<uint8_t> line_buffer(pixelConverter.length * fb->width());
+    for (int y = 0; y < fb->height(); ++y) {
+        // Convert and send a line at a time
+        uint8_t *raw_pixel(line_buffer.data());
+        for (unsigned x = 0; x < fb->width(); ++x) {
+            pixelConverter.fromPixel(raw_pixel, fb->pixel(x, y));
+            raw_pixel += pixelConverter.length;
+        }
 
+        write(line_buffer.data(), line_buffer.size());
+    }
 }
 
 void
 VncServer::sendFrameBufferResized()
 {
-    assert(fbPtr && dataFd > 0 && curState == NormalPhase);
+    assert(fb && dataFd > 0 && curState == NormalPhase);
     DPRINTF(VNC, "Sending framebuffer resize\n");
 
     FrameBufferUpdate fbu;
@@ -692,19 +702,23 @@ VncServer::sendFrameBufferResized()
 }
 
 void
-VncServer::setFrameBufferParams(VideoConvert::Mode mode, uint16_t width,
-    uint16_t height)
+VncServer::setDirty()
 {
-    VncInput::setFrameBufferParams(mode, width, height);
-
-    if (mode != videoMode || width != videoWidth() || height != videoHeight()) {
-        if (dataFd > 0 && fbPtr && curState == NormalPhase) {
-            if (supportsResizeEnc)
-                sendFrameBufferResized();
-            else
-                // The frame buffer changed size and we can't update the client
-                detach();
-        }
+    VncInput::setDirty();
+
+    sendUpdate = true;
+    sendFrameBufferUpdate();
+}
+
+void
+VncServer::frameBufferResized()
+{
+    if (dataFd > 0 && curState == NormalPhase) {
+        if (supportsResizeEnc)
+            sendFrameBufferResized();
+        else
+            // The frame buffer changed size and we can't update the client
+            detach();
     }
 }
 
diff --git a/src/base/vnc/vncserver.hh b/src/base/vnc/vncserver.hh
index cd1f186f5..0222a7726 100644
--- a/src/base/vnc/vncserver.hh
+++ b/src/base/vnc/vncserver.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010 ARM Limited
+ * Copyright (c) 2010, 2015 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -47,7 +47,6 @@
 
 #include <iostream>
 
-#include "base/vnc/convert.hh"
 #include "base/vnc/vncinput.hh"
 #include "base/bitmap.hh"
 #include "base/circlebuf.hh"
@@ -305,24 +304,11 @@ class VncServer : public VncInput
      */
     void sendFrameBufferResized();
 
-  public:
-    /** The frame buffer uses this call to notify the vnc server that
-     * the frame buffer has been updated and a new image needs to be sent to the
-     * client
-     */
-    void
-    setDirty()
-    {
-        VncInput::setDirty();
-        sendUpdate = true;
-        sendFrameBufferUpdate();
-    }
+    static const PixelConverter pixelConverter;
 
-    /** Set the mode of the data the frame buffer will be sending us
-     * @param mode the mode
-     */
-    void setFrameBufferParams(VideoConvert::Mode mode, uint16_t width,
-        uint16_t height);
+  public:
+    void setDirty() M5_ATTR_OVERRIDE;
+    void frameBufferResized() M5_ATTR_OVERRIDE;
 };
 
 #endif