On Tue, 2016-06-07 at 11:17 +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> server/dcc-encoders.c | 30 ++++++++++++++++--------------
> server/dcc-encoders.h | 3 +++
> server/dcc.c | 23 +++++++++++++----------
> server/dcc.h | 2 --
> 4 files changed, 32 insertions(+), 26 deletions(-)
>
> diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
> index 99e5507..54b970a 100644
> --- a/server/dcc-encoders.c
> +++ b/server/dcc-encoders.c
> @@ -308,19 +308,19 @@ static void dcc_init_quic(EncodersData *enc)
> }
> }
>
> -static void dcc_init_lz(DisplayChannelClient *dcc)
> +static void dcc_init_lz(EncodersData *enc)
Same comment as previous patch about dcc_ vs encoders_data_ prefix
> {
> - 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;
> + enc->lz_data.usr.error = lz_usr_error;
> + enc->lz_data.usr.warn = lz_usr_warn;
> + enc->lz_data.usr.info = lz_usr_warn;
> + enc->lz_data.usr.malloc = lz_usr_malloc;
> + enc->lz_data.usr.free = lz_usr_free;
> + enc->lz_data.usr.more_space = lz_usr_more_space;
> + enc->lz_data.usr.more_lines = lz_usr_more_lines;
>
> - dcc->lz = lz_create(&dcc->lz_data.usr);
> + enc->lz = lz_create(&enc->lz_data.usr);
>
> - if (!dcc->lz) {
> + if (!enc->lz) {
> spice_critical("create lz failed");
> }
> }
> @@ -400,9 +400,11 @@ static void dcc_init_zlib(DisplayChannelClient *dcc)
>
> void dcc_encoders_init(DisplayChannelClient *dcc)
> {
> + EncodersData *enc = &dcc->encoders;
> +
> dcc_init_glz_data(dcc);
> - dcc_init_quic(&dcc->encoders);
> - dcc_init_lz(dcc);
> + dcc_init_quic(enc);
> + dcc_init_lz(enc);
> dcc_init_jpeg(dcc);
> #ifdef USE_LZ4
> dcc_init_lz4(dcc);
> @@ -418,8 +420,8 @@ void dcc_encoders_free(DisplayChannelClient *dcc)
> EncodersData *enc = &dcc->encoders;
> quic_destroy(enc->quic);
> enc->quic = NULL;
> - lz_destroy(dcc->lz);
> - dcc->lz = NULL;
> + lz_destroy(enc->lz);
> + enc->lz = NULL;
> jpeg_encoder_destroy(dcc->jpeg);
> dcc->jpeg = NULL;
> #ifdef USE_LZ4
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index a75678e..21049cd 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -165,6 +165,9 @@ struct RedGlzDrawable {
> struct EncodersData {
> QuicData quic_data;
> QuicContext *quic;
> +
> + LzData lz_data;
> + LzContext *lz;
> };
>
> typedef struct compress_send_data_t {
> diff --git a/server/dcc.c b/server/dcc.c
> index 0bc806a..05a511d 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -784,17 +784,18 @@ glz:
> return TRUE;
> }
>
> -static int dcc_compress_image_lz(DisplayChannelClient *dcc,
> +static int dcc_compress_image_lz(EncodersData *enc,
> SpiceImage *dest, SpiceBitmap *src,
> - compress_send_data_t* o_comp_data)
> + compress_send_data_t* o_comp_data,
> + stat_info_t *stats)
as in quic patch, I think this stats change should be done in a separate patch.
Presumably it could be done for all encoders at once (if there are more of them
coming).
> {
> - LzData *lz_data = &dcc->lz_data;
> - LzContext *lz = dcc->lz;
> + LzData *lz_data = &enc->lz_data;
> + LzContext *lz = enc->lz;
> LzImageType type = bitmap_fmt_to_lz_image_type[src->format];
> int size; // size of the compressed data
>
> stat_start_time_t start_time;
> - stat_start_time_init(&start_time, &DCC_TO_DC(dcc)->lz_stat);
> + stat_start_time_init(&start_time, stats);
>
> #ifdef COMPRESS_DEBUG
> spice_info("LZ LOCAL compress");
> @@ -841,11 +842,10 @@ static int dcc_compress_image_lz(DisplayChannelClient
> *dcc,
> o_comp_data->comp_buf = lz_data->data.bufs_head;
> o_comp_data->comp_buf_size = size;
>
> - dcc_palette_cache_palette(dcc, dest->u.lz_plt.palette, &(dest-
> >u.lz_plt.flags));
I see this is moving to the caller. That's fine, but it doesn't really belong in
this patch.
> o_comp_data->lzplt_palette = dest->u.lz_plt.palette;
> }
>
> - stat_compress_add(&DCC_TO_DC(dcc)->lz_stat, start_time, src->stride *
> src->y,
> + stat_compress_add(stats, start_time, src->stride * src->y,
> o_comp_data->comp_buf_size);
> return TRUE;
> }
> @@ -854,9 +854,9 @@ static int dcc_compress_image_jpeg(DisplayChannelClient
> *dcc, SpiceImage *dest,
> SpiceBitmap *src, compress_send_data_t*
> o_comp_data)
> {
> JpegData *jpeg_data = &dcc->jpeg_data;
> - LzData *lz_data = &dcc->lz_data;
> + LzData *lz_data = &dcc->encoders.lz_data;
> JpegEncoderContext *jpeg = dcc->jpeg;
> - LzContext *lz = dcc->lz;
> + LzContext *lz = dcc->encoders.lz;
> volatile JpegEncoderImageType jpeg_in_type;
> int jpeg_size = 0;
> volatile int has_alpha = FALSE;
> @@ -1159,7 +1159,10 @@ int dcc_compress_image(DisplayChannelClient *dcc,
> #endif
> lz_compress:
> case SPICE_IMAGE_COMPRESSION_LZ:
> - success = dcc_compress_image_lz(dcc, dest, src, o_comp_data);
> + success = dcc_compress_image_lz(&dcc->encoders, dest, src,
> o_comp_data, &display_channel->lz_stat);
> + if (success && !bitmap_fmt_is_rgb(src->format)) {
> + dcc_palette_cache_palette(dcc, dest->u.lz_plt.palette, &(dest-
> >u.lz_plt.flags));
so the palette call has moved here, but why the additional check for
bitmap_fmt_is_rgb()? This needs some additional explanation.
> + }
> break;
> default:
> spice_error("invalid image compression type %u", image_compression);
> diff --git a/server/dcc.h b/server/dcc.h
> index 4b93c73..532ce7b 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -66,8 +66,6 @@ struct DisplayChannelClient {
>
> int zlib_level;
>
> - LzData lz_data;
> - LzContext *lz;
> JpegData jpeg_data;
> JpegEncoderContext *jpeg;
> #ifdef USE_LZ4
Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel