(I see here we add a non-stats field to ImageEncoderGlobals. So per my earlier
suggestion, i'd prefer a name of something like ImageEncoderSharedData.)
Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
On Fri, 2016-06-10 at 09:48 +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> server/dcc-encoders.c | 76 +++++++++++++++++++++++++--------------------
> ---
> server/dcc-encoders.h | 19 +++++++-----
> server/dcc.c | 18 +++++-------
> server/dcc.h | 5 ----
> server/display-channel.c | 12 ++++----
> server/display-channel.h | 2 --
> server/red-worker.c | 4 +--
> 7 files changed, 66 insertions(+), 70 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 94b07fe..656ce25 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -26,8 +26,8 @@
>
> #define ZLIB_DEFAULT_COMPRESSION_LEVEL 3
>
> -static void dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
> - GlzDrawableInstanceItem *item);
> +static void image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
> + GlzDrawableInstanceItem
> *instance);
>
> static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
> quic_usr_error(QuicUsrContext *usr, const char *fmt, ...)
> @@ -329,10 +329,10 @@ static void glz_usr_free_image(GlzEncoderUsrContext
> *usr, GlzUsrImageContext *im
> {
> GlzData *lz_data = (GlzData *)usr;
> GlzDrawableInstanceItem *glz_drawable_instance = (GlzDrawableInstanceItem
> *)image;
> - DisplayChannelClient *drawable_cc = glz_drawable_instance->glz_drawable-
> >dcc;
> - DisplayChannelClient *this_cc = SPICE_CONTAINEROF(lz_data,
> DisplayChannelClient, encoders.glz_data);
> - if (this_cc == drawable_cc) {
> - dcc_free_glz_drawable_instance(drawable_cc, glz_drawable_instance);
> + ImageEncoders *drawable_enc = glz_drawable_instance->glz_drawable-
> >encoders;
> + ImageEncoders *this_enc = SPICE_CONTAINEROF(lz_data, ImageEncoders,
> glz_data);
> + if (this_enc == drawable_enc) {
> + image_encoders_free_glz_drawable_instance(drawable_enc,
> glz_drawable_instance);
> } else {
> /* The glz dictionary is shared between all DisplayChannelClient
> * instances that belong to the same client, and glz_usr_free_image
> @@ -341,10 +341,10 @@ static void glz_usr_free_image(GlzEncoderUsrContext
> *usr, GlzUsrImageContext *im
> * called from any DisplayChannelClient thread, hence the need for
> * this check.
> */
> - pthread_mutex_lock(&drawable_cc->glz_drawables_inst_to_free_lock);
> + pthread_mutex_lock(&drawable_enc->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);
> + &drawable_enc->glz_drawables_inst_to_free);
> + pthread_mutex_unlock(&drawable_enc->glz_drawables_inst_to_free_lock);
> }
> }
>
> @@ -403,6 +403,10 @@ void image_encoders_init(ImageEncoders *enc,
> ImageEncoderGlobals *globals)
> spice_assert(globals);
> enc->globals = globals;
>
> + ring_init(&enc->glz_drawables);
> + ring_init(&enc->glz_drawables_inst_to_free);
> + pthread_mutex_init(&enc->glz_drawables_inst_to_free_lock, NULL);
> +
> image_encoders_init_glz_data(enc);
> image_encoders_init_quic(enc);
> image_encoders_init_lz(enc);
> @@ -437,10 +441,9 @@ void image_encoders_free(ImageEncoders *enc)
> it is not used by Drawable).
> NOTE - 1) can be called only by the display channel that created the
> drawable
> 2) it is assumed that the instance was already removed from the
> dictionary*/
> -static void dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
> - GlzDrawableInstanceItem *instance)
> +static void image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
> + GlzDrawableInstanceItem
> *instance)
> {
> - DisplayChannel *display_channel = DCC_TO_DC(dcc);
> RedGlzDrawable *glz_drawable;
>
> spice_assert(instance);
> @@ -448,7 +451,7 @@ static void
> dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
>
> glz_drawable = instance->glz_drawable;
>
> - spice_assert(glz_drawable->dcc == dcc);
> + spice_assert(glz_drawable->encoders == enc);
> spice_assert(glz_drawable->instances_count > 0);
>
> ring_remove(&instance->glz_link);
> @@ -469,7 +472,7 @@ static void
> dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
> ring_remove(&glz_drawable->drawable_link);
> }
> red_drawable_unref(glz_drawable->red_drawable);
> - display_channel->glz_drawable_count--;
> + enc->globals->glz_drawable_count--;
> if (ring_item_is_linked(&glz_drawable->link)) {
> ring_remove(&glz_drawable->link);
> }
> @@ -483,14 +486,14 @@ static void
> dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
> * if possible.
> * NOTE - the caller should prevent encoding using the dictionary during this
> operation
> */
> -void dcc_free_glz_drawable(DisplayChannelClient *dcc, RedGlzDrawable
> *drawable)
> +void image_encoders_free_glz_drawable(ImageEncoders *enc, RedGlzDrawable
> *drawable)
> {
> RingItem *head_instance = ring_get_head(&drawable->instances);
> int cont = (head_instance != NULL);
>
> while (cont) {
> if (drawable->instances_count == 1) {
> - /* Last instance: dcc_free_glz_drawable_instance will free the
> drawable */
> + /* Last instance: image_encoders_free_glz_drawable_instance will
> free the drawable */
> cont = FALSE;
> }
> GlzDrawableInstanceItem *instance = SPICE_CONTAINEROF(head_instance,
> @@ -498,11 +501,11 @@ void dcc_free_glz_drawable(DisplayChannelClient *dcc,
> RedGlzDrawable *drawable)
> glz_link);
> if (!ring_item_is_linked(&instance->free_link)) {
> // the instance didn't get out from window yet
> - glz_enc_dictionary_remove_image(dcc->encoders.glz_dict->dict,
> + glz_enc_dictionary_remove_image(enc->glz_dict->dict,
> instance->context,
> - &dcc->encoders.glz_data.usr);
> + &enc->glz_data.usr);
> }
> - dcc_free_glz_drawable_instance(dcc, instance);
> + image_encoders_free_glz_drawable_instance(enc, instance);
>
> if (cont) {
> head_instance = ring_get_head(&drawable->instances);
> @@ -515,49 +518,49 @@ void dcc_free_glz_drawable(DisplayChannelClient *dcc,
> RedGlzDrawable *drawable)
> * Drawable (their qxl drawables are released too).
> * NOTE - the caller should prevent encoding using the dictionary during the
> operation
> */
> -int dcc_free_some_independent_glz_drawables(DisplayChannelClient *dcc)
> +int image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc)
> {
> RingItem *ring_link;
> int n = 0;
>
> - if (!dcc) {
> + if (!enc) {
> return 0;
> }
> - ring_link = ring_get_head(&dcc->glz_drawables);
> + ring_link = ring_get_head(&enc->glz_drawables);
> while ((n < RED_RELEASE_BUNCH_SIZE) && (ring_link != NULL)) {
> RedGlzDrawable *glz_drawable = SPICE_CONTAINEROF(ring_link,
> RedGlzDrawable, link);
> - ring_link = ring_next(&dcc->glz_drawables, ring_link);
> + ring_link = ring_next(&enc->glz_drawables, ring_link);
> if (!glz_drawable->drawable) {
> - dcc_free_glz_drawable(dcc, glz_drawable);
> + image_encoders_free_glz_drawable(enc, glz_drawable);
> n++;
> }
> }
> return n;
> }
>
> -void dcc_free_glz_drawables_to_free(DisplayChannelClient* dcc)
> +void image_encoders_free_glz_drawables_to_free(ImageEncoders* enc)
> {
> RingItem *ring_link;
>
> - if (!dcc->encoders.glz_dict) {
> + if (!enc->glz_dict) {
> return;
> }
> - pthread_mutex_lock(&dcc->glz_drawables_inst_to_free_lock);
> - while ((ring_link = ring_get_head(&dcc->glz_drawables_inst_to_free))) {
> + pthread_mutex_lock(&enc->glz_drawables_inst_to_free_lock);
> + while ((ring_link = ring_get_head(&enc->glz_drawables_inst_to_free))) {
> GlzDrawableInstanceItem *drawable_instance =
> SPICE_CONTAINEROF(ring_link,
> GlzDrawableI
> nstanceItem,
> free_link);
> - dcc_free_glz_drawable_instance(dcc, drawable_instance);
> + image_encoders_free_glz_drawable_instance(enc, drawable_instance);
> }
> - pthread_mutex_unlock(&dcc->glz_drawables_inst_to_free_lock);
> + pthread_mutex_unlock(&enc->glz_drawables_inst_to_free_lock);
> }
>
> /* Clear all lz drawables - enforce their removal from the global dictionary.
> NOTE - prevents encoding using the dictionary during the operation*/
> -void dcc_free_glz_drawables(DisplayChannelClient *dcc)
> +void image_encoders_free_glz_drawables(ImageEncoders *enc)
> {
> RingItem *ring_link;
> - GlzSharedDictionary *glz_dict = dcc ? dcc->encoders.glz_dict : NULL;
> + GlzSharedDictionary *glz_dict = enc ? enc->glz_dict : NULL;
>
> if (!glz_dict) {
> return;
> @@ -565,11 +568,11 @@ void dcc_free_glz_drawables(DisplayChannelClient *dcc)
>
> // assure no display channel is during global lz encoding
> pthread_rwlock_wrlock(&glz_dict->encode_lock);
> - while ((ring_link = ring_get_head(&dcc->glz_drawables))) {
> + while ((ring_link = ring_get_head(&enc->glz_drawables))) {
> RedGlzDrawable *drawable = SPICE_CONTAINEROF(ring_link,
> RedGlzDrawable, link);
> // no need to lock the to_free list, since we assured no other thread
> is encoding and
> // thus not other thread access the to_free list of the channel
> - dcc_free_glz_drawable(dcc, drawable);
> + image_encoders_free_glz_drawable(enc, drawable);
> }
> pthread_rwlock_unlock(&glz_dict->encode_lock);
> }
> @@ -695,12 +698,11 @@ gboolean image_encoders_glz_create(ImageEncoders *enc,
> uint8_t id)
> }
>
> /* destroy encoder, and dictionary if no one uses it*/
> -void dcc_release_glz(DisplayChannelClient *dcc)
> +void image_encoders_release_glz(ImageEncoders *enc)
> {
> - ImageEncoders *enc = &dcc->encoders;
> GlzSharedDictionary *shared_dict;
>
> - dcc_free_glz_drawables(dcc);
> + image_encoders_free_glz_drawables(enc);
>
> glz_encoder_destroy(enc->glz);
> enc->glz = NULL;
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index 51f0cfe..b32b34b 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -43,14 +43,13 @@ void image_encoder_globals_stat_print(const
> ImageEncoderGlobals *globals);
>
> void image_encoders_init(ImageEncoders *enc, ImageEncoderGlobals *globals);
> void image_encoders_free(ImageEncoders *enc);
> -void dcc_free_glz_drawable (DisplayChannelC
> lient *dcc,
> - RedGlzDrawable
> *drawable);
> -int dcc_free_some_independent_glz_drawables (DisplayChannelC
> lient *dcc);
> -void dcc_free_glz_drawables (DisplayChannelC
> lient *dcc);
> -void dcc_free_glz_drawables_to_free (DisplayChannelC
> lient* dcc);
> +void image_encoders_free_glz_drawable(ImageEncoders *enc, RedGlzDrawable
> *drawable);
> +int image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc);
> +void image_encoders_free_glz_drawables(ImageEncoders *enc);
> +void image_encoders_free_glz_drawables_to_free(ImageEncoders* enc);
> gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id);
> void image_encoders_freeze_glz(ImageEncoders *enc);
> -void dcc_release_glz (DisplayChannelC
> lient *dcc);
> +void image_encoders_release_glz(ImageEncoders *enc);
>
> #define RED_COMPRESS_BUF_SIZE (1024 * 64)
> struct RedCompressBuf {
> @@ -166,10 +165,12 @@ struct RedGlzDrawable {
> GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES];
> Ring instances;
> uint8_t instances_count;
> - DisplayChannelClient *dcc;
> + ImageEncoders *encoders;
> };
>
> struct ImageEncoderGlobals {
> + uint32_t glz_drawable_count;
> +
> stat_info_t off_stat;
> stat_info_t lz_stat;
> stat_info_t glz_stat;
> @@ -208,6 +209,10 @@ struct ImageEncoders {
> GlzSharedDictionary *glz_dict;
> GlzEncoderContext *glz;
> GlzData glz_data;
> +
> + Ring glz_drawables; // all the living lz drawable, ordered
> by encoding time
> + Ring glz_drawables_inst_to_free; // list of instances to be
> freed
> + pthread_mutex_t glz_drawables_inst_to_free_lock;
> };
>
> typedef struct compress_send_data_t {
> diff --git a/server/dcc.c b/server/dcc.c
> index 1cc85b5..82862af 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -392,10 +392,6 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
>
> dcc_init_stream_agents(dcc);
>
> - ring_init(&dcc->glz_drawables);
> - ring_init(&dcc->glz_drawables_inst_to_free);
> - pthread_mutex_init(&dcc->glz_drawables_inst_to_free_lock, NULL);
> -
> image_encoders_init(&dcc->encoders, &display->encoder_globals);
>
> return dcc;
> @@ -493,7 +489,7 @@ void dcc_stop(DisplayChannelClient *dcc)
>
> pixmap_cache_unref(dcc->pixmap_cache);
> dcc->pixmap_cache = NULL;
> - dcc_release_glz(dcc);
> + image_encoders_release_glz(&dcc->encoders);
> dcc_palette_cache_reset(dcc);
> free(dcc->send_data.stream_outbuf);
> free(dcc->send_data.free_list.res);
> @@ -638,7 +634,7 @@ void dcc_destroy_surface(DisplayChannelClient *dcc,
> uint32_t surface_id)
>
> /* if already exists, returns it. Otherwise allocates and adds it (1) to the
> ring tail
> in the channel (2) to the Drawable*/
> -static RedGlzDrawable *get_glz_drawable(DisplayChannelClient *dcc, Drawable
> *drawable)
> +static RedGlzDrawable *get_glz_drawable(ImageEncoders *enc, Drawable
> *drawable)
> {
> RedGlzDrawable *ret;
> RingItem *item, *next;
> @@ -647,14 +643,14 @@ static RedGlzDrawable
> *get_glz_drawable(DisplayChannelClient *dcc, Drawable *dra
> // now that we have multiple glz_dicts, so the only way to go from dcc to
> drawable glz is to go
> // over the glz_ring (unless adding some better data structure then a
> ring)
> DRAWABLE_FOREACH_GLZ_SAFE(drawable, item, next, ret) {
> - if (ret->dcc == dcc) {
> + if (ret->encoders == enc) {
> return ret;
> }
> }
>
> ret = spice_new(RedGlzDrawable, 1);
>
> - ret->dcc = dcc;
> + ret->encoders = enc;
> ret->red_drawable = red_drawable_ref(drawable->red_drawable);
> ret->drawable = drawable;
> ret->instances_count = 0;
> @@ -662,9 +658,9 @@ static RedGlzDrawable
> *get_glz_drawable(DisplayChannelClient *dcc, Drawable *dra
>
> ring_item_init(&ret->link);
> ring_item_init(&ret->drawable_link);
> - ring_add_before(&ret->link, &dcc->glz_drawables);
> + ring_add_before(&ret->link, &enc->glz_drawables);
> ring_add(&drawable->glz_ring, &ret->drawable_link);
> - DCC_TO_DC(dcc)->glz_drawable_count++;
> + enc->globals->glz_drawable_count++;
> return ret;
> }
>
> @@ -724,7 +720,7 @@ static int dcc_compress_image_glz(DisplayChannelClient
> *dcc,
>
> encoder_data_init(&glz_data->data);
>
> - glz_drawable = get_glz_drawable(dcc, drawable);
> + glz_drawable = get_glz_drawable(&dcc->encoders, drawable);
> glz_drawable_instance = add_glz_drawable_instance(glz_drawable);
>
> glz_data->data.u.lines_data.chunks = src->data;
> diff --git a/server/dcc.h b/server/dcc.h
> index a4b917b..362237d 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -83,11 +83,6 @@ struct DisplayChannelClient {
> int num_pixmap_cache_items;
> } send_data;
>
> - /* global lz encoding entities */
> - Ring glz_drawables; // all the living lz drawable, ordered
> by encoding time
> - Ring glz_drawables_inst_to_free; // list of instances to be
> freed
> - pthread_mutex_t glz_drawables_inst_to_free_lock;
> -
> uint8_t surface_client_created[NUM_SURFACES];
> QRegion surface_client_lossy_region[NUM_SURFACES];
>
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 4aa912a..c48db00 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -1143,7 +1143,7 @@ void
> display_channel_free_glz_drawables_to_free(DisplayChannel *display)
> spice_return_if_fail(display);
>
> FOREACH_CLIENT(display, link, next, dcc) {
> - dcc_free_glz_drawables_to_free(dcc);
> + image_encoders_free_glz_drawables_to_free(&dcc->encoders);
> }
> }
>
> @@ -1155,7 +1155,7 @@ void display_channel_free_glz_drawables(DisplayChannel
> *display)
> spice_return_if_fail(display);
>
> FOREACH_CLIENT(display, link, next, dcc) {
> - dcc_free_glz_drawables(dcc);
> + image_encoders_free_glz_drawables(&dcc->encoders);
> }
> }
>
> @@ -1174,7 +1174,7 @@ static bool free_one_drawable(DisplayChannel *display,
> int force_glz_free)
> RingItem *glz_item, *next_item;
> RedGlzDrawable *glz;
> DRAWABLE_FOREACH_GLZ_SAFE(drawable, glz_item, next_item, glz) {
> - dcc_free_glz_drawable(glz->dcc, glz);
> + image_encoders_free_glz_drawable(glz->encoders, glz);
> }
> }
> drawable_draw(display, drawable);
> @@ -1200,7 +1200,7 @@ void display_channel_free_some(DisplayChannel *display)
> GList *link, *next;
>
> spice_debug("#draw=%d, #glz_draw=%d", display->drawable_count,
> - display->glz_drawable_count);
> + display->encoder_globals.glz_drawable_count);
> FOREACH_CLIENT(display, link, next, dcc) {
> GlzSharedDictionary *glz_dict = dcc->encoders.glz_dict;
>
> @@ -1208,7 +1208,7 @@ void display_channel_free_some(DisplayChannel *display)
> // encoding using the dictionary is prevented since the following
> operations might
> // change the dictionary
> pthread_rwlock_wrlock(&glz_dict->encode_lock);
> - n = dcc_free_some_independent_glz_drawables(dcc);
> + n = image_encoders_free_some_independent_glz_drawables(&dcc-
> >encoders);
> }
> }
>
> @@ -1853,7 +1853,7 @@ static void on_disconnect(RedChannelClient *rcc)
> // this was the last channel client
> spice_debug("#draw=%d, #glz_draw=%d",
> display->drawable_count,
> - display->glz_drawable_count);
> + display->encoder_globals.glz_drawable_count);
> }
>
> static int handle_migrate_flush_mark(RedChannelClient *rcc)
> diff --git a/server/display-channel.h b/server/display-channel.h
> index cdf1dce..8bf4d5d 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -183,8 +183,6 @@ struct DisplayChannel {
> _Drawable drawables[NUM_DRAWABLES];
> _Drawable *free_drawables;
>
> - uint32_t glz_drawable_count;
> -
> int stream_video;
> uint32_t stream_count;
> Stream streams_buf[NUM_STREAMS];
> diff --git a/server/red-worker.c b/server/red-worker.c
> index c53bc15..aba3e6f 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -823,7 +823,7 @@ static void handle_dev_oom(void *opaque, void *payload)
> // streams? but without streams also leak
> spice_debug("OOM1 #draw=%u, #glz_draw=%u current %u pipes %u",
> display->drawable_count,
> - display->glz_drawable_count,
> + display->encoder_globals.glz_drawable_count,
> display->current_size,
> red_channel_sum_pipes_size(display_red_channel));
> while (red_process_display(worker, &ring_is_empty)) {
> @@ -835,7 +835,7 @@ static void handle_dev_oom(void *opaque, void *payload)
> }
> spice_debug("OOM2 #draw=%u, #glz_draw=%u current %u pipes %u",
> display->drawable_count,
> - display->glz_drawable_count,
> + display->encoder_globals.glz_drawable_count,
> display->current_size,
> red_channel_sum_pipes_size(display_red_channel));
> red_qxl_clear_pending(worker->qxl->st, RED_DISPATCHER_PENDING_OOM);
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel