[Spice-devel] [PATCH v3 02/17] Move some glz fields to ImageEncoders
Jonathon Jongsma
jjongsma at redhat.com
Mon Jun 13 21:59:34 UTC 2016
On Fri, 2016-06-10 at 09:48 +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
> server/dcc-encoders.c | 99 +++++++++++++++++++++++++++------------------
> ---
> server/dcc-encoders.h | 22 +++++++----
> server/dcc-send.c | 10 ++---
> server/dcc.c | 45 +++++++++++-----------
> server/dcc.h | 4 --
> server/display-channel.c | 4 +-
> 6 files changed, 100 insertions(+), 84 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 92dc721..94b07fe 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -330,7 +330,7 @@ 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, glz_data);
> + 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);
> } else {
> @@ -348,16 +348,16 @@ static void glz_usr_free_image(GlzEncoderUsrContext
> *usr, GlzUsrImageContext *im
> }
> }
>
> -static void dcc_init_glz_data(DisplayChannelClient *dcc)
> +static void image_encoders_init_glz_data(ImageEncoders *enc)
> {
> - 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;
> + enc->glz_data.usr.error = glz_usr_error;
> + enc->glz_data.usr.warn = glz_usr_warn;
> + enc->glz_data.usr.info = glz_usr_warn;
> + enc->glz_data.usr.malloc = glz_usr_malloc;
> + enc->glz_data.usr.free = glz_usr_free;
> + enc->glz_data.usr.more_space = glz_usr_more_space;
> + enc->glz_data.usr.more_lines = glz_usr_more_lines;
> + enc->glz_data.usr.free_image = glz_usr_free_image;
> }
>
> static void image_encoders_init_jpeg(ImageEncoders *enc)
> @@ -398,14 +398,12 @@ static void image_encoders_init_zlib(ImageEncoders *enc)
> }
> }
>
> -void dcc_encoders_init(DisplayChannelClient *dcc, ImageEncoderGlobals
> *globals)
> +void image_encoders_init(ImageEncoders *enc, ImageEncoderGlobals *globals)
This particular change doesn't necessarily belong with this patch.
> {
> - ImageEncoders *enc = &dcc->encoders;
> -
> spice_assert(globals);
> enc->globals = globals;
>
> - dcc_init_glz_data(dcc);
> + image_encoders_init_glz_data(enc);
> image_encoders_init_quic(enc);
> image_encoders_init_lz(enc);
> image_encoders_init_jpeg(enc);
> @@ -500,9 +498,9 @@ 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->glz_dict->dict,
> + glz_enc_dictionary_remove_image(dcc->encoders.glz_dict->dict,
> instance->context,
> - &dcc->glz_data.usr);
> + &dcc->encoders.glz_data.usr);
> }
> dcc_free_glz_drawable_instance(dcc, instance);
>
> @@ -541,7 +539,7 @@ void dcc_free_glz_drawables_to_free(DisplayChannelClient*
> dcc)
> {
> RingItem *ring_link;
>
> - if (!dcc->glz_dict) {
> + if (!dcc->encoders.glz_dict) {
> return;
> }
> pthread_mutex_lock(&dcc->glz_drawables_inst_to_free_lock);
> @@ -559,7 +557,7 @@ void dcc_free_glz_drawables_to_free(DisplayChannelClient*
> dcc)
> void dcc_free_glz_drawables(DisplayChannelClient *dcc)
> {
> RingItem *ring_link;
> - GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
> + GlzSharedDictionary *glz_dict = dcc ? dcc->encoders.glz_dict : NULL;
>
> if (!glz_dict) {
> return;
> @@ -576,11 +574,11 @@ void dcc_free_glz_drawables(DisplayChannelClient *dcc)
> pthread_rwlock_unlock(&glz_dict->encode_lock);
> }
>
> -void dcc_freeze_glz(DisplayChannelClient *dcc)
> +void image_encoders_freeze_glz(ImageEncoders *enc)
> {
> - pthread_rwlock_wrlock(&dcc->glz_dict->encode_lock);
> - dcc->glz_dict->migrate_freeze = TRUE;
> - pthread_rwlock_unlock(&dcc->glz_dict->encode_lock);
> + pthread_rwlock_wrlock(&enc->glz_dict->encode_lock);
> + enc->glz_dict->migrate_freeze = TRUE;
> + pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
> }
>
> static GlzSharedDictionary *glz_shared_dictionary_new(RedClient *client,
> uint8_t id,
> @@ -623,82 +621,95 @@ static GlzSharedDictionary
> *find_glz_dictionary(RedClient *client, uint8_t dict_
>
> #define MAX_LZ_ENCODERS MAX_CACHE_CLIENTS
>
> -static GlzSharedDictionary *create_glz_dictionary(DisplayChannelClient *dcc,
> +static GlzSharedDictionary *create_glz_dictionary(ImageEncoders *enc,
> + RedClient *client,
> uint8_t id, int
> window_size)
> {
> spice_info("Lz Window %d Size=%d", id, window_size);
>
> GlzEncDictContext *glz_dict =
> - glz_enc_dictionary_create(window_size, MAX_LZ_ENCODERS, &dcc-
> >glz_data.usr);
> + glz_enc_dictionary_create(window_size, MAX_LZ_ENCODERS, &enc-
> >glz_data.usr);
>
> - return glz_shared_dictionary_new(RED_CHANNEL_CLIENT(dcc)->client, id,
> glz_dict);
> + return glz_shared_dictionary_new(client, id, glz_dict);
> }
>
> -GlzSharedDictionary *dcc_get_glz_dictionary(DisplayChannelClient *dcc,
> - uint8_t id, int window_size)
> +gboolean image_encoders_get_glz_dictionary(ImageEncoders *enc,
> + RedClient *client,
> + uint8_t id, int window_size)
> {
> GlzSharedDictionary *shared_dict;
>
> pthread_mutex_lock(&glz_dictionary_list_lock);
>
> - shared_dict = find_glz_dictionary(RED_CHANNEL_CLIENT(dcc)->client, id);
> + shared_dict = find_glz_dictionary(client, id);
> if (shared_dict) {
> shared_dict->refs++;
> } else {
> - shared_dict = create_glz_dictionary(dcc, id, window_size);
> + shared_dict = create_glz_dictionary(enc, client, id, window_size);
> ring_add(&glz_dictionary_list, &shared_dict->base);
> }
>
> pthread_mutex_unlock(&glz_dictionary_list_lock);
> - return shared_dict;
> + enc->glz_dict = shared_dict;
> + return shared_dict != NULL;
> }
>
> -static GlzSharedDictionary *restore_glz_dictionary(DisplayChannelClient *dcc,
> +static GlzSharedDictionary *restore_glz_dictionary(ImageEncoders *enc,
> + RedClient *client,
> uint8_t id,
> GlzEncDictRestoreData
> *restore_data)
> {
> GlzEncDictContext *glz_dict =
> - glz_enc_dictionary_restore(restore_data, &dcc->glz_data.usr);
> + glz_enc_dictionary_restore(restore_data, &enc->glz_data.usr);
>
> - return glz_shared_dictionary_new(RED_CHANNEL_CLIENT(dcc)->client, id,
> glz_dict);
> + return glz_shared_dictionary_new(client, id, glz_dict);
> }
>
> -GlzSharedDictionary *dcc_restore_glz_dictionary(DisplayChannelClient *dcc,
> - uint8_t id,
> - GlzEncDictRestoreData
> *restore_data)
> +gboolean image_encoders_restore_glz_dictionary(ImageEncoders *enc,
> + RedClient *client,
> + uint8_t id,
> + GlzEncDictRestoreData
> *restore_data)
> {
> GlzSharedDictionary *shared_dict = NULL;
>
> pthread_mutex_lock(&glz_dictionary_list_lock);
>
> - shared_dict = find_glz_dictionary(RED_CHANNEL_CLIENT(dcc)->client, id);
> + shared_dict = find_glz_dictionary(client, id);
>
> if (shared_dict) {
> shared_dict->refs++;
> } else {
> - shared_dict = restore_glz_dictionary(dcc, id, restore_data);
> + shared_dict = restore_glz_dictionary(enc, client, id, restore_data);
> ring_add(&glz_dictionary_list, &shared_dict->base);
> }
>
> pthread_mutex_unlock(&glz_dictionary_list_lock);
> - return shared_dict;
> + enc->glz_dict = shared_dict;
> + return shared_dict != NULL;
> +}
> +
> +gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id)
> +{
> + enc->glz = glz_encoder_create(id, enc->glz_dict->dict, &enc-
> >glz_data.usr);
> + return enc->glz != NULL;
> }
>
> /* destroy encoder, and dictionary if no one uses it*/
> void dcc_release_glz(DisplayChannelClient *dcc)
> {
> + ImageEncoders *enc = &dcc->encoders;
> GlzSharedDictionary *shared_dict;
>
> dcc_free_glz_drawables(dcc);
>
> - glz_encoder_destroy(dcc->glz);
> - dcc->glz = NULL;
> + glz_encoder_destroy(enc->glz);
> + enc->glz = NULL;
>
> - if (!(shared_dict = dcc->glz_dict)) {
> + if (!(shared_dict = enc->glz_dict)) {
> return;
> }
>
> - dcc->glz_dict = NULL;
> + enc->glz_dict = NULL;
> pthread_mutex_lock(&glz_dictionary_list_lock);
> if (--shared_dict->refs != 0) {
> pthread_mutex_unlock(&glz_dictionary_list_lock);
> @@ -706,7 +717,7 @@ void dcc_release_glz(DisplayChannelClient *dcc)
> }
> ring_remove(&shared_dict->base);
> pthread_mutex_unlock(&glz_dictionary_list_lock);
> - glz_enc_dictionary_destroy(shared_dict->dict, &dcc->glz_data.usr);
> + glz_enc_dictionary_destroy(shared_dict->dict, &enc->glz_data.usr);
> free(shared_dict);
> }
>
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index f62a496..51f0cfe 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -41,14 +41,15 @@ void image_encoder_globals_init(ImageEncoderGlobals
> *globals);
> void image_encoder_globals_stat_reset(ImageEncoderGlobals *globals);
> void image_encoder_globals_stat_print(const ImageEncoderGlobals *globals);
>
> -void dcc_encoders_init(DisplayChannelClient *dcc, 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 dcc_freeze_glz (DisplayChannelC
> lient *dcc);
> +gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id);
> +void image_encoders_freeze_glz(ImageEncoders *enc);
> void dcc_release_glz (DisplayChannelC
> lient *dcc);
>
> #define RED_COMPRESS_BUF_SIZE (1024 * 64)
> @@ -79,11 +80,13 @@ typedef struct GlzSharedDictionary {
> RedClient *client; // channel clients of the same client share the dict
> } GlzSharedDictionary;
>
> -GlzSharedDictionary*
> dcc_get_glz_dictionary (DisplayChannelClient *dcc,
> - uint8_t id, int
> window_size);
> -GlzSharedDictionary*
> dcc_restore_glz_dictionary (DisplayChannelClient *dcc,
> - uint8_t id,
> - GlzEncDictResto
> reData *restore_data);
> +gboolean image_encoders_get_glz_dictionary(ImageEncoders *enc,
> + struct RedClient *client,
> + uint8_t id, int window_size);
> +gboolean image_encoders_restore_glz_dictionary(ImageEncoders *enc,
> + struct RedClient *client,
> + uint8_t id,
> + GlzEncDictRestoreData
> *restore_data);
>
> typedef struct {
> RedCompressBuf *bufs_head;
> @@ -200,6 +203,11 @@ struct ImageEncoders {
>
> ZlibData zlib_data;
> ZlibEncoder *zlib;
> +
> + /* global lz encoding entities */
> + GlzSharedDictionary *glz_dict;
> + GlzEncoderContext *glz;
> + GlzData glz_data;
> };
>
> typedef struct compress_send_data_t {
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index 6c10565..c0c7573 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -1859,12 +1859,12 @@ static void
> display_channel_marshall_migrate_data(RedChannelClient *rcc,
> memcpy(display_data.pixmap_cache_clients, dcc->pixmap_cache->sync,
> sizeof(display_data.pixmap_cache_clients));
>
> - spice_assert(dcc->glz_dict);
> - dcc_freeze_glz(dcc);
> - display_data.glz_dict_id = dcc->glz_dict->id;
> - glz_enc_dictionary_get_restore_data(dcc->glz_dict->dict,
> + spice_assert(dcc->encoders.glz_dict);
> + image_encoders_freeze_glz(&dcc->encoders);
> + display_data.glz_dict_id = dcc->encoders.glz_dict->id;
> + glz_enc_dictionary_get_restore_data(dcc->encoders.glz_dict->dict,
> &display_data.glz_dict_data,
> - &dcc->glz_data.usr);
> + &dcc->encoders.glz_data.usr);
>
> /* all data besided the surfaces ref */
> spice_marshaller_add(base_marshaller,
> diff --git a/server/dcc.c b/server/dcc.c
> index 8f44e64..1cc85b5 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -396,7 +396,7 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
> ring_init(&dcc->glz_drawables_inst_to_free);
> pthread_mutex_init(&dcc->glz_drawables_inst_to_free_lock, NULL);
>
> - dcc_encoders_init(dcc, &display->encoder_globals);
> + image_encoders_init(&dcc->encoders, &display->encoder_globals);
>
> return dcc;
> }
> @@ -422,12 +422,11 @@ static int
> display_channel_client_wait_for_init(DisplayChannelClient *dcc)
> if (!red_channel_client_is_connected(RED_CHANNEL_CLIENT(dcc))) {
> break;
> }
> - if (dcc->pixmap_cache && dcc->glz_dict) {
> + if (dcc->pixmap_cache && dcc->encoders.glz_dict) {
> dcc->pixmap_cache_generation = dcc->pixmap_cache->generation;
> /* TODO: move common.id? if it's used for a per client
> structure.. */
> spice_info("creating encoder with id == %d", dcc->id);
> - dcc->glz = glz_encoder_create(dcc->id, dcc->glz_dict->dict, &dcc-
> >glz_data.usr);
> - if (!dcc->glz) {
> + if (!image_encoders_glz_create(&dcc->encoders, dcc->id)) {
> spice_critical("create global lz failed");
> }
> return TRUE;
> @@ -711,7 +710,7 @@ static int dcc_compress_image_glz(DisplayChannelClient
> *dcc,
> stat_start_time_t start_time;
> stat_start_time_init(&start_time, &display_channel-
> >encoder_globals.zlib_glz_stat);
> spice_assert(bitmap_fmt_is_rgb(src->format));
> - GlzData *glz_data = &dcc->glz_data;
> + GlzData *glz_data = &dcc->encoders.glz_data;
> ZlibData *zlib_data;
> LzImageType type = bitmap_fmt_to_lz_image_type[src->format];
> RedGlzDrawable *glz_drawable;
> @@ -733,7 +732,7 @@ static int dcc_compress_image_glz(DisplayChannelClient
> *dcc,
> glz_data->data.u.lines_data.next = 0;
> glz_data->data.u.lines_data.reverse = 0;
>
> - glz_size = glz_encode(dcc->glz, type, src->x, src->y,
> + glz_size = glz_encode(dcc->encoders.glz, type, src->x, src->y,
> (src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN), NULL,
> 0,
> src->stride, glz_data->data.bufs_head->buf.bytes,
> sizeof(glz_data->data.bufs_head->buf),
> @@ -893,15 +892,15 @@ int dcc_compress_image(DisplayChannelClient *dcc,
> success = image_encoders_compress_quic(&dcc->encoders, dest, src,
> o_comp_data);
> break;
> case SPICE_IMAGE_COMPRESSION_GLZ:
> - if ((src->x * src->y) < glz_enc_dictionary_get_size(dcc->glz_dict-
> >dict)) {
> + if ((src->x * src->y) < glz_enc_dictionary_get_size(dcc-
> >encoders.glz_dict->dict)) {
> int frozen;
> /* using the global dictionary only if it is not frozen */
> - pthread_rwlock_rdlock(&dcc->glz_dict->encode_lock);
> - frozen = dcc->glz_dict->migrate_freeze;
> + pthread_rwlock_rdlock(&dcc->encoders.glz_dict->encode_lock);
> + frozen = dcc->encoders.glz_dict->migrate_freeze;
> if (!frozen) {
> success = dcc_compress_image_glz(dcc, dest, src, drawable,
> o_comp_data);
> }
> - pthread_rwlock_unlock(&dcc->glz_dict->encode_lock);
> + pthread_rwlock_unlock(&dcc->encoders.glz_dict->encode_lock);
> if (!frozen) {
> break;
> }
> @@ -1053,6 +1052,8 @@ int dcc_pixmap_cache_unlocked_add(DisplayChannelClient
> *dcc, uint64_t id,
>
> static int dcc_handle_init(DisplayChannelClient *dcc, SpiceMsgcDisplayInit
> *init)
> {
> + gboolean success;
> +
> spice_return_val_if_fail(dcc->expect_init, FALSE);
> dcc->expect_init = FALSE;
>
> @@ -1062,11 +1063,12 @@ static int dcc_handle_init(DisplayChannelClient *dcc,
> SpiceMsgcDisplayInit *init
> init->pixmap_cache_size);
> spice_return_val_if_fail(dcc->pixmap_cache, FALSE);
>
> - spice_return_val_if_fail(!dcc->glz_dict, FALSE);
> - dcc->glz_dict = dcc_get_glz_dictionary(dcc,
> - init->glz_dictionary_id,
> - init->glz_dictionary_window_size);
> - spice_return_val_if_fail(dcc->glz_dict, FALSE);
> + spice_return_val_if_fail(!dcc->encoders.glz_dict, FALSE);
> + success = image_encoders_get_glz_dictionary(&dcc->encoders,
> + RED_CHANNEL_CLIENT(dcc)-
> >client,
> + init->glz_dictionary_id,
> + init-
> >glz_dictionary_window_size);
> + spice_return_val_if_fail(success, FALSE);
>
> return TRUE;
> }
> @@ -1166,12 +1168,12 @@ int dcc_handle_message(RedChannelClient *rcc, uint32_t
> size, uint16_t type, void
> static int dcc_handle_migrate_glz_dictionary(DisplayChannelClient *dcc,
> SpiceMigrateDataDisplay
> *migrate)
> {
> - spice_return_val_if_fail(!dcc->glz_dict, FALSE);
> + spice_return_val_if_fail(!dcc->encoders.glz_dict, FALSE);
>
> - dcc->glz_dict = dcc_restore_glz_dictionary(dcc,
> - migrate->glz_dict_id,
> - &migrate->glz_dict_data);
> - return dcc->glz_dict != NULL;
> + return image_encoders_restore_glz_dictionary(&dcc->encoders,
> + RED_CHANNEL_CLIENT(dcc)-
> >client,
> + migrate->glz_dict_id,
> + &migrate->glz_dict_data);
> }
>
> static int restore_surface(DisplayChannelClient *dcc, uint32_t surface_id)
> @@ -1264,8 +1266,7 @@ int dcc_handle_migrate_data(DisplayChannelClient *dcc,
> uint32_t size, void *mess
> }
>
> if (dcc_handle_migrate_glz_dictionary(dcc, migrate_data)) {
> - dcc->glz =
> - glz_encoder_create(dcc->id, dcc->glz_dict->dict, &dcc-
> >glz_data.usr);
> + image_encoders_glz_create(&dcc->encoders, dcc->id);
> } else {
> spice_critical("restoring global lz dictionary failed");
> }
> diff --git a/server/dcc.h b/server/dcc.h
> index c6c9903..a4b917b 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -84,10 +84,6 @@ struct DisplayChannelClient {
> } send_data;
>
> /* global lz encoding entities */
> - 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;
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 2f2885f..4aa912a 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -1202,7 +1202,7 @@ void display_channel_free_some(DisplayChannel *display)
> spice_debug("#draw=%d, #glz_draw=%d", display->drawable_count,
> display->glz_drawable_count);
> FOREACH_CLIENT(display, link, next, dcc) {
> - GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
> + GlzSharedDictionary *glz_dict = dcc->encoders.glz_dict;
>
> if (glz_dict) {
> // encoding using the dictionary is prevented since the following
> operations might
> @@ -1217,7 +1217,7 @@ void display_channel_free_some(DisplayChannel *display)
> }
>
> FOREACH_CLIENT(display, link, next, dcc) {
> - GlzSharedDictionary *glz_dict = dcc ? dcc->glz_dict : NULL;
> + GlzSharedDictionary *glz_dict = dcc->encoders.glz_dict;
>
> if (glz_dict) {
> pthread_rwlock_unlock(&glz_dict->encode_lock);
Other than the unrelated image_encoders_init() change, ACK
Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
More information about the Spice-devel
mailing list