Re: [PATCH v2 01/18] worker: move encoders to dcc-encoders

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> On Wed, Nov 18, 2015 at 10:42 PM, Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> wrote:
> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> >
> > Author:    Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> > Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> > ---
> >
> > Changes since v1
> >
> > - Remove extra space in "# define"
> > - remove commented-out "fixme: remove?" lines
> >
> >  server/Makefile.am       |   2 +
> >  server/dcc-encoders.c    | 428 ++++++++++++++++++++++++++++++++++++
> >  server/dcc-encoders.h    | 153 +++++++++++++
> >  server/display-channel.c |   6 +-
> >  server/display-channel.h |  74 ++-----
> >  server/red_worker.c      | 548
> >  ++---------------------------------------------
> >  6 files changed, 620 insertions(+), 591 deletions(-)
> >  create mode 100644 server/dcc-encoders.c
> >  create mode 100644 server/dcc-encoders.h
> >
> > diff --git a/server/Makefile.am b/server/Makefile.am
> > index 52703c9..5907dd2 100644
> > --- a/server/Makefile.am
> > +++ b/server/Makefile.am
> > @@ -138,6 +138,8 @@ libspice_server_la_SOURCES =                        \
> >         utils.h                                 \
> >         stream.c                                        \
> >         stream.h                                        \
> > +       dcc-encoders.c                                  \
> > +       dcc-encoders.h                                  \
> >         $(NULL)
> >
> >  if HAVE_GL
> > diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> > new file mode 100644
> > index 0000000..45db96f
> > --- /dev/null
> > +++ b/server/dcc-encoders.c
> > @@ -0,0 +1,428 @@
> > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > +/*
> > +   Copyright (C) 2009-2015 Red Hat, Inc.
> > +
> > +   This library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   This library 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
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with this library; if not, see
> > <http://www.gnu.org/licenses/>.
> > +*/
> > +#ifdef HAVE_CONFIG_H
> > +#include <config.h>
> > +#endif
> > +
> > +#include <glib.h>
> > +#include <setjmp.h>
> > +
> > +#include "dcc-encoders.h"
> > +#include "display-channel.h"
> > +
> > +#define ZLIB_DEFAULT_COMPRESSION_LEVEL 3
> > +
> > +static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
> > +quic_usr_error(QuicUsrContext *usr, const char *fmt, ...)
> > +{
> > +    EncoderData *usr_data = &(((QuicData *)usr)->data);
> > +    va_list ap;
> > +
> > +    va_start(ap, fmt);
> > +    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt,
> > ap);
> > +    va_end(ap);
> > +    spice_critical("%s", usr_data->message_buf);
> > +
> > +    longjmp(usr_data->jmp_env, 1);
> > +}
> > +
> > +static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
> > +lz_usr_error(LzUsrContext *usr, const char *fmt, ...)
> > +{
> > +    EncoderData *usr_data = &(((LzData *)usr)->data);
> > +    va_list ap;
> > +
> > +    va_start(ap, fmt);
> > +    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt,
> > ap);
> > +    va_end(ap);
> > +    spice_critical("%s", usr_data->message_buf);
> > +
> > +    longjmp(usr_data->jmp_env, 1);
> > +}
> > +
> > +static SPICE_GNUC_PRINTF(2, 3) void
> > +glz_usr_error(GlzEncoderUsrContext *usr, const char *fmt, ...)
> > +{
> > +    EncoderData *usr_data = &(((GlzData *)usr)->data);
> > +    va_list ap;
> > +
> > +    va_start(ap, fmt);
> > +    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt,
> > ap);
> > +    va_end(ap);
> > +
> > +    spice_critical("%s", usr_data->message_buf); // if global lz fails in
> > the middle
> > +                                        // the consequences are not
> > predictable since the window
> > +                                        // can turn to be unsynchronized
> > between the server and
> > +                                        // and the client
> > +}
> 
> These 3 functions have pretty much the same code, which could be
> easily replaced by a macro.
> 
> > +
> > +static SPICE_GNUC_PRINTF(2, 3) void
> > +quic_usr_warn(QuicUsrContext *usr, const char *fmt, ...)
> > +{
> > +    EncoderData *usr_data = &(((QuicData *)usr)->data);
> > +    va_list ap;
> > +
> > +    va_start(ap, fmt);
> > +    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt,
> > ap);
> > +    va_end(ap);
> > +    spice_warning("%s", usr_data->message_buf);
> > +}
> > +
> > +static SPICE_GNUC_PRINTF(2, 3) void
> > +lz_usr_warn(LzUsrContext *usr, const char *fmt, ...)
> > +{
> > +    EncoderData *usr_data = &(((LzData *)usr)->data);
> > +    va_list ap;
> > +
> > +    va_start(ap, fmt);
> > +    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt,
> > ap);
> > +    va_end(ap);
> > +    spice_warning("%s", usr_data->message_buf);
> > +}
> > +
> > +static SPICE_GNUC_PRINTF(2, 3) void
> > +glz_usr_warn(GlzEncoderUsrContext *usr, const char *fmt, ...)
> > +{
> > +    EncoderData *usr_data = &(((GlzData *)usr)->data);
> > +    va_list ap;
> > +
> > +    va_start(ap, fmt);
> > +    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt,
> > ap);
> > +    va_end(ap);
> > +    spice_warning("%s", usr_data->message_buf);
> > +}
> > +
> 
> Same here ...
> 
> > +static void *quic_usr_malloc(QuicUsrContext *usr, int size)
> > +{
> > +    return spice_malloc(size);
> > +}
> > +
> > +static void *lz_usr_malloc(LzUsrContext *usr, int size)
> > +{
> > +    return spice_malloc(size);
> > +}
> > +
> > +static void *glz_usr_malloc(GlzEncoderUsrContext *usr, int size)
> > +{
> > +    return spice_malloc(size);
> > +}
> > +
> > +static void quic_usr_free(QuicUsrContext *usr, void *ptr)
> > +{
> > +    free(ptr);
> > +}
> > +
> > +static void lz_usr_free(LzUsrContext *usr, void *ptr)
> > +{
> > +    free(ptr);
> > +}
> > +
> > +static void glz_usr_free(GlzEncoderUsrContext *usr, void *ptr)
> > +{
> > +    free(ptr);
> > +}
> > +
> > +RedCompressBuf* compress_buf_new(void)
> > +{
> > +    RedCompressBuf *buf = g_slice_new(RedCompressBuf);
> > +
> > +    buf->send_next = NULL;
> > +
> > +    return buf;
> > +}
> > +
> > +void compress_buf_free(RedCompressBuf *buf)
> > +{
> > +    g_slice_free(RedCompressBuf, buf);
> > +}
> > +
> > +/* Allocate more space for compressed buffer.
> > + * The pointer returned in io_ptr is garanteed to be aligned to 4 bytes.
> > + */
> > +static int encoder_usr_more_space(EncoderData *enc_data, uint8_t **io_ptr)
> > +{
> > +    RedCompressBuf *buf;
> > +
> > +    buf = compress_buf_new();
> > +    enc_data->bufs_tail->send_next = buf;
> > +    enc_data->bufs_tail = buf;
> > +    buf->send_next = NULL;
> > +    *io_ptr = buf->buf.bytes;
> > +    return sizeof(buf->buf);
> > +}
> > +
> > +static int quic_usr_more_space(QuicUsrContext *usr, uint32_t **io_ptr, int
> > rows_completed)
> > +{
> > +    EncoderData *usr_data = &(((QuicData *)usr)->data);
> > +    return encoder_usr_more_space(usr_data, (uint8_t **)io_ptr) /
> > sizeof(uint32_t);
> > +}
> > +
> > +static int lz_usr_more_space(LzUsrContext *usr, uint8_t **io_ptr)
> > +{
> > +    EncoderData *usr_data = &(((LzData *)usr)->data);
> > +    return encoder_usr_more_space(usr_data, io_ptr);
> > +}
> > +
> > +static int glz_usr_more_space(GlzEncoderUsrContext *usr, uint8_t **io_ptr)
> > +{
> > +    EncoderData *usr_data = &(((GlzData *)usr)->data);
> > +    return encoder_usr_more_space(usr_data, io_ptr);
> > +}
> > +
> > +static int jpeg_usr_more_space(JpegEncoderUsrContext *usr, uint8_t
> > **io_ptr)
> > +{
> > +    EncoderData *usr_data = &(((JpegData *)usr)->data);
> > +    return encoder_usr_more_space(usr_data, io_ptr);
> > +}
> > +
> > +#ifdef USE_LZ4
> > +static int lz4_usr_more_space(Lz4EncoderUsrContext *usr, uint8_t **io_ptr)
> > +{
> > +    EncoderData *usr_data = &(((Lz4Data *)usr)->data);
> > +    return encoder_usr_more_space(usr_data, io_ptr);
> > +}
> > +#endif
> > +
> > +static int zlib_usr_more_space(ZlibEncoderUsrContext *usr, uint8_t
> > **io_ptr)
> > +{
> > +    EncoderData *usr_data = &(((ZlibData *)usr)->data);
> > +    return encoder_usr_more_space(usr_data, io_ptr);
> > +}
> 
> 
> A macro could be used for pretty much all these _more_space functions as
> well.
> 
> > +
> > +static inline int encoder_usr_more_lines(EncoderData *enc_data, uint8_t
> > **lines)
> > +{
> > +    struct SpiceChunk *chunk;
> > +
> > +    if (enc_data->u.lines_data.reverse) {
> > +        if (!(enc_data->u.lines_data.next >= 0)) {
> > +            return 0;
> > +        }
> > +    } else {
> > +        if (!(enc_data->u.lines_data.next <
> > enc_data->u.lines_data.chunks->num_chunks)) {
> > +            return 0;
> > +        }
> > +    }
> > +
> > +    chunk =
> > &enc_data->u.lines_data.chunks->chunk[enc_data->u.lines_data.next];
> > +    if (chunk->len % enc_data->u.lines_data.stride) {
> > +        return 0;
> > +    }
> > +
> > +    if (enc_data->u.lines_data.reverse) {
> > +        enc_data->u.lines_data.next--;
> > +        *lines = chunk->data + chunk->len - enc_data->u.lines_data.stride;
> > +    } else {
> > +        enc_data->u.lines_data.next++;
> > +        *lines = chunk->data;
> > +    }
> > +
> > +    return chunk->len / enc_data->u.lines_data.stride;
> > +}
> > +
> > +static int quic_usr_more_lines(QuicUsrContext *usr, uint8_t **lines)
> > +{
> > +    EncoderData *usr_data = &(((QuicData *)usr)->data);
> > +    return encoder_usr_more_lines(usr_data, lines);
> > +}
> > +
> > +static int lz_usr_more_lines(LzUsrContext *usr, uint8_t **lines)
> > +{
> > +    EncoderData *usr_data = &(((LzData *)usr)->data);
> > +    return encoder_usr_more_lines(usr_data, lines);
> > +}
> > +
> > +static int glz_usr_more_lines(GlzEncoderUsrContext *usr, uint8_t **lines)
> > +{
> > +    EncoderData *usr_data = &(((GlzData *)usr)->data);
> > +    return encoder_usr_more_lines(usr_data, lines);
> > +}
> > +
> > +static int jpeg_usr_more_lines(JpegEncoderUsrContext *usr, uint8_t
> > **lines)
> > +{
> > +    EncoderData *usr_data = &(((JpegData *)usr)->data);
> > +    return encoder_usr_more_lines(usr_data, lines);
> > +}
> > +
> > +#ifdef USE_LZ4
> > +static int lz4_usr_more_lines(Lz4EncoderUsrContext *usr, uint8_t **lines)
> > +{
> > +    EncoderData *usr_data = &(((Lz4Data *)usr)->data);
> > +    return encoder_usr_more_lines(usr_data, lines);
> > +}
> > +#endif
> > +
> 
> Same for _more_lines ...
> 
> > +static int zlib_usr_more_input(ZlibEncoderUsrContext *usr, uint8_t**
> > input)
> > +{
> > +    EncoderData *usr_data = &(((ZlibData *)usr)->data);
> > +    int buf_size;
> > +
> > +    if (!usr_data->u.compressed_data.next) {
> > +        spice_assert(usr_data->u.compressed_data.size_left == 0);
> > +        return 0;
> > +    }
> > +
> > +    *input = usr_data->u.compressed_data.next->buf.bytes;
> > +    buf_size = MIN(sizeof(usr_data->u.compressed_data.next->buf),
> > +                   usr_data->u.compressed_data.size_left);
> > +
> > +    usr_data->u.compressed_data.next =
> > usr_data->u.compressed_data.next->send_next;
> > +    usr_data->u.compressed_data.size_left -= buf_size;
> > +    return buf_size;
> > +}
> > +
> > +static void dcc_init_quic(DisplayChannelClient *dcc)
> > +{
> > +    dcc->quic_data.usr.error = quic_usr_error;
> > +    dcc->quic_data.usr.warn = quic_usr_warn;
> > +    dcc->quic_data.usr.info = quic_usr_warn;
> > +    dcc->quic_data.usr.malloc = quic_usr_malloc;
> > +    dcc->quic_data.usr.free = quic_usr_free;
> > +    dcc->quic_data.usr.more_space = quic_usr_more_space;
> > +    dcc->quic_data.usr.more_lines = quic_usr_more_lines;
> > +
> > +    dcc->quic = quic_create(&dcc->quic_data.usr);
> > +
> > +    if (!dcc->quic) {
> > +        spice_critical("create quic failed");
> > +    }
> > +}
> > +
> > +static void dcc_init_lz(DisplayChannelClient *dcc)
> > +{
> > +    dcc->lz_data.usr.error = lz_usr_error;
> > +    dcc->lz_data.usr.warn = lz_usr_warn;
> > +    dcc->lz_data.usr.info = lz_usr_warn;
> > +    dcc->lz_data.usr.malloc = lz_usr_malloc;
> > +    dcc->lz_data.usr.free = lz_usr_free;
> > +    dcc->lz_data.usr.more_space = lz_usr_more_space;
> > +    dcc->lz_data.usr.more_lines = lz_usr_more_lines;
> > +
> > +    dcc->lz = lz_create(&dcc->lz_data.usr);
> > +
> > +    if (!dcc->lz) {
> > +        spice_critical("create lz failed");
> > +    }
> > +}
> > +
> > +static void glz_usr_free_image(GlzEncoderUsrContext *usr,
> > GlzUsrImageContext *image)
> > +{
> > +    GlzData *lz_data = (GlzData *)usr;
> > +    GlzDrawableInstanceItem *glz_drawable_instance =
> > (GlzDrawableInstanceItem *)image;
> > +    DisplayChannelClient *drawable_cc =
> > glz_drawable_instance->red_glz_drawable->dcc;
> > +    DisplayChannelClient *this_cc = SPICE_CONTAINEROF(lz_data,
> > DisplayChannelClient, glz_data);
> > +    if (this_cc == drawable_cc) {
> > +        dcc_free_glz_drawable_instance(drawable_cc,
> > glz_drawable_instance);
> > +    } else {
> > +        /* The glz dictionary is shared between all DisplayChannelClient
> > +         * instances that belong to the same client, and
> > glz_usr_free_image
> > +         * can be called by the dictionary code
> > +         * (glz_dictionary_window_remove_head). Thus this function can be
> > +         * called from any DisplayChannelClient thread, hence the need for
> > +         * this check.
> > +         */
> > +        pthread_mutex_lock(&drawable_cc->glz_drawables_inst_to_free_lock);
> > +        ring_add_before(&glz_drawable_instance->free_link,
> > +                        &drawable_cc->glz_drawables_inst_to_free);
> > +
> > pthread_mutex_unlock(&drawable_cc->glz_drawables_inst_to_free_lock);
> > +    }
> > +}
> > +
> > +static void dcc_init_glz_data(DisplayChannelClient *dcc)
> > +{
> > +    dcc->glz_data.usr.error = glz_usr_error;
> > +    dcc->glz_data.usr.warn = glz_usr_warn;
> > +    dcc->glz_data.usr.info = glz_usr_warn;
> > +    dcc->glz_data.usr.malloc = glz_usr_malloc;
> > +    dcc->glz_data.usr.free = glz_usr_free;
> > +    dcc->glz_data.usr.more_space = glz_usr_more_space;
> > +    dcc->glz_data.usr.more_lines = glz_usr_more_lines;
> > +    dcc->glz_data.usr.free_image = glz_usr_free_image;
> > +}
> > +
> > +static void dcc_init_jpeg(DisplayChannelClient *dcc)
> > +{
> > +    dcc->jpeg_data.usr.more_space = jpeg_usr_more_space;
> > +    dcc->jpeg_data.usr.more_lines = jpeg_usr_more_lines;
> > +
> > +    dcc->jpeg = jpeg_encoder_create(&dcc->jpeg_data.usr);
> > +
> > +    if (!dcc->jpeg) {
> > +        spice_critical("create jpeg encoder failed");
> > +    }
> > +}
> > +
> > +#ifdef USE_LZ4
> > +static inline void dcc_init_lz4(DisplayChannelClient *dcc)
> > +{
> > +    dcc->lz4_data.usr.more_space = lz4_usr_more_space;
> > +    dcc->lz4_data.usr.more_lines = lz4_usr_more_lines;
> > +
> > +    dcc->lz4 = lz4_encoder_create(&dcc->lz4_data.usr);
> > +
> > +    if (!dcc->lz4) {
> > +        spice_critical("create lz4 encoder failed");
> > +    }
> > +}
> > +#endif
> > +
> > +static void dcc_init_zlib(DisplayChannelClient *dcc)
> > +{
> > +    dcc->zlib_data.usr.more_space = zlib_usr_more_space;
> > +    dcc->zlib_data.usr.more_input = zlib_usr_more_input;
> > +
> > +    dcc->zlib = zlib_encoder_create(&dcc->zlib_data.usr,
> > ZLIB_DEFAULT_COMPRESSION_LEVEL);
> > +
> > +    if (!dcc->zlib) {
> > +        spice_critical("create zlib encoder failed");
> > +    }
> > +}
> > +
> > +void dcc_encoders_init(DisplayChannelClient *dcc)
> > +{
> > +    dcc_init_glz_data(dcc);
> > +    dcc_init_quic(dcc);
> > +    dcc_init_lz(dcc);
> > +    dcc_init_jpeg(dcc);
> > +#ifdef USE_LZ4
> > +    dcc_init_lz4(dcc);
> > +#endif
> > +    dcc_init_zlib(dcc);
> > +
> > +    // todo: tune level according to bandwidth
> > +    dcc->zlib_level = ZLIB_DEFAULT_COMPRESSION_LEVEL;
> > +}
> > +
> > +static void marshaller_compress_buf_free(uint8_t *data, void *opaque)
> > +{
> > +    compress_buf_free((RedCompressBuf *) opaque);
> > +}
> > +
> > +void marshaller_add_compressed(SpiceMarshaller *m,
> > +                               RedCompressBuf *comp_buf, size_t size)
> > +{
> > +    size_t max = size;
> > +    size_t now;
> > +    do {
> > +        spice_return_if_fail(comp_buf);
> > +        now = MIN(sizeof(comp_buf->buf), max);
> > +        max -= now;
> > +        spice_marshaller_add_ref_full(m, comp_buf->buf.bytes, now,
> > +                                      marshaller_compress_buf_free,
> > comp_buf);
> > +        comp_buf = comp_buf->send_next;
> > +    } while (max);
> > +}
> > diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> > new file mode 100644
> > index 0000000..c9e06e7
> > --- /dev/null
> > +++ b/server/dcc-encoders.h
> > @@ -0,0 +1,153 @@
> > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > +/*
> > +   Copyright (C) 2009-2015 Red Hat, Inc.
> > +
> > +   This library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   This library 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
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with this library; if not, see
> > <http://www.gnu.org/licenses/>.
> > +*/
> > +#ifndef DCC_ENCODERS_H_
> > +#define DCC_ENCODERS_H_
> > +
> > +#include "common/marshaller.h"
> > +#include "common/quic.h"
> > +#include "red_channel.h"
> > +#include "red_parse_qxl.h"
> > +#include "spice_image_cache.h"
> > +#include "glz_encoder_dictionary.h"
> > +#include "glz_encoder.h"
> > +#include "jpeg_encoder.h"
> > +#ifdef USE_LZ4
> > +#include "lz4_encoder.h"
> > +#endif
> > +#include "zlib_encoder.h"
> > +
> > +typedef struct RedCompressBuf RedCompressBuf;
> > +typedef struct GlzDrawableInstanceItem GlzDrawableInstanceItem;
> > +
> > +void             dcc_encoders_init
> > (DisplayChannelClient *dcc);
> > +void             dcc_free_glz_drawable_instance
> > (DisplayChannelClient *dcc,
> > +
> > GlzDrawableInstanceItem
> > *item);
> > +void             marshaller_add_compressed
> > (SpiceMarshaller *m,
> > +
> > RedCompressBuf
> > *comp_buf,
> > +                                                              size_t
> > size);
> > +
> > +RedCompressBuf*  compress_buf_new                            (void);
> > +void             compress_buf_free
> > (RedCompressBuf *buf);
> > +
> > +#define RED_COMPRESS_BUF_SIZE (1024 * 64)
> > +struct RedCompressBuf {
> > +    /* This buffer provide space for compression algorithms.
> > +     * Some algorithms access the buffer as an array of 32 bit words
> > +     * so is defined to make sure is always aligned that way.
> > +     */
> > +    union {
> > +        uint8_t  bytes[RED_COMPRESS_BUF_SIZE];
> > +        uint32_t words[RED_COMPRESS_BUF_SIZE / 4];
> > +    } buf;
> > +    RedCompressBuf *send_next;
> > +};
> > +
> > +typedef struct GlzSharedDictionary {
> > +    RingItem base;
> > +    GlzEncDictContext *dict;
> > +    uint32_t refs;
> > +    uint8_t id;
> > +    pthread_rwlock_t encode_lock;
> > +    int migrate_freeze;
> > +    RedClient *client; // channel clients of the same client share the
> > dict
> > +} GlzSharedDictionary;
> > +
> > +typedef struct  {
> > +    DisplayChannelClient *dcc;
> > +    RedCompressBuf *bufs_head;
> > +    RedCompressBuf *bufs_tail;
> > +    jmp_buf jmp_env;
> > +    union {
> > +        struct {
> > +            SpiceChunks *chunks;
> > +            int next;
> > +            int stride;
> > +            int reverse;
> > +        } lines_data;
> > +        struct {
> > +            RedCompressBuf* next;
> > +            int size_left;
> > +        } compressed_data; // for encoding data that was already
> > compressed by another method
> > +    } u;
> > +    char message_buf[512];
> > +} EncoderData;
> > +
> > +typedef struct {
> > +    QuicUsrContext usr;
> > +    EncoderData data;
> > +} QuicData;
> > +
> > +typedef struct {
> > +    LzUsrContext usr;
> > +    EncoderData data;
> > +} LzData;
> > +
> > +typedef struct {
> > +    JpegEncoderUsrContext usr;
> > +    EncoderData data;
> > +} JpegData;
> > +
> > +#ifdef USE_LZ4
> > +typedef struct {
> > +    Lz4EncoderUsrContext usr;
> > +    EncoderData data;
> > +} Lz4Data;
> > +#endif
> > +
> > +typedef struct {
> > +    ZlibEncoderUsrContext usr;
> > +    EncoderData data;
> > +} ZlibData;
> > +
> > +typedef struct {
> > +    GlzEncoderUsrContext usr;
> > +    EncoderData data;
> > +} GlzData;
> > +
> > +#define MAX_GLZ_DRAWABLE_INSTANCES 2
> > +
> > +typedef struct RedGlzDrawable RedGlzDrawable;
> > +
> > +/* for each qxl drawable, there may be several instances of lz drawables
> > */
> > +/* TODO - reuse this stuff for the top level. I just added a second level
> > of multiplicity
> > + * at the Drawable by keeping a ring, so:
> > + * Drawable -> (ring of) RedGlzDrawable -> (up to 2)
> > GlzDrawableInstanceItem
> > + * and it should probably (but need to be sure...) be
> > + * Drawable -> ring of GlzDrawableInstanceItem.
> > + */
> > +struct GlzDrawableInstanceItem {
> > +    RingItem glz_link;
> > +    RingItem free_link;
> > +    GlzEncDictImageContext *glz_instance;
> > +    RedGlzDrawable         *red_glz_drawable;
> > +};
> > +
> > +struct RedGlzDrawable {
> > +    RingItem link;    // ordered by the time it was encoded
> > +    RingItem drawable_link;
> > +    RedDrawable *red_drawable;
> > +    Drawable    *drawable;
> > +    uint32_t     group_id;
> > +    GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES];
> > +    Ring instances;
> > +    uint8_t instances_count;
> > +    DisplayChannelClient *dcc;
> > +};
> > +
> > +
> > +#endif /* DCC_ENCODERS_H_ */
> > diff --git a/server/display-channel.c b/server/display-channel.c
> > index c26b5dd..d4fcc7e 100644
> > --- a/server/display-channel.c
> > +++ b/server/display-channel.c
> > @@ -159,6 +159,10 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
> >      dcc->image_compression = image_compression;
> >      dcc->jpeg_state = jpeg_state;
> >      dcc->zlib_glz_state = zlib_glz_state;
> > +    // todo: tune quality according to bandwidth
> 
> s/todo/TODO/
> 
> > +    dcc->jpeg_quality = 85;
> > +
> > +    dcc_encoders_init(dcc);
> >
> >      return dcc;
> >  }
> > @@ -236,7 +240,7 @@ static MonitorsConfigItem
> > *monitors_config_item_new(RedChannel* channel,
> >      return mci;
> >  }
> >
> > -static inline void red_monitors_config_item_add(DisplayChannelClient *dcc)
> > +static void red_monitors_config_item_add(DisplayChannelClient *dcc)
> >  {
> >      DisplayChannel *dc = DCC_TO_DC(dcc);
> >      MonitorsConfigItem *mci;
> > diff --git a/server/display-channel.h b/server/display-channel.h
> > index 8deb71c..90f8455 100644
> > --- a/server/display-channel.h
> > +++ b/server/display-channel.h
> > @@ -30,20 +30,13 @@
> >  #include "reds_gl_canvas.h"
> >  #endif /* USE_OPENGL */
> >  #include "reds_sw_canvas.h"
> > -#include "glz_encoder_dictionary.h"
> > -#include "glz_encoder.h"
> >  #include "stat.h"
> >  #include "reds.h"
> >  #include "mjpeg_encoder.h"
> >  #include "red_memslots.h"
> >  #include "red_parse_qxl.h"
> >  #include "red_record_qxl.h"
> > -#include "jpeg_encoder.h"
> > -#ifdef USE_LZ4
> > -#include "lz4_encoder.h"
> > -#endif
> >  #include "demarshallers.h"
> > -#include "zlib_encoder.h"
> >  #include "red_channel.h"
> >  #include "red_dispatcher.h"
> >  #include "dispatcher.h"
> > @@ -56,6 +49,7 @@
> >  #include "utils.h"
> >  #include "tree.h"
> >  #include "stream.h"
> > +#include "dcc-encoders.h"
> >
> >  #define PALETTE_CACHE_HASH_SHIFT 8
> >  #define PALETTE_CACHE_HASH_SIZE (1 << PALETTE_CACHE_HASH_SHIFT)
> > @@ -70,20 +64,6 @@
> >  #define NUM_STREAMS 50
> >  #define NUM_SURFACES 10000
> >
> > -#define RED_COMPRESS_BUF_SIZE (1024 * 64)
> > -typedef struct RedCompressBuf RedCompressBuf;
> > -struct RedCompressBuf {
> > -    /* This buffer provide space for compression algorithms.
> > -     * Some algorithms access the buffer as an array of 32 bit words
> > -     * so is defined to make sure is always aligned that way.
> > -     */
> > -    union {
> > -        uint8_t  bytes[RED_COMPRESS_BUF_SIZE];
> > -        uint32_t words[RED_COMPRESS_BUF_SIZE / 4];
> > -    } buf;
> > -    RedCompressBuf *send_next;
> > -};
> > -
> >  typedef struct WaitForChannels {
> >      SpiceMsgWaitForChannels header;
> >      SpiceWaitForChannel buf[MAX_CACHE_CLIENTS];
> > @@ -96,41 +76,6 @@ typedef struct FreeList {
> >      WaitForChannels wait;
> >  } FreeList;
> >
> > -typedef struct GlzSharedDictionary {
> > -    RingItem base;
> > -    GlzEncDictContext *dict;
> > -    uint32_t refs;
> > -    uint8_t id;
> > -    pthread_rwlock_t encode_lock;
> > -    int migrate_freeze;
> > -    RedClient *client; // channel clients of the same client share the
> > dict
> > -} GlzSharedDictionary;
> > -
> > -typedef struct  {
> > -    DisplayChannelClient *dcc;
> > -    RedCompressBuf *bufs_head;
> > -    RedCompressBuf *bufs_tail;
> > -    jmp_buf jmp_env;
> > -    union {
> > -        struct {
> > -            SpiceChunks *chunks;
> > -            int next;
> > -            int stride;
> > -            int reverse;
> > -        } lines_data;
> > -        struct {
> > -            RedCompressBuf* next;
> > -            int size_left;
> > -        } compressed_data; // for encoding data that was already
> > compressed by another method
> > -    } u;
> > -    char message_buf[512];
> > -} EncoderData;
> > -
> > -typedef struct {
> > -    GlzEncoderUsrContext usr;
> > -    EncoderData data;
> > -} GlzData;
> > -
> >  typedef struct DependItem {
> >      Drawable *drawable;
> >      RingItem ring_item;
> > @@ -175,6 +120,21 @@ struct DisplayChannelClient {
> >      SpiceImageCompression image_compression;
> >      spice_wan_compression_t jpeg_state;
> >      spice_wan_compression_t zlib_glz_state;
> > +    int jpeg_quality;
> > +    int zlib_level;
> > +
> > +    QuicData quic_data;
> > +    QuicContext *quic;
> > +    LzData lz_data;
> > +    LzContext  *lz;
> > +    JpegData jpeg_data;
> > +    JpegEncoderContext *jpeg;
> > +#ifdef USE_LZ4
> > +    Lz4Data lz4_data;
> > +    Lz4EncoderContext *lz4;
> > +#endif
> > +    ZlibData zlib_data;
> > +    ZlibEncoder *zlib;
> >
> >      int expect_init;
> >
> > @@ -333,9 +293,7 @@ struct DisplayChannel {
> >      uint32_t renderers[RED_RENDERER_LAST];
> >      uint32_t renderer;
> >      int enable_jpeg;
> > -    int jpeg_quality;
> >      int enable_zlib_glz_wrap;
> > -    int zlib_level;
> >
> >      Ring current_list; // of TreeItem
> >      uint32_t current_size;
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index 16677fc..5609b47 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -51,7 +51,6 @@
> >  #include <spice/qxl_dev.h>
> >  #include "common/lz.h"
> >  #include "common/marshaller.h"
> > -#include "common/quic.h"
> >  #include "common/rect.h"
> >  #include "common/region.h"
> >  #include "common/ring.h"
> > @@ -79,7 +78,6 @@
> >
> >  #define DISPLAY_FREE_LIST_DEFAULT_SIZE 128
> >
> > -#define ZLIB_DEFAULT_COMPRESSION_LEVEL 3
> >  #define MIN_GLZ_SIZE_FOR_ZLIB 100
> >
> >  #define VALIDATE_SURFACE_RET(worker, surface_id) \
> > @@ -136,66 +134,6 @@ typedef struct ImageItem {
> >      uint8_t data[0];
> >  } ImageItem;
> >
> > -typedef struct {
> > -    QuicUsrContext usr;
> > -    EncoderData data;
> > -} QuicData;
> > -
> > -typedef struct {
> > -    LzUsrContext usr;
> > -    EncoderData data;
> > -} LzData;
> > -
> > -typedef struct {
> > -    JpegEncoderUsrContext usr;
> > -    EncoderData data;
> > -} JpegData;
> > -
> > -#ifdef USE_LZ4
> > -typedef struct {
> > -    Lz4EncoderUsrContext usr;
> > -    EncoderData data;
> > -} Lz4Data;
> > -#endif
> > -
> > -typedef struct {
> > -    ZlibEncoderUsrContext usr;
> > -    EncoderData data;
> > -} ZlibData;
> > -
> > -/**********************************/
> > -/* LZ dictionary related entities */
> > -/**********************************/
> > -#define MAX_GLZ_DRAWABLE_INSTANCES 2
> > -
> > -typedef struct RedGlzDrawable RedGlzDrawable;
> > -
> > -/* for each qxl drawable, there may be several instances of lz drawables
> > */
> > -/* TODO - reuse this stuff for the top level. I just added a second level
> > of multiplicity
> > - * at the Drawable by keeping a ring, so:
> > - * Drawable -> (ring of) RedGlzDrawable -> (up to 2)
> > GlzDrawableInstanceItem
> > - * and it should probably (but need to be sure...) be
> > - * Drawable -> ring of GlzDrawableInstanceItem.
> > - */
> > -typedef struct GlzDrawableInstanceItem {
> > -    RingItem glz_link;
> > -    RingItem free_link;
> > -    GlzEncDictImageContext *glz_instance;
> > -    RedGlzDrawable         *red_glz_drawable;
> > -} GlzDrawableInstanceItem;
> > -
> > -struct RedGlzDrawable {
> > -    RingItem link;    // ordered by the time it was encoded
> > -    RingItem drawable_link;
> > -    RedDrawable *red_drawable;
> > -    Drawable    *drawable;
> > -    uint32_t     group_id;
> > -    GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES];
> > -    Ring instances;
> > -    uint8_t instances_count;
> > -    DisplayChannelClient *dcc;
> > -};
> > -
> >  pthread_mutex_t glz_dictionary_list_lock = PTHREAD_MUTEX_INITIALIZER;
> >  Ring glz_dictionary_list = {&glz_dictionary_list, &glz_dictionary_list};
> >
> > @@ -232,23 +170,6 @@ struct RedWorker {
> >      spice_wan_compression_t jpeg_state;
> >      spice_wan_compression_t zlib_glz_state;
> >
> > -    QuicData quic_data;
> > -    QuicContext *quic;
> > -
> > -    LzData lz_data;
> > -    LzContext  *lz;
> > -
> > -    JpegData jpeg_data;
> > -    JpegEncoderContext *jpeg;
> > -
> > -#ifdef USE_LZ4
> > -    Lz4Data lz4_data;
> > -    Lz4EncoderContext *lz4;
> > -#endif
> > -
> > -    ZlibData zlib_data;
> > -    ZlibEncoder *zlib;
> > -
> >      uint32_t process_commands_generation;
> >  #ifdef RED_STATISTICS
> >      StatNodeRef stat;
> > @@ -2175,37 +2096,6 @@ static void
> > red_push_surface_image(DisplayChannelClient *dcc, int surface_id)
> >      red_channel_client_push(RED_CHANNEL_CLIENT(dcc));
> >  }
> >
> > -static RedCompressBuf *compress_buf_new(void)
> > -{
> > -    return g_slice_new(RedCompressBuf);
> > -}
> > -
> > -static inline void compress_buf_free(RedCompressBuf *buf)
> > -{
> > -    g_slice_free(RedCompressBuf, buf);
> > -}
> > -
> > -static void marshaller_compress_buf_free(uint8_t *data, void *opaque)
> > -{
> > -    compress_buf_free((RedCompressBuf *) opaque);
> > -}
> > -
> > -static void marshaller_add_compressed(SpiceMarshaller *m,
> > -                                      RedCompressBuf *comp_buf, size_t
> > size)
> > -{
> > -    size_t max = size;
> > -    size_t now;
> > -    do {
> > -        spice_return_if_fail(comp_buf);
> > -        now = MIN(sizeof(comp_buf->buf), max);
> > -        max -= now;
> > -        spice_marshaller_add_ref_full(m, comp_buf->buf.bytes, now,
> > -                                      marshaller_compress_buf_free,
> > comp_buf);
> > -        comp_buf = comp_buf->send_next;
> > -    } while (max);
> > -}
> > -
> > -
> >  static void fill_base(SpiceMarshaller *base_marshaller, Drawable
> >  *drawable)
> >  {
> >      SpiceMsgDisplayBase base;
> > @@ -2446,357 +2336,6 @@ static int
> > red_display_free_some_independent_glz_drawables(DisplayChannelClient
> >      return n;
> >  }
> >
> > -/******************************************************
> > - *              Encoders callbacks
> > -*******************************************************/
> > -static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
> > -quic_usr_error(QuicUsrContext *usr, const char *fmt, ...)
> > -{
> > -    EncoderData *usr_data = &(((QuicData *)usr)->data);
> > -    va_list ap;
> > -
> > -    va_start(ap, fmt);
> > -    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt,
> > ap);
> > -    va_end(ap);
> > -    spice_critical("%s", usr_data->message_buf);
> > -
> > -    longjmp(usr_data->jmp_env, 1);
> > -}
> > -
> > -static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
> > -lz_usr_error(LzUsrContext *usr, const char *fmt, ...)
> > -{
> > -    EncoderData *usr_data = &(((LzData *)usr)->data);
> > -    va_list ap;
> > -
> > -    va_start(ap, fmt);
> > -    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt,
> > ap);
> > -    va_end(ap);
> > -    spice_critical("%s", usr_data->message_buf);
> > -
> > -    longjmp(usr_data->jmp_env, 1);
> > -}
> > -
> > -static SPICE_GNUC_PRINTF(2, 3) void glz_usr_error(GlzEncoderUsrContext
> > *usr, const char *fmt, ...)
> > -{
> > -    EncoderData *usr_data = &(((GlzData *)usr)->data);
> > -    va_list ap;
> > -
> > -    va_start(ap, fmt);
> > -    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt,
> > ap);
> > -    va_end(ap);
> > -
> > -    spice_critical("%s", usr_data->message_buf); // if global lz fails in
> > the middle
> > -                                        // the consequences are not
> > predictable since the window
> > -                                        // can turn to be unsynchronized
> > between the server and
> > -                                        // and the client
> > -}
> > -
> > -static SPICE_GNUC_PRINTF(2, 3) void quic_usr_warn(QuicUsrContext *usr,
> > const char *fmt, ...)
> > -{
> > -    EncoderData *usr_data = &(((QuicData *)usr)->data);
> > -    va_list ap;
> > -
> > -    va_start(ap, fmt);
> > -    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt,
> > ap);
> > -    va_end(ap);
> > -    spice_warning("%s", usr_data->message_buf);
> > -}
> > -
> > -static SPICE_GNUC_PRINTF(2, 3) void lz_usr_warn(LzUsrContext *usr, const
> > char *fmt, ...)
> > -{
> > -    EncoderData *usr_data = &(((LzData *)usr)->data);
> > -    va_list ap;
> > -
> > -    va_start(ap, fmt);
> > -    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt,
> > ap);
> > -    va_end(ap);
> > -    spice_warning("%s", usr_data->message_buf);
> > -}
> > -
> > -static SPICE_GNUC_PRINTF(2, 3) void glz_usr_warn(GlzEncoderUsrContext
> > *usr, const char *fmt, ...)
> > -{
> > -    EncoderData *usr_data = &(((GlzData *)usr)->data);
> > -    va_list ap;
> > -
> > -    va_start(ap, fmt);
> > -    vsnprintf(usr_data->message_buf, sizeof(usr_data->message_buf), fmt,
> > ap);
> > -    va_end(ap);
> > -    spice_warning("%s", usr_data->message_buf);
> > -}
> > -
> > -static void *quic_usr_malloc(QuicUsrContext *usr, int size)
> > -{
> > -    return spice_malloc(size);
> > -}
> > -
> > -static void *lz_usr_malloc(LzUsrContext *usr, int size)
> > -{
> > -    return spice_malloc(size);
> > -}
> > -
> > -static void *glz_usr_malloc(GlzEncoderUsrContext *usr, int size)
> > -{
> > -    return spice_malloc(size);
> > -}
> > -
> > -static void quic_usr_free(QuicUsrContext *usr, void *ptr)
> > -{
> > -    free(ptr);
> > -}
> > -
> > -static void lz_usr_free(LzUsrContext *usr, void *ptr)
> > -{
> > -    free(ptr);
> > -}
> > -
> > -static void glz_usr_free(GlzEncoderUsrContext *usr, void *ptr)
> > -{
> > -    free(ptr);
> > -}
> > -
> > -/* Allocate more space for compressed buffer.
> > - * The pointer returned in io_ptr is garanteed to be aligned to 4 bytes.
> > - */
> > -static int encoder_usr_more_space(EncoderData *enc_data, uint8_t **io_ptr)
> > -{
> > -    RedCompressBuf *buf;
> > -
> > -    buf = compress_buf_new();
> > -    enc_data->bufs_tail->send_next = buf;
> > -    enc_data->bufs_tail = buf;
> > -    buf->send_next = NULL;
> > -    *io_ptr = buf->buf.bytes;
> > -    return sizeof(buf->buf);
> > -}
> > -
> > -static int quic_usr_more_space(QuicUsrContext *usr, uint32_t **io_ptr, int
> > rows_completed)
> > -{
> > -    EncoderData *usr_data = &(((QuicData *)usr)->data);
> > -    return encoder_usr_more_space(usr_data, (uint8_t **)io_ptr) /
> > sizeof(uint32_t);
> > -}
> > -
> > -static int lz_usr_more_space(LzUsrContext *usr, uint8_t **io_ptr)
> > -{
> > -    EncoderData *usr_data = &(((LzData *)usr)->data);
> > -    return encoder_usr_more_space(usr_data, io_ptr);
> > -}
> > -
> > -static int glz_usr_more_space(GlzEncoderUsrContext *usr, uint8_t **io_ptr)
> > -{
> > -    EncoderData *usr_data = &(((GlzData *)usr)->data);
> > -    return encoder_usr_more_space(usr_data, io_ptr);
> > -}
> > -
> > -static int jpeg_usr_more_space(JpegEncoderUsrContext *usr, uint8_t
> > **io_ptr)
> > -{
> > -    EncoderData *usr_data = &(((JpegData *)usr)->data);
> > -    return encoder_usr_more_space(usr_data, io_ptr);
> > -}
> > -
> > -#ifdef USE_LZ4
> > -static int lz4_usr_more_space(Lz4EncoderUsrContext *usr, uint8_t **io_ptr)
> > -{
> > -    EncoderData *usr_data = &(((Lz4Data *)usr)->data);
> > -    return encoder_usr_more_space(usr_data, io_ptr);
> > -}
> > -#endif
> > -
> > -static int zlib_usr_more_space(ZlibEncoderUsrContext *usr, uint8_t
> > **io_ptr)
> > -{
> > -    EncoderData *usr_data = &(((ZlibData *)usr)->data);
> > -    return encoder_usr_more_space(usr_data, io_ptr);
> > -}
> > -
> > -static inline int encoder_usr_more_lines(EncoderData *enc_data, uint8_t
> > **lines)
> > -{
> > -    struct SpiceChunk *chunk;
> > -
> > -    if (enc_data->u.lines_data.reverse) {
> > -        if (!(enc_data->u.lines_data.next >= 0)) {
> > -            return 0;
> > -        }
> > -    } else {
> > -        if (!(enc_data->u.lines_data.next <
> > enc_data->u.lines_data.chunks->num_chunks)) {
> > -            return 0;
> > -        }
> > -    }
> > -
> > -    chunk =
> > &enc_data->u.lines_data.chunks->chunk[enc_data->u.lines_data.next];
> > -    if (chunk->len % enc_data->u.lines_data.stride) {
> > -        return 0;
> > -    }
> > -
> > -    if (enc_data->u.lines_data.reverse) {
> > -        enc_data->u.lines_data.next--;
> > -        *lines = chunk->data + chunk->len - enc_data->u.lines_data.stride;
> > -    } else {
> > -        enc_data->u.lines_data.next++;
> > -        *lines = chunk->data;
> > -    }
> > -
> > -    return chunk->len / enc_data->u.lines_data.stride;
> > -}
> > -
> > -static int quic_usr_more_lines(QuicUsrContext *usr, uint8_t **lines)
> > -{
> > -    EncoderData *usr_data = &(((QuicData *)usr)->data);
> > -    return encoder_usr_more_lines(usr_data, lines);
> > -}
> > -
> > -static int lz_usr_more_lines(LzUsrContext *usr, uint8_t **lines)
> > -{
> > -    EncoderData *usr_data = &(((LzData *)usr)->data);
> > -    return encoder_usr_more_lines(usr_data, lines);
> > -}
> > -
> > -static int glz_usr_more_lines(GlzEncoderUsrContext *usr, uint8_t **lines)
> > -{
> > -    EncoderData *usr_data = &(((GlzData *)usr)->data);
> > -    return encoder_usr_more_lines(usr_data, lines);
> > -}
> > -
> > -static int jpeg_usr_more_lines(JpegEncoderUsrContext *usr, uint8_t
> > **lines)
> > -{
> > -    EncoderData *usr_data = &(((JpegData *)usr)->data);
> > -    return encoder_usr_more_lines(usr_data, lines);
> > -}
> > -
> > -#ifdef USE_LZ4
> > -static int lz4_usr_more_lines(Lz4EncoderUsrContext *usr, uint8_t **lines)
> > -{
> > -    EncoderData *usr_data = &(((Lz4Data *)usr)->data);
> > -    return encoder_usr_more_lines(usr_data, lines);
> > -}
> > -#endif
> > -
> > -static int zlib_usr_more_input(ZlibEncoderUsrContext *usr, uint8_t**
> > input)
> > -{
> > -    EncoderData *usr_data = &(((ZlibData *)usr)->data);
> > -    int buf_size;
> > -
> > -    if (!usr_data->u.compressed_data.next) {
> > -        spice_assert(usr_data->u.compressed_data.size_left == 0);
> > -        return 0;
> > -    }
> > -
> > -    *input = usr_data->u.compressed_data.next->buf.bytes;
> > -    buf_size = MIN(sizeof(usr_data->u.compressed_data.next->buf),
> > -                   usr_data->u.compressed_data.size_left);
> > -
> > -    usr_data->u.compressed_data.next =
> > usr_data->u.compressed_data.next->send_next;
> > -    usr_data->u.compressed_data.size_left -= buf_size;
> > -    return buf_size;
> > -}
> > -
> > -static void glz_usr_free_image(GlzEncoderUsrContext *usr,
> > GlzUsrImageContext *image)
> > -{
> > -    GlzData *lz_data = (GlzData *)usr;
> > -    GlzDrawableInstanceItem *glz_drawable_instance =
> > (GlzDrawableInstanceItem *)image;
> > -    DisplayChannelClient *drawable_cc =
> > glz_drawable_instance->red_glz_drawable->dcc;
> > -    DisplayChannelClient *this_cc = SPICE_CONTAINEROF(lz_data,
> > DisplayChannelClient, glz_data);
> > -    if (this_cc == drawable_cc) {
> > -        dcc_free_glz_drawable_instance(drawable_cc,
> > glz_drawable_instance);
> > -    } else {
> > -        /* The glz dictionary is shared between all DisplayChannelClient
> > -         * instances that belong to the same client, and
> > glz_usr_free_image
> > -         * can be called by the dictionary code
> > -         * (glz_dictionary_window_remove_head). Thus this function can be
> > -         * called from any DisplayChannelClient thread, hence the need for
> > -         * this check.
> > -         */
> > -        pthread_mutex_lock(&drawable_cc->glz_drawables_inst_to_free_lock);
> > -        ring_add_before(&glz_drawable_instance->free_link,
> > -                        &drawable_cc->glz_drawables_inst_to_free);
> > -
> > pthread_mutex_unlock(&drawable_cc->glz_drawables_inst_to_free_lock);
> > -    }
> > -}
> > -
> > -static inline void red_init_quic(RedWorker *worker)
> > -{
> > -    worker->quic_data.usr.error = quic_usr_error;
> > -    worker->quic_data.usr.warn = quic_usr_warn;
> > -    worker->quic_data.usr.info = quic_usr_warn;
> > -    worker->quic_data.usr.malloc = quic_usr_malloc;
> > -    worker->quic_data.usr.free = quic_usr_free;
> > -    worker->quic_data.usr.more_space = quic_usr_more_space;
> > -    worker->quic_data.usr.more_lines = quic_usr_more_lines;
> > -
> > -    worker->quic = quic_create(&worker->quic_data.usr);
> > -
> > -    if (!worker->quic) {
> > -        spice_critical("create quic failed");
> > -    }
> > -}
> > -
> > -static inline void red_init_lz(RedWorker *worker)
> > -{
> > -    worker->lz_data.usr.error = lz_usr_error;
> > -    worker->lz_data.usr.warn = lz_usr_warn;
> > -    worker->lz_data.usr.info = lz_usr_warn;
> > -    worker->lz_data.usr.malloc = lz_usr_malloc;
> > -    worker->lz_data.usr.free = lz_usr_free;
> > -    worker->lz_data.usr.more_space = lz_usr_more_space;
> > -    worker->lz_data.usr.more_lines = lz_usr_more_lines;
> > -
> > -    worker->lz = lz_create(&worker->lz_data.usr);
> > -
> > -    if (!worker->lz) {
> > -        spice_critical("create lz failed");
> > -    }
> > -}
> > -
> > -/* TODO: split off to DisplayChannel? avoid just copying those cb pointers
> > */
> > -static inline void red_display_init_glz_data(DisplayChannelClient *dcc)
> > -{
> > -    dcc->glz_data.usr.error = glz_usr_error;
> > -    dcc->glz_data.usr.warn = glz_usr_warn;
> > -    dcc->glz_data.usr.info = glz_usr_warn;
> > -    dcc->glz_data.usr.malloc = glz_usr_malloc;
> > -    dcc->glz_data.usr.free = glz_usr_free;
> > -    dcc->glz_data.usr.more_space = glz_usr_more_space;
> > -    dcc->glz_data.usr.more_lines = glz_usr_more_lines;
> > -    dcc->glz_data.usr.free_image = glz_usr_free_image;
> > -}
> > -
> > -static inline void red_init_jpeg(RedWorker *worker)
> > -{
> > -    worker->jpeg_data.usr.more_space = jpeg_usr_more_space;
> > -    worker->jpeg_data.usr.more_lines = jpeg_usr_more_lines;
> > -
> > -    worker->jpeg = jpeg_encoder_create(&worker->jpeg_data.usr);
> > -
> > -    if (!worker->jpeg) {
> > -        spice_critical("create jpeg encoder failed");
> > -    }
> > -}
> > -
> > -#ifdef USE_LZ4
> > -static inline void red_init_lz4(RedWorker *worker)
> > -{
> > -    worker->lz4_data.usr.more_space = lz4_usr_more_space;
> > -    worker->lz4_data.usr.more_lines = lz4_usr_more_lines;
> > -
> > -    worker->lz4 = lz4_encoder_create(&worker->lz4_data.usr);
> > -
> > -    if (!worker->lz4) {
> > -        spice_critical("create lz4 encoder failed");
> > -    }
> > -}
> > -#endif
> > -
> > -static inline void red_init_zlib(RedWorker *worker)
> > -{
> > -    worker->zlib_data.usr.more_space = zlib_usr_more_space;
> > -    worker->zlib_data.usr.more_input = zlib_usr_more_input;
> > -
> > -    worker->zlib = zlib_encoder_create(&worker->zlib_data.usr,
> > ZLIB_DEFAULT_COMPRESSION_LEVEL);
> > -
> > -    if (!worker->zlib) {
> > -        spice_critical("create zlib encoder failed");
> > -    }
> > -}
> > -
> >  typedef struct compress_send_data_t {
> >      void*    comp_buf;
> >      uint32_t comp_buf_size;
> > @@ -2809,7 +2348,6 @@ static inline int
> > red_glz_compress_image(DisplayChannelClient *dcc,
> >                                           compress_send_data_t*
> >                                           o_comp_data)
> >  {
> >      DisplayChannel *display_channel = DCC_TO_DC(dcc);
> > -    RedWorker *worker = display_channel->common.worker;
> >  #ifdef COMPRESS_STAT
> >      stat_time_t start_time =
> >      stat_now(display_channel->zlib_glz_stat.clock);
> >  #endif
> > @@ -2824,12 +2362,6 @@ static inline int
> > red_glz_compress_image(DisplayChannelClient *dcc,
> >
> >      glz_data->data.bufs_tail = compress_buf_new();
> >      glz_data->data.bufs_head = glz_data->data.bufs_tail;
> > -
> > -    if (!glz_data->data.bufs_head) {
> > -        return FALSE;
> > -    }
> > -
> > -    glz_data->data.bufs_head->send_next = NULL;
> >      glz_data->data.dcc = dcc;
> >
> >      glz_drawable = red_display_get_glz_drawable(dcc, drawable);
> > @@ -2839,7 +2371,6 @@ static inline int
> > red_glz_compress_image(DisplayChannelClient *dcc,
> >      glz_data->data.u.lines_data.stride = src->stride;
> >      glz_data->data.u.lines_data.next = 0;
> >      glz_data->data.u.lines_data.reverse = 0;
> > -    glz_data->usr.more_lines = glz_usr_more_lines;
> >
> >      glz_size = glz_encode(dcc->glz, type, src->x, src->y,
> >                            (src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN),
> >                            NULL, 0,
> > @@ -2856,23 +2387,16 @@ static inline int
> > red_glz_compress_image(DisplayChannelClient *dcc,
> >  #ifdef COMPRESS_STAT
> >      start_time = stat_now(display_channel->zlib_glz_stat.clock);
> >  #endif
> > -    zlib_data = &worker->zlib_data;
> > +    zlib_data = &dcc->zlib_data;
> >
> >      zlib_data->data.bufs_tail = compress_buf_new();
> >      zlib_data->data.bufs_head = zlib_data->data.bufs_tail;
> > -
> > -    if (!zlib_data->data.bufs_head) {
> > -        spice_warning("failed to allocate zlib compress buffer");
> > -        goto glz;
> > -    }
> > -
> > -    zlib_data->data.bufs_head->send_next = NULL;
> >      zlib_data->data.dcc = dcc;
> >
> >      zlib_data->data.u.compressed_data.next = glz_data->data.bufs_head;
> >      zlib_data->data.u.compressed_data.size_left = glz_size;
> >
> > -    zlib_size = zlib_encode(worker->zlib, display_channel->zlib_level,
> > +    zlib_size = zlib_encode(dcc->zlib, dcc->zlib_level,
> >                              glz_size,
> >                              zlib_data->data.bufs_head->buf.bytes,
> >                              sizeof(zlib_data->data.bufs_head->buf));
> >
> > @@ -2909,10 +2433,8 @@ static inline int
> > red_lz_compress_image(DisplayChannelClient *dcc,
> >                                          SpiceImage *dest, SpiceBitmap
> >                                          *src,
> >                                          compress_send_data_t* o_comp_data,
> >                                          uint32_t group_id)
> >  {
> > -    DisplayChannel *display_channel = DCC_TO_DC(dcc);
> > -    RedWorker *worker = display_channel->common.worker;
> > -    LzData *lz_data = &worker->lz_data;
> > -    LzContext *lz = worker->lz;
> > +    LzData *lz_data = &dcc->lz_data;
> > +    LzContext *lz = dcc->lz;
> >      LzImageType type = MAP_BITMAP_FMT_TO_LZ_IMAGE_TYPE[src->format];
> >      int size;            // size of the compressed data
> >
> > @@ -2922,12 +2444,6 @@ static inline int
> > red_lz_compress_image(DisplayChannelClient *dcc,
> >
> >      lz_data->data.bufs_tail = compress_buf_new();
> >      lz_data->data.bufs_head = lz_data->data.bufs_tail;
> > -
> > -    if (!lz_data->data.bufs_head) {
> > -        return FALSE;
> > -    }
> > -
> > -    lz_data->data.bufs_head->send_next = NULL;
> >      lz_data->data.dcc = dcc;
> >
> >      if (setjmp(lz_data->data.jmp_env)) {
> > @@ -2943,7 +2459,6 @@ static inline int
> > red_lz_compress_image(DisplayChannelClient *dcc,
> >      lz_data->data.u.lines_data.stride = src->stride;
> >      lz_data->data.u.lines_data.next = 0;
> >      lz_data->data.u.lines_data.reverse = 0;
> > -    lz_data->usr.more_lines = lz_usr_more_lines;
> >
> >      size = lz_encode(lz, type, src->x, src->y,
> >                       !!(src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN),
> > @@ -2987,12 +2502,10 @@ static int
> > red_jpeg_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
> >                                     SpiceBitmap *src, compress_send_data_t*
> >                                     o_comp_data,
> >                                     uint32_t group_id)
> >  {
> > -    DisplayChannel *display_channel = DCC_TO_DC(dcc);
> > -    RedWorker *worker = display_channel->common.worker;
> > -    JpegData *jpeg_data = &worker->jpeg_data;
> > -    LzData *lz_data = &worker->lz_data;
> > -    JpegEncoderContext *jpeg = worker->jpeg;
> > -    LzContext *lz = worker->lz;
> > +    JpegData *jpeg_data = &dcc->jpeg_data;
> > +    LzData *lz_data = &dcc->lz_data;
> > +    JpegEncoderContext *jpeg = dcc->jpeg;
> > +    LzContext *lz = dcc->lz;
> >      volatile JpegEncoderImageType jpeg_in_type;
> >      int jpeg_size = 0;
> >      volatile int has_alpha = FALSE;
> > @@ -3025,13 +2538,6 @@ static int
> > red_jpeg_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
> >
> >      jpeg_data->data.bufs_tail = compress_buf_new();
> >      jpeg_data->data.bufs_head = jpeg_data->data.bufs_tail;
> > -
> > -    if (!jpeg_data->data.bufs_head) {
> > -        spice_warning("failed to allocate compress buffer");
> > -        return FALSE;
> > -    }
> > -
> > -    jpeg_data->data.bufs_head->send_next = NULL;
> >      jpeg_data->data.dcc = dcc;
> >
> >      if (setjmp(jpeg_data->data.jmp_env)) {
> > @@ -3049,7 +2555,6 @@ static int
> > red_jpeg_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
> >
> >      jpeg_data->data.u.lines_data.chunks = src->data;
> >      jpeg_data->data.u.lines_data.stride = src->stride;
> > -    jpeg_data->usr.more_lines = jpeg_usr_more_lines;
> >      if ((src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN)) {
> >          jpeg_data->data.u.lines_data.next = 0;
> >          jpeg_data->data.u.lines_data.reverse = 0;
> > @@ -3059,7 +2564,7 @@ static int
> > red_jpeg_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
> >          jpeg_data->data.u.lines_data.reverse = 1;
> >          stride = -src->stride;
> >      }
> > -    jpeg_size = jpeg_encode(jpeg, display_channel->jpeg_quality,
> > jpeg_in_type,
> > +    jpeg_size = jpeg_encode(jpeg, dcc->jpeg_quality, jpeg_in_type,
> >                              src->x, src->y, NULL,
> >                              0, stride,
> >                              jpeg_data->data.bufs_head->buf.bytes,
> >                              sizeof(jpeg_data->data.bufs_head->buf));
> > @@ -3095,7 +2600,6 @@ static int
> > red_jpeg_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
> >      lz_data->data.u.lines_data.stride = src->stride;
> >      lz_data->data.u.lines_data.next = 0;
> >      lz_data->data.u.lines_data.reverse = 0;
> > -    lz_data->usr.more_lines = lz_usr_more_lines;
> >
> >      alpha_lz_size = lz_encode(lz, LZ_IMAGE_TYPE_XXXA, src->x, src->y,
> >                                 !!(src->flags &
> >                                 SPICE_BITMAP_FLAGS_TOP_DOWN),
> > @@ -3130,14 +2634,12 @@ static int
> > red_lz4_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
> >                                    SpiceBitmap *src, compress_send_data_t*
> >                                    o_comp_data,
> >                                    uint32_t group_id)
> >  {
> > -    DisplayChannel *display_channel = DCC_TO_DC(dcc);
> > -    RedWorker *worker = display_channel->common.worker;
> > -    Lz4Data *lz4_data = &worker->lz4_data;
> > -    Lz4EncoderContext *lz4 = worker->lz4;
> > +    Lz4Data *lz4_data = &dcc->lz4_data;
> > +    Lz4EncoderContext *lz4 = dcc->lz4;
> >      int lz4_size = 0;
> >
> >  #ifdef COMPRESS_STAT
> > -    stat_time_t start_time = stat_now(worker->clockid);
> > +    stat_time_t start_time = stat_now(DCC_TO_DC(dcc)->lz4_stat.clock);
> >  #endif
> >
> >      lz4_data->data.bufs_tail = compress_buf_new();
> > @@ -3168,7 +2670,7 @@ static int
> > red_lz4_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
> >      lz4_data->data.u.lines_data.stride = src->stride;
> >      lz4_data->data.u.lines_data.next = 0;
> >      lz4_data->data.u.lines_data.reverse = 0;
> > -    lz4_data->usr.more_lines = lz4_usr_more_lines;
> > +    /* fixme remove? lz4_data->usr.more_lines = lz4_usr_more_lines; */
> >
> >      lz4_size = lz4_encode(lz4, src->y, src->stride,
> >      lz4_data->data.bufs_head->buf.bytes,
> >                            sizeof(lz4_data->data.bufs_head->buf),
> > @@ -3185,7 +2687,7 @@ static int
> > red_lz4_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
> >      o_comp_data->comp_buf = lz4_data->data.bufs_head;
> >      o_comp_data->comp_buf_size = lz4_size;
> >
> > -    stat_compress_add(&display_channel->lz4_stat, start_time, src->stride
> > * src->y,
> > +    stat_compress_add(&DCC_TO_DC(dcc)->lz4_stat, start_time, src->stride *
> > src->y,
> >                        o_comp_data->comp_buf_size);
> >      return TRUE;
> >  }
> > @@ -3195,10 +2697,8 @@ static inline int
> > red_quic_compress_image(DisplayChannelClient *dcc, SpiceImage
> >                                            SpiceBitmap *src,
> >                                            compress_send_data_t*
> >                                            o_comp_data,
> >                                            uint32_t group_id)
> >  {
> > -    DisplayChannel *display_channel = DCC_TO_DC(dcc);
> > -    RedWorker *worker = display_channel->common.worker;
> > -    QuicData *quic_data = &worker->quic_data;
> > -    QuicContext *quic = worker->quic;
> > +    QuicData *quic_data = &dcc->quic_data;
> > +    QuicContext *quic = dcc->quic;
> >      volatile QuicImageType type;
> >      int size, stride;
> >
> > @@ -3225,7 +2725,6 @@ static inline int
> > red_quic_compress_image(DisplayChannelClient *dcc, SpiceImage
> >
> >      quic_data->data.bufs_tail = compress_buf_new();
> >      quic_data->data.bufs_head = quic_data->data.bufs_tail;
> > -    quic_data->data.bufs_head->send_next = NULL;
> >      quic_data->data.dcc = dcc;
> >
> >      if (setjmp(quic_data->data.jmp_env)) {
> > @@ -3243,7 +2742,6 @@ static inline int
> > red_quic_compress_image(DisplayChannelClient *dcc, SpiceImage
> >
> >      quic_data->data.u.lines_data.chunks = src->data;
> >      quic_data->data.u.lines_data.stride = src->stride;
> > -    quic_data->usr.more_lines = quic_usr_more_lines;
> >      if ((src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN)) {
> >          quic_data->data.u.lines_data.next = 0;
> >          quic_data->data.u.lines_data.reverse = 0;
> > @@ -7227,8 +6725,6 @@ static void handle_new_display_channel(RedWorker
> > *worker, RedClient *client, Red
> >      stream_buf_size = 32*1024;
> >      dcc->send_data.stream_outbuf = spice_malloc(stream_buf_size);
> >      dcc->send_data.stream_outbuf_size = stream_buf_size;
> > -    red_display_init_glz_data(dcc);
> > -
> >      dcc->send_data.free_list.res =
> >          spice_malloc(sizeof(SpiceResourceList) +
> >                       DISPLAY_FREE_LIST_DEFAULT_SIZE *
> >                       sizeof(SpiceResourceID));
> > @@ -7240,9 +6736,6 @@ static void handle_new_display_channel(RedWorker
> > *worker, RedClient *client, Red
> >          display_channel->enable_jpeg = (dcc->jpeg_state ==
> >          SPICE_WAN_COMPRESSION_ALWAYS);
> >      }
> >
> > -    // todo: tune quality according to bandwidth
> > -    display_channel->jpeg_quality = 85;
> > -
> >      if (dcc->zlib_glz_state == SPICE_WAN_COMPRESSION_AUTO) {
> >          display_channel->enable_zlib_glz_wrap =
> >          dcc->common.is_low_bandwidth;
> >      } else {
> > @@ -7255,8 +6748,6 @@ static void handle_new_display_channel(RedWorker
> > *worker, RedClient *client, Red
> >
> >      guest_set_client_capabilities(worker);
> >
> > -    // todo: tune level according to bandwidth
> > -    display_channel->zlib_level = ZLIB_DEFAULT_COMPRESSION_LEVEL;
> >      dcc_init_stream_agents(dcc);
> >      on_new_display_channel_client(dcc);
> >  }
> > @@ -8279,13 +7770,6 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> > RedDispatcher *red_dispatcher)
> >
> >      spice_warn_if(init_info.n_surfaces > NUM_SURFACES);
> >
> > -    red_init_quic(worker);
> > -    red_init_lz(worker);
> > -    red_init_jpeg(worker);
> > -#ifdef USE_LZ4
> > -    red_init_lz4(worker);
> > -#endif
> > -    red_init_zlib(worker);
> >      worker->event_timeout = INF_EVENT_WAIT;
> >
> >      worker->cursor_channel = cursor_channel_new(worker);
> > --
> > 2.4.3
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> Seems okay. ACK.
> Note: The comments about using macros are a suggestion not a must be
> ... feel free to ignore them.
> 

I think the additional macro would be better in another patch as this is just supposed
to move stuff.

Merged.

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]