[Qemu-devel] Re: [PATCH v2 05/10] vnc: add basic tight support

[Anthony Liguori <aliguori@linux.vnet.ibm.com>]

On 05/18/2010 07:49 AM, Corentin Chary wrote:
> Add support for tight encoding [1]. This patch only add support
> for "basic" tight compression without any filter.
>
> [1] http://tigervnc.org/cgi-bin/rfbproto#tight-encoding.
>
> Signed-off-by: Corentin Chary<corentincj@iksaif.net>
> ---
>   Makefile               |    2 +
>   Makefile.objs          |    1 +
>   vnc-encoding-hextile.c |    5 +-
>   vnc-encoding-tight.c   |  287 ++++++++++++++++++++++++++++++++++++++++++++++++
>   vnc-encoding-tight.h   |  199 +++++++++++++++++++++++++++++++++
>   vnc-encoding-zlib.c    |   17 ++-
>   vnc.c                  |   37 +++++--
>   vnc.h                  |   17 +++-
>   8 files changed, 543 insertions(+), 22 deletions(-)
>   create mode 100644 vnc-encoding-tight.c
>   create mode 100644 vnc-encoding-tight.h
>
> diff --git a/Makefile b/Makefile
> index eb9e02b..99d9c5d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -124,6 +124,8 @@ vnc-encoding-zlib.o: vnc.h
>
>   vnc-encoding-hextile.o: vnc.h
>
> +vnc-encoding-tight.o: vnc.h vnc-encoding-tight.h
> +
>   curses.o: curses.c keymaps.h curses_keys.h
>
>   bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS)
> diff --git a/Makefile.objs b/Makefile.objs
> index acbaf22..070ee09 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -104,6 +104,7 @@ common-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o
>   common-obj-$(CONFIG_CURSES) += curses.o
>   common-obj-y += vnc.o acl.o d3des.o
>   common-obj-y += vnc-encoding-zlib.o vnc-encoding-hextile.o
> +common-obj-y += vnc-encoding-tight.o
>   common-obj-y += iov.o
>   common-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o
>   common-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o
> diff --git a/vnc-encoding-hextile.c b/vnc-encoding-hextile.c
> index a01c5e2..728f25e 100644
> --- a/vnc-encoding-hextile.c
> +++ b/vnc-encoding-hextile.c
> @@ -62,8 +62,8 @@ static void hextile_enc_cord(uint8_t *ptr, int x, int y, int w, int h)
>   #undef BPP
>   #undef GENERIC
>
> -void vnc_hextile_send_framebuffer_update(VncState *vs, int x,
> -                                         int y, int w, int h)
> +int vnc_hextile_send_framebuffer_update(VncState *vs, int x,
> +                                        int y, int w, int h)
>   {
>       int i, j;
>       int has_fg, has_bg;
> @@ -83,6 +83,7 @@ void vnc_hextile_send_framebuffer_update(VncState *vs, int x,
>       free(last_fg);
>       free(last_bg);
>
> +    return 1;
>   }
>
>   void vnc_hextile_set_pixel_conversion(VncState *vs, int generic)
> diff --git a/vnc-encoding-tight.c b/vnc-encoding-tight.c
> new file mode 100644
> index 0000000..0d328c2
> --- /dev/null
> +++ b/vnc-encoding-tight.c
> @@ -0,0 +1,287 @@
> +/*
> + * QEMU VNC display driver: tight encoding
> + *
> + * Copyright (C) 2006 Anthony Liguori<anthony@codemonkey.ws>
> + * Copyright (C) 2006 Fabrice Bellard
> + * Copyright (C) 2009 Red Hat, Inc
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +
> +#include<stdbool.h>
> +
> +#include "vnc.h"
> +#include "vnc-encoding-tight.h"
> +
> +/* Compression level stuff. The following array contains various
> +   encoder parameters for each of 10 compression levels (0..9).
> +   Last three parameters correspond to JPEG quality levels (0..9). */
> +
> +static const struct {
> +    int max_rect_size, max_rect_width;
> +    int mono_min_rect_size, gradient_min_rect_size;
> +    int idx_zlib_level, mono_zlib_level, raw_zlib_level, gradient_zlib_level;
> +    int gradient_threshold, gradient_threshold24;
> +    int idx_max_colors_divisor;
> +    int jpeg_quality, jpeg_threshold, jpeg_threshold24;
> +} tight_conf[] = {
> +    {   512,   32,   6, 65536, 0, 0, 0, 0,   0,   0,   4,  5, 10000, 23000 },
> +    {  2048,  128,   6, 65536, 1, 1, 1, 0,   0,   0,   8, 10,  8000, 18000 },
> +    {  6144,  256,   8, 65536, 3, 3, 2, 0,   0,   0,  24, 15,  6500, 15000 },
> +    { 10240, 1024,  12, 65536, 5, 5, 3, 0,   0,   0,  32, 25,  5000, 12000 },
> +    { 16384, 2048,  12, 65536, 6, 6, 4, 0,   0,   0,  32, 37,  4000, 10000 },
> +    { 32768, 2048,  12,  4096, 7, 7, 5, 4, 150, 380,  32, 50,  3000,  8000 },
> +    { 65536, 2048,  16,  4096, 7, 7, 6, 4, 170, 420,  48, 60,  2000,  5000 },
> +    { 65536, 2048,  16,  4096, 8, 8, 7, 5, 180, 450,  64, 70,  1000,  2500 },
> +    { 65536, 2048,  32,  8192, 9, 9, 8, 6, 190, 475,  64, 75,   500,  1200 },
> +    { 65536, 2048,  32,  8192, 9, 9, 9, 6, 200, 500,  96, 80,   200,   500 }
> +};
>
>    

I assume this was borrowed from somewhere so we need to preserve the 
original copyright statements.
> +static int tight_init_stream(VncState *vs, int stream_id,
> +                             int level, int strategy)
> +{
> +    z_streamp zstream =&vs->tight_stream[stream_id];
> +
> +    if (zstream->opaque != vs) {
> +        int err;
>    

IIUC, if zstream->opaque != vs, then it == NULL?  I think the 
affirmative check would make more sense.

> +        VNC_DEBUG("VNC: TIGHT: initializing zlib stream %d\n", stream_id);
> +        VNC_DEBUG("VNC: TIGHT: opaque = %p | vs = %p\n", zstream->opaque, vs);
> +        zstream->zalloc = vnc_zlib_zalloc;
> +        zstream->zfree = vnc_zlib_zfree;
> +
> +        err = deflateInit2(zstream, level, Z_DEFLATED, MAX_WBITS,
> +                           MAX_MEM_LEVEL, strategy);
> +
> +        if (err != Z_OK) {
> +            fprintf(stderr, "VNC: error initializing zlib\n");
> +            return -1;
> +        }
> +
> +        vs->tight_levels[stream_id] = level;
> +        zstream->opaque = vs;
> +    }
> +
> +    if (vs->tight_levels[stream_id] != level) {
> +        if (deflateParams (zstream, level, strategy) != Z_OK) {
>    

There's an extra space in the function call.

> +            return -1;
> +        }
> +        vs->tight_levels[stream_id] = level;
> +    }
> +    return 0;
> +}
> +
> +static void tight_send_compact_size(VncState *vs, size_t len)
> +{
> +    int lpc = 0;
> +    int bytes = 0;
> +    char buf[3] = {0, 0, 0};
> +
> +    buf[bytes++] = len&  0x7F;
> +    if (len>  0x7F) {
> +        buf[bytes-1] |= 0x80;
> +        buf[bytes++] = len>>  7&  0x7F;
>    

I'd suggest using parenthesis here for readability.

> +        if (len>  0x3FFF) {
> +            buf[bytes-1] |= 0x80;
> +            buf[bytes++] = len>>  14&  0xFF;
>    

Here too.

> +        }
> +    }
> +    for(lpc = 0; lpc<  bytes; lpc++) {
>    

Missing space.

> +        vnc_write_u8(vs, buf[lpc]);
> +    }
> +}
> +
> +static int tight_compress_data(VncState *vs, int stream_id, size_t bytes,
> +                               int level, int strategy)
> +{
> +    z_streamp zstream =&vs->tight_stream[stream_id];
> +    int previous_out;
> +
> +    if (bytes<  VNC_TIGHT_MIN_TO_COMPRESS) {
> +        vnc_write(vs, vs->tight.buffer, vs->tight.offset);
> +        return bytes;
> +    }
> +
> +    if (tight_init_stream(vs, stream_id, level, strategy))
> +        return -1;
>    

Missing {}s.

> +    // reserve memory in output buffer
> +    buffer_reserve(&vs->tight_zlib, bytes + 64);
>    

Please avoid C99 comments.

> +    // set pointers
> +    zstream->next_in = vs->tight.buffer;
> +    zstream->avail_in = vs->tight.offset;
> +    zstream->next_out = vs->tight_zlib.buffer + vs->tight_zlib.offset;
> +    zstream->avail_out = vs->tight_zlib.capacity - vs->tight_zlib.offset;
> +    zstream->data_type = Z_BINARY;
> +    previous_out = zstream->total_out;
> +
> +    // start encoding
> +    if (deflate(zstream, Z_SYNC_FLUSH) != Z_OK) {
> +        fprintf(stderr, "VNC: error during tight compression\n");
> +        return -1;
> +    }
> +
> +    vs->tight_zlib.offset = vs->tight_zlib.capacity - zstream->avail_out;
> +    bytes = zstream->total_out - previous_out;
> +
> +    tight_send_compact_size(vs, bytes);
> +    vnc_write(vs, vs->tight_zlib.buffer, bytes);
> +
> +    buffer_reset(&vs->tight_zlib);
> +
> +    return bytes;
> +}
> +
> +/*
> + * Subencoding implementations.
> + */
> +static void tight_pack24(VncState *vs, size_t count)
> +{
> +    unsigned char *buf;
> +    uint32_t *buf32;
> +    uint32_t pix;
> +    int rshift, gshift, bshift;
> +
> +    buf = vs->tight.buffer;
> +    buf32 = (uint32_t *)buf;
> +
> +    if ((vs->clientds.flags&  QEMU_BIG_ENDIAN_FLAG) ==
> +        (vs->ds->surface->flags&  QEMU_BIG_ENDIAN_FLAG)) {
> +        rshift = vs->clientds.pf.rshift;
> +        gshift = vs->clientds.pf.gshift;
> +        bshift = vs->clientds.pf.bshift;
> +    } else {
> +        rshift = 24 - vs->clientds.pf.rshift;
> +        gshift = 24 - vs->clientds.pf.gshift;
> +        bshift = 24 - vs->clientds.pf.bshift;
>    

Shouldn't this be 32 instead of 24?

> +    }
> +
> +    vs->tight.offset = count * 3;
> +
> +    while (count--) {
> +        pix = *buf32++;
> +        *buf++ = (char)(pix>>  rshift);
> +        *buf++ = (char)(pix>>  gshift);
> +        *buf++ = (char)(pix>>  bshift);
> +    }
> +}
> +
> +static int send_full_color_rect(VncState *vs, int w, int h)
> +{
> +    int stream = 0;
> +    size_t bytes;
> +
> +    vnc_write_u8(vs, stream<<  4); /* no flushing, no filter */
> +
> +    if (vs->tight_pixel24) {
> +        tight_pack24(vs, w * h);
> +        bytes = 3;
> +    } else {
> +        bytes = vs->clientds.pf.bytes_per_pixel;
> +    }
> +
> +    bytes = tight_compress_data(vs, stream, w * h * bytes,
> +                                tight_conf[vs->tight_compression].raw_zlib_level,
> +                                Z_DEFAULT_STRATEGY);
> +
> +    return (bytes>= 0);
> +}
> +
> +static void vnc_tight_start(VncState *vs)
> +{
> +    buffer_reset(&vs->tight);
> +
> +    // make the output buffer be the zlib buffer, so we can compress it later
> +    vs->tight_tmp = vs->output;
> +    vs->output = vs->tight;
> +}
> +
> +static void vnc_tight_stop(VncState *vs)
> +{
> +    // switch back to normal output/zlib buffers
> +    vs->tight = vs->output;
> +    vs->output = vs->tight_tmp;
> +}
> +
> +static int send_sub_rect(VncState *vs, int x, int y, int w, int h)
> +{
> +    vnc_framebuffer_update(vs, x, y, w, h, VNC_ENCODING_TIGHT);
> +
> +    /*
> +     * Convert pixels and store them in vs->tight
> +     * We will probably rework that later, probably
> +     * when adding other sub-encodings
> +     */
> +    vnc_tight_start(vs);
> +    vnc_raw_send_framebuffer_update(vs, x, y, w, h);
> +    vnc_tight_stop(vs);
> +
> +    return send_full_color_rect(vs, w, h);
> +}
> +
> +static int send_rect_simple(VncState *vs, int x, int y, int w, int h)
> +{
> +    int max_size, max_width;
> +    int max_sub_width, max_sub_height;
> +    int dx, dy;
> +    int rw, rh;
> +    int n = 0;
> +
> +    max_size = tight_conf[vs->tight_compression].max_rect_size;
> +    max_width = tight_conf[vs->tight_compression].max_rect_width;
> +
> +    if (w>  max_width || w * h>  max_size) {
> +        max_sub_width = (w>  max_width) ? max_width : w;
>    

We have a MAX() macro.

> +        max_sub_height = max_size / max_sub_width;
> +
> +        for (dy = 0; dy<  h; dy += max_sub_height) {
> +            for (dx = 0; dx<  w; dx += max_width) {
> +                rw = (dx + max_sub_width<  w) ? max_sub_width : w - dx;
> +                rh = (dy + max_sub_height<  h) ? max_sub_height : h - dy;
> +                n += send_sub_rect(vs, x+dx, y+dy, rw, rh);
> +            }
> +        }
> +    } else {
> +        n += send_sub_rect(vs, x, y, w, h);
> +    }
> +
> +    return n;
> +}
> +
> +void vnc_tight_init(VncState *vs)
> +{
> +    int i;
> +    for (i=0; i<ARRAY_SIZE(vs->tight_stream); i++)
> +        vs->tight_stream[i].opaque = NULL;
>    

Missing {}s

> +}
> +
> +int vnc_tight_send_framebuffer_update(VncState *vs, int x, int y,
> +                                      int w, int h)
> +{
> +    if (vs->clientds.pf.bytes_per_pixel == 4&&  vs->clientds.pf.rmax == 0xFF&&
> +        vs->clientds.pf.bmax == 0xFF&&  vs->clientds.pf.gmax == 0xFF) {
> +        vs->tight_pixel24 = true;
> +    } else {
> +        vs->tight_pixel24 = false;
> +    }
> +
> +    return send_rect_simple(vs, x, y, w, h);
> +}
> diff --git a/vnc-encoding-tight.h b/vnc-encoding-tight.h
> new file mode 100644
> index 0000000..108e184
> --- /dev/null
> +++ b/vnc-encoding-tight.h
> @@ -0,0 +1,199 @@
> +/*
> + * QEMU VNC display driver: tight encoding
> + *
> + * Copyright (C) 2006 Anthony Liguori<anthony@codemonkey.ws>
> + * Copyright (C) 2006 Fabrice Bellard
> + * Copyright (C) 2009 Red Hat, Inc
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#ifndef __QEMU_VNC_ENCODING_TIGHT_H
> +# define __QEMU_VNC_ENCODING_TIGHT_H
>    

Please avoid the __ prefix and the space in #define.

> +/*- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
> + * Tight Encoding.
> + *
> + *-- The first byte of each Tight-encoded rectangle is a "compression control
> + *   byte". Its format is as follows (bit 0 is the least significant one):
> + *
> + *   bit 0:    if 1, then compression stream 0 should be reset;
> + *   bit 1:    if 1, then compression stream 1 should be reset;
> + *   bit 2:    if 1, then compression stream 2 should be reset;
> + *   bit 3:    if 1, then compression stream 3 should be reset;
> + *   bits 7-4: if 1000 (0x08), then the compression type is "fill",
> + *             if 1001 (0x09), then the compression type is "jpeg",
> + *             if 0xxx, then the compression type is "basic",
> + *             values greater than 1001 are not valid.
> + *
> + * If the compression type is "basic", then bits 6..4 of the
> + * compression control byte (those xxx in 0xxx) specify the following:
> + *
> + *   bits 5-4:  decimal representation is the index of a particular zlib
> + *              stream which should be used for decompressing the data;
> + *   bit 6:     if 1, then a "filter id" byte is following this byte.
> + *
> + *-- The data that follows after the compression control byte described
> + * above depends on the compression type ("fill", "jpeg" or "basic").
> + *
> + *-- If the compression type is "fill", then the only pixel value follows, in
> + * client pixel format (see NOTE 1). This value applies to all pixels of the
> + * rectangle.
> + *
> + *-- If the compression type is "jpeg", the following data stream looks like
> + * this:
> + *
> + *   1..3 bytes:  data size (N) in compact representation;
> + *   N bytes:     JPEG image.
> + *
> + * Data size is compactly represented in one, two or three bytes, according
> + * to the following scheme:
> + *
> + *  0xxxxxxx                    (for values 0..127)
> + *  1xxxxxxx 0yyyyyyy           (for values 128..16383)
> + *  1xxxxxxx 1yyyyyyy zzzzzzzz  (for values 16384..4194303)
> + *
> + * Here each character denotes one bit, xxxxxxx are the least significant 7
> + * bits of the value (bits 0-6), yyyyyyy are bits 7-13, and zzzzzzzz are the
> + * most significant 8 bits (bits 14-21). For example, decimal value 10000
> + * should be represented as two bytes: binary 10010000 01001110, or
> + * hexadecimal 90 4E.
> + *
> + *-- If the compression type is "basic" and bit 6 of the compression control
> + * byte was set to 1, then the next (second) byte specifies "filter id" which
> + * tells the decoder what filter type was used by the encoder to pre-process
> + * pixel data before the compression. The "filter id" byte can be one of the
> + * following:
> + *
> + *   0:  no filter ("copy" filter);
> + *   1:  "palette" filter;
> + *   2:  "gradient" filter.
> + *
> + *-- If bit 6 of the compression control byte is set to 0 (no "filter id"
> + * byte), or if the filter id is 0, then raw pixel values in the client
> + * format (see NOTE 1) will be compressed. See below details on the
> + * compression.
> + *
> + *-- The "gradient" filter pre-processes pixel data with a simple algorithm
> + * which converts each color component to a difference between a "predicted"
> + * intensity and the actual intensity. Such a technique does not affect
> + * uncompressed data size, but helps to compress photo-like images better.
> + * Pseudo-code for converting intensities to differences is the following:
> + *
> + *   P[i,j] := V[i-1,j] + V[i,j-1] - V[i-1,j-1];
> + *   if (P[i,j]<  0) then P[i,j] := 0;
> + *   if (P[i,j]>  MAX) then P[i,j] := MAX;
> + *   D[i,j] := V[i,j] - P[i,j];
> + *
> + * Here V[i,j] is the intensity of a color component for a pixel at
> + * coordinates (i,j). MAX is the maximum value of intensity for a color
> + * component.
> + *
> + *-- The "palette" filter converts true-color pixel data to indexed colors
> + * and a palette which can consist of 2..256 colors. If the number of colors
> + * is 2, then each pixel is encoded in 1 bit, otherwise 8 bits is used to
> + * encode one pixel. 1-bit encoding is performed such way that the most
> + * significant bits correspond to the leftmost pixels, and each raw of pixels
> + * is aligned to the byte boundary. When "palette" filter is used, the
> + * palette is sent before the pixel data. The palette begins with an unsigned
> + * byte which value is the number of colors in the palette minus 1 (i.e. 1
> + * means 2 colors, 255 means 256 colors in the palette). Then follows the
> + * palette itself which consist of pixel values in client pixel format (see
> + * NOTE 1).
> + *
> + *-- The pixel data is compressed using the zlib library. But if the data
> + * size after applying the filter but before the compression is less then 12,
> + * then the data is sent as is, uncompressed. Four separate zlib streams
> + * (0..3) can be used and the decoder should read the actual stream id from
> + * the compression control byte (see NOTE 2).
> + *
> + * If the compression is not used, then the pixel data is sent as is,
> + * otherwise the data stream looks like this:
> + *
> + *   1..3 bytes:  data size (N) in compact representation;
> + *   N bytes:     zlib-compressed data.
> + *
> + * Data size is compactly represented in one, two or three bytes, just like
> + * in the "jpeg" compression method (see above).
> + *
> + *-- NOTE 1. If the color depth is 24, and all three color components are
> + * 8-bit wide, then one pixel in Tight encoding is always represented by
> + * three bytes, where the first byte is red component, the second byte is
> + * green component, and the third byte is blue component of the pixel color
> + * value. This applies to colors in palettes as well.
> + *
> + *-- NOTE 2. The decoder must reset compression streams' states before
> + * decoding the rectangle, if some of bits 0,1,2,3 in the compression control
> + * byte are set to 1. Note that the decoder must reset zlib streams even if
> + * the compression type is "fill" or "jpeg".
> + *
> + *-- NOTE 3. The "gradient" filter and "jpeg" compression may be used only
> + * when bits-per-pixel value is either 16 or 32, not 8.
> + *
> + *-- NOTE 4. The width of any Tight-encoded rectangle cannot exceed 2048
> + * pixels. If a rectangle is wider, it must be split into several rectangles
> + * and each one should be encoded separately.
> + *
> + */
>    

Definitely need to carry the original copyright here.

> +#define VNC_TIGHT_EXPLICIT_FILTER       0x04
> +#define VNC_TIGHT_FILL                  0x08
> +#define VNC_TIGHT_JPEG                  0x09
> +#define VNC_TIGHT_MAX_SUBENCODING       0x09
> +
> +/* Filters to improve compression efficiency */
> +#define VNC_TIGHT_FILTER_COPY             0x00
> +#define VNC_TIGHT_FILTER_PALETTE          0x01
> +#define VNC_TIGHT_FILTER_GRADIENT         0x02
> +
> +/* Note: The following constant should not be changed. */
> +#define VNC_TIGHT_MIN_TO_COMPRESS 12
> +
> +/* The parameters below may be adjusted. */
> +#define VNC_TIGHT_MIN_SPLIT_RECT_SIZE     4096
> +#define VNC_TIGHT_MIN_SOLID_SUBRECT_SIZE  2048
> +#define VNC_TIGHT_MAX_SPLIT_TILE_SIZE       16
> +
> +/*
> + * min()/max()/clamp() macros that also do
> + * strict type-checking.. See the
> + * "unnecessary" pointer comparison.
> + */
> +#define min(x, y) ({				\
> +	typeof(x) _min1 = (x);			\
> +	typeof(y) _min2 = (y);			\
> +	(void) (&_min1 ==&_min2);		\
> +	_min1<  _min2 ? _min1 : _min2; })
> +
> +#define max(x, y) ({				\
> +	typeof(x) _max1 = (x);			\
> +	typeof(y) _max2 = (y);			\
> +	(void) (&_max1 ==&_max2);		\
> +	_max1>  _max2 ? _max1 : _max2; })
> +
> +#define clamp(val, min, max) ({			\
> +	typeof(val) __val = (val);		\
> +	typeof(min) __min = (min);		\
> +	typeof(max) __max = (max);		\
> +	(void) (&__val ==&__min);		\
> +	(void) (&__val ==&__max);		\
> +	__val = __val<  __min ? __min: __val;	\
> +	__val>  __max ? __max: __val; })
>    

You can drop these macros.

Regards,

Anthony Liguori

> +
> +#endif /* __QEMU_VNC_ENCODING_TIGHT_H */
> diff --git a/vnc-encoding-zlib.c b/vnc-encoding-zlib.c
> index 29dd1b7..17a61bf 100644
> --- a/vnc-encoding-zlib.c
> +++ b/vnc-encoding-zlib.c
> @@ -28,7 +28,7 @@
>
>   #define ZALLOC_ALIGNMENT 16
>
> -static void *zalloc(void *x, unsigned items, unsigned size)
> +void *vnc_zlib_zalloc(void *x, unsigned items, unsigned size)
>   {
>       void *p;
>
> @@ -40,7 +40,7 @@ static void *zalloc(void *x, unsigned items, unsigned size)
>       return (p);
>   }
>
> -static void zfree(void *x, void *addr)
> +void vnc_zlib_zfree(void *x, void *addr)
>   {
>       qemu_free(addr);
>   }
> @@ -72,8 +72,8 @@ static int vnc_zlib_stop(VncState *vs)
>
>           VNC_DEBUG("VNC: initializing zlib stream\n");
>           VNC_DEBUG("VNC: opaque = %p | vs = %p\n", zstream->opaque, vs);
> -        zstream->zalloc = zalloc;
> -        zstream->zfree = zfree;
> +        zstream->zalloc = vnc_zlib_zalloc;
> +        zstream->zfree = vnc_zlib_zfree;
>
>           err = deflateInit2(zstream, vs->tight_compression, Z_DEFLATED, MAX_WBITS,
>                              MAX_MEM_LEVEL, Z_DEFAULT_STRATEGY);
> @@ -116,7 +116,7 @@ static int vnc_zlib_stop(VncState *vs)
>       return zstream->total_out - previous_out;
>   }
>
> -void vnc_zlib_send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
> +int vnc_zlib_send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
>   {
>       int old_offset, new_offset, bytes_written;
>
> @@ -131,14 +131,17 @@ void vnc_zlib_send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
>       vnc_raw_send_framebuffer_update(vs, x, y, w, h);
>       bytes_written = vnc_zlib_stop(vs);
>
> -    if (bytes_written == -1)
> -        return;
> +    if (bytes_written == -1) {
> +        return 0;
> +    }
>
>       // hack in the size
>       new_offset = vs->output.offset;
>       vs->output.offset = old_offset;
>       vnc_write_u32(vs, bytes_written);
>       vs->output.offset = new_offset;
> +
> +    return 1;
>   }
>
>   void vnc_zlib_init(VncState *vs)
> diff --git a/vnc.c b/vnc.c
> index d0c0d00..c7b76ff 100644
> --- a/vnc.c
> +++ b/vnc.c
> @@ -638,7 +638,7 @@ static void vnc_write_pixels_generic(VncState *vs, void *pixels1, int size)
>       }
>   }
>
> -void vnc_raw_send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
> +int vnc_raw_send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
>   {
>       int i;
>       uint8_t *row;
> @@ -649,23 +649,31 @@ void vnc_raw_send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
>           vs->write_pixels(vs, row, w * ds_get_bytes_per_pixel(vs->ds));
>           row += ds_get_linesize(vs->ds);
>       }
> +    return 1;
>   }
>
> -static void send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
> +static int send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
>   {
> +    int n = 0;
> +
>       switch(vs->vnc_encoding) {
>           case VNC_ENCODING_ZLIB:
> -            vnc_zlib_send_framebuffer_update(vs, x, y, w, h);
> +            n = vnc_zlib_send_framebuffer_update(vs, x, y, w, h);
>               break;
>           case VNC_ENCODING_HEXTILE:
>               vnc_framebuffer_update(vs, x, y, w, h, VNC_ENCODING_HEXTILE);
> -            vnc_hextile_send_framebuffer_update(vs, x, y, w, h);
> +            n = vnc_hextile_send_framebuffer_update(vs, x, y, w, h);
> +            break;
> +        case VNC_ENCODING_TIGHT:
> +            n = vnc_tight_send_framebuffer_update(vs, x, y, w, h);
>               break;
>           default:
>               vnc_framebuffer_update(vs, x, y, w, h, VNC_ENCODING_RAW);
> -            vnc_raw_send_framebuffer_update(vs, x, y, w, h);
> +            n = vnc_raw_send_framebuffer_update(vs, x, y, w, h);
>               break;
>       }
> +
> +    return n;
>   }
>
>   static void vnc_copy(VncState *vs, int src_x, int src_y, int dst_x, int dst_y, int w, int h)
> @@ -776,6 +784,7 @@ static int vnc_update_client(VncState *vs, int has_dirty)
>           int y;
>           int n_rectangles;
>           int saved_offset;
> +        int n;
>
>           if (vs->output.offset&&  !vs->audio_cap&&  !vs->force_update)
>               /* kernel send buffers are full ->  drop frames to throttle */
> @@ -808,16 +817,18 @@ static int vnc_update_client(VncState *vs, int has_dirty)
>                   } else {
>                       if (last_x != -1) {
>                           int h = find_and_clear_dirty_height(vs, y, last_x, x);
> -                        send_framebuffer_update(vs, last_x * 16, y, (x - last_x) * 16, h);
> -                        n_rectangles++;
> +                        n = send_framebuffer_update(vs, last_x * 16, y,
> +                                                    (x - last_x) * 16, h);
> +                        n_rectangles += n;
>                       }
>                       last_x = -1;
>                   }
>               }
>               if (last_x != -1) {
>                   int h = find_and_clear_dirty_height(vs, y, last_x, x);
> -                send_framebuffer_update(vs, last_x * 16, y, (x - last_x) * 16, h);
> -                n_rectangles++;
> +                n = send_framebuffer_update(vs, last_x * 16, y,
> +                                            (x - last_x) * 16, h);
> +                n_rectangles += n;
>               }
>           }
>           vs->output.buffer[saved_offset] = (n_rectangles>>  8)&  0xFF;
> @@ -1600,8 +1611,10 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
>       unsigned int enc = 0;
>
>       vnc_zlib_init(vs);
> +    vnc_tight_init(vs);
>       vs->features = 0;
> -    vs->vnc_encoding = -1;
> +    if (!vs->vnc_encoding)
> +        vs->vnc_encoding = -1;
>       vs->tight_compression = 9;
>       vs->tight_quality = 9;
>       vs->absolute = -1;
> @@ -1619,6 +1632,10 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
>               vs->features |= VNC_FEATURE_HEXTILE_MASK;
>               set_encoding(vs, enc);
>               break;
> +        case VNC_ENCODING_TIGHT:
> +            vs->features |= VNC_FEATURE_TIGHT_MASK;
> +            set_encoding(vs, enc);
> +            break;
>           case VNC_ENCODING_ZLIB:
>               vs->features |= VNC_FEATURE_ZLIB_MASK;
>               set_encoding(vs, enc);
> diff --git a/vnc.h b/vnc.h
> index 96f3fe7..ec7f481 100644
> --- a/vnc.h
> +++ b/vnc.h
> @@ -166,6 +166,12 @@ struct VncState
>       /* Tight */
>       uint8_t tight_quality;
>       uint8_t tight_compression;
> +    uint8_t tight_pixel24;
> +    Buffer tight;
> +    Buffer tight_tmp;
> +    Buffer tight_zlib;
> +    int tight_levels[4];
> +    z_stream tight_stream[4];
>
>       /* Hextile */
>       VncSendHextileTile *send_hextile_tile;
> @@ -391,13 +397,18 @@ void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
>   void vnc_convert_pixel(VncState *vs, uint8_t *buf, uint32_t v);
>
>   /* Encodings */
> -void vnc_raw_send_framebuffer_update(VncState *vs, int x, int y, int w, int h);
> +int vnc_raw_send_framebuffer_update(VncState *vs, int x, int y, int w, int h);
>
> -void vnc_hextile_send_framebuffer_update(VncState *vs, int x,
> +int vnc_hextile_send_framebuffer_update(VncState *vs, int x,
>                                            int y, int w, int h);
>   void vnc_hextile_set_pixel_conversion(VncState *vs, int generic);
>
>   void vnc_zlib_init(VncState *vs);
> -void vnc_zlib_send_framebuffer_update(VncState *vs, int x, int y, int w, int h);
> +void *vnc_zlib_zalloc(void *x, unsigned items, unsigned size);
> +void vnc_zlib_zfree(void *x, void *addr);
> +int vnc_zlib_send_framebuffer_update(VncState *vs, int x, int y, int w, int h);
> +
> +void vnc_tight_init(VncState *vs);
> +int vnc_tight_send_framebuffer_update(VncState *vs, int x, int y, int w, int h);
>
>   #endif /* __QEMU_VNC_H */
>