[Spice-devel] [PATCH v5 1/9] Encapsulate some data in dcc-encoders

Jonathon Jongsma jjongsma at redhat.com
Wed Jun 15 14:06:19 UTC 2016


Acked-by: Jonathon Jongsma <jjongsma at redhat.com>


On Wed, 2016-06-15 at 10:37 +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/dcc-encoders.c    | 59 ++++++++++++++++++++++++++++++++++++++++++++++-
> -
>  server/dcc-encoders.h    | 33 ++-------------------------
>  server/display-channel.c | 13 +++--------
>  server/display-channel.h |  5 ----
>  4 files changed, 62 insertions(+), 48 deletions(-)
> 
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 8c3acd0..2fa5f1b 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -26,7 +26,43 @@
>  
>  #define ZLIB_DEFAULT_COMPRESSION_LEVEL 3
>  
> +#define MAX_GLZ_DRAWABLE_INSTANCES 2
> +
> +typedef struct GlzDrawableInstanceItem GlzDrawableInstanceItem;
> +
> +/* 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 *context;
> +    RedGlzDrawable         *glz_drawable;
> +};
> +
> +struct RedGlzDrawable {
> +    RingItem link;    // ordered by the time it was encoded
> +    RingItem drawable_link;
> +    RedDrawable *red_drawable;
> +    GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES];
> +    Ring instances;
> +    uint8_t instances_count;
> +    gboolean has_drawable;
> +    ImageEncoders *encoders;
> +};
> +
> +#define LINK_TO_GLZ(ptr) SPICE_CONTAINEROF((ptr), RedGlzDrawable, \
> +                                           drawable_link)
> +#define DRAWABLE_FOREACH_GLZ_SAFE(drawable, link, next, glz) \
> +    SAFE_FOREACH(link, next, drawable, &(drawable)->glz_ring, glz,
> LINK_TO_GLZ(link))
> +
>  static void glz_drawable_instance_item_free(GlzDrawableInstanceItem
> *instance);
> +static void encoder_data_init(EncoderData *data);
> +static void encoder_data_reset(EncoderData *data);
>  static void image_encoders_release_glz(ImageEncoders *enc);
>  
>  
> @@ -140,14 +176,14 @@ static void glz_usr_free(GlzEncoderUsrContext *usr, void
> *ptr)
>      free(ptr);
>  }
>  
> -void encoder_data_init(EncoderData *data)
> +static void encoder_data_init(EncoderData *data)
>  {
>      data->bufs_tail = g_new(RedCompressBuf, 1);
>      data->bufs_head = data->bufs_tail;
>      data->bufs_head->send_next = NULL;
>  }
>  
> -void encoder_data_reset(EncoderData *data)
> +static void encoder_data_reset(EncoderData *data)
>  {
>      RedCompressBuf *buf = data->bufs_head;
>      while (buf) {
> @@ -576,6 +612,25 @@ void image_encoders_free_glz_drawables(ImageEncoders
> *enc)
>      pthread_rwlock_unlock(&glz_dict->encode_lock);
>  }
>  
> +void drawable_free_glz_drawables(struct Drawable *drawable)
> +{
> +    RingItem *glz_item, *next_item;
> +    RedGlzDrawable *glz;
> +    DRAWABLE_FOREACH_GLZ_SAFE(drawable, glz_item, next_item, glz) {
> +        red_glz_drawable_free(glz);
> +    }
> +}
> +
> +void drawable_detach_glz_drawables(struct Drawable *drawable)
> +{
> +    RingItem *item, *next;
> +
> +    RING_FOREACH_SAFE(item, next, &drawable->glz_ring) {
> +        SPICE_CONTAINEROF(item, RedGlzDrawable, drawable_link)->has_drawable
> = FALSE;
> +        ring_remove(item);
> +    }
> +}
> +
>  static void image_encoders_freeze_glz(ImageEncoders *enc)
>  {
>      pthread_rwlock_wrlock(&enc->glz_dict->encode_lock);
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index 815356a..ad0a7ca 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -32,7 +32,6 @@
>  #include "zlib-encoder.h"
>  
>  typedef struct RedCompressBuf RedCompressBuf;
> -typedef struct GlzDrawableInstanceItem GlzDrawableInstanceItem;
>  typedef struct RedGlzDrawable RedGlzDrawable;
>  typedef struct ImageEncoders ImageEncoders;
>  typedef struct ImageEncoderSharedData ImageEncoderSharedData;
> @@ -50,6 +49,8 @@ void
> image_encoders_free_glz_drawables_to_free(ImageEncoders* enc);
>  gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id);
>  void image_encoders_glz_get_restore_data(ImageEncoders *enc,
>                                           uint8_t *out_id,
> GlzEncDictRestoreData *out_data);
> +void drawable_free_glz_drawables(struct Drawable *drawable);
> +void drawable_detach_glz_drawables(struct Drawable *drawable);
>  
>  #define RED_COMPRESS_BUF_SIZE (1024 * 64)
>  struct RedCompressBuf {
> @@ -106,9 +107,6 @@ typedef struct  {
>      char message_buf[512];
>  } EncoderData;
>  
> -void encoder_data_init(EncoderData *data);
> -void encoder_data_reset(EncoderData *data);
> -
>  typedef struct {
>      QuicUsrContext usr;
>      EncoderData data;
> @@ -141,33 +139,6 @@ typedef struct {
>      EncoderData data;
>  } GlzData;
>  
> -#define MAX_GLZ_DRAWABLE_INSTANCES 2
> -
> -/* 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 *context;
> -    RedGlzDrawable         *glz_drawable;
> -};
> -
> -struct RedGlzDrawable {
> -    RingItem link;    // ordered by the time it was encoded
> -    RingItem drawable_link;
> -    RedDrawable *red_drawable;
> -    GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES];
> -    Ring instances;
> -    uint8_t instances_count;
> -    gboolean has_drawable;
> -    ImageEncoders *encoders;
> -};
> -
>  struct ImageEncoderSharedData {
>      uint32_t glz_drawable_count;
>  
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 1bc9d57..fae2e25 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -1179,11 +1179,7 @@ static bool free_one_drawable(DisplayChannel *display,
> int force_glz_free)
>  
>      drawable = SPICE_CONTAINEROF(ring_item, Drawable, list_link);
>      if (force_glz_free) {
> -        RingItem *glz_item, *next_item;
> -        RedGlzDrawable *glz;
> -        DRAWABLE_FOREACH_GLZ_SAFE(drawable, glz_item, next_item, glz) {
> -            red_glz_drawable_free(glz);
> -        }
> +        drawable_free_glz_drawables(drawable);
>      }
>      drawable_draw(display, drawable);
>      container = drawable->tree_item.base.container;
> @@ -1336,7 +1332,6 @@ static void drawable_unref_surface_deps(DisplayChannel
> *display, Drawable *drawa
>  void drawable_unref(Drawable *drawable)
>  {
>      DisplayChannel *display = drawable->display;
> -    RingItem *item, *next;
>  
>      if (--drawable->refs != 0)
>          return;
> @@ -1353,10 +1348,8 @@ void drawable_unref(Drawable *drawable)
>      drawable_unref_surface_deps(display, drawable);
>      display_channel_surface_unref(display, drawable->surface_id);
>  
> -    RING_FOREACH_SAFE(item, next, &drawable->glz_ring) {
> -        SPICE_CONTAINEROF(item, RedGlzDrawable, drawable_link)->has_drawable
> = FALSE;
> -        ring_remove(item);
> -    }
> +    drawable_detach_glz_drawables(drawable);
> +
>      if (drawable->red_drawable) {
>          red_drawable_unref(drawable->red_drawable);
>      }
> diff --git a/server/display-channel.h b/server/display-channel.h
> index 7984c85..04ae0d0 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -87,11 +87,6 @@ void drawable_unref (Drawable *drawable);
>  #define DRAWABLE_FOREACH_DPI_SAFE(drawable, link, next, dpi)            \
>      SAFE_FOREACH(link, next, drawable,  &(drawable)->pipes, dpi,
> LINK_TO_DPI(link))
>  
> -#define LINK_TO_GLZ(ptr) SPICE_CONTAINEROF((ptr), RedGlzDrawable, \
> -                                           drawable_link)
> -#define DRAWABLE_FOREACH_GLZ_SAFE(drawable, link, next, glz) \
> -    SAFE_FOREACH(link, next, drawable, &(drawable)->glz_ring, glz,
> LINK_TO_GLZ(link))
> -
>  enum {
>      RED_PIPE_ITEM_TYPE_DRAW = RED_PIPE_ITEM_TYPE_COMMON_LAST,
>      RED_PIPE_ITEM_TYPE_IMAGE,


More information about the Spice-devel mailing list