[Spice-devel] [PATCH 03/11] worker: move compression parameters to dcc
Frediano Ziglio
fziglio at redhat.com
Wed Nov 18 03:45:52 PST 2015
>
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau at gmail.com>
> [updated for the preferred compression]
> Signed-off-by: Pavel Grunt <pgrunt at redhat.com>
> ---
> server/display-channel.c | 9 ++++++++-
> server/display-channel.h | 8 +++++++-
> server/red_worker.c | 32 ++++++++++++++++----------------
> 3 files changed, 31 insertions(+), 18 deletions(-)
>
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 3b5a246..1d5d8d3 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -124,7 +124,11 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
> RedClient *client, RedsStream *stream,
> int mig_target,
> uint32_t *common_caps, int num_common_caps,
> - uint32_t *caps, int num_caps)
> + uint32_t *caps, int num_caps,
> + SpiceImageCompression image_compression,
> + spice_wan_compression_t jpeg_state,
> + spice_wan_compression_t zlib_glz_state)
> +
> {
> DisplayChannelClient *dcc;
>
Style note (more for future consideration):
these 3 parameters are more or less together. Would be worth having them
inside a structure to you could use
DisplayChannelClient *dcc_new(DisplayChannel *display,
RedClient *client, RedsStream *stream,
int mig_target,
uint32_t *common_caps, int num_common_caps,
uint32_t *caps, int num_caps)
uint32_t *caps, int num_caps,
ImageCompressSettings compress_settings)
and ...
> @@ -137,6 +141,9 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
>
> ring_init(&dcc->palette_cache_lru);
> dcc->palette_cache_available = CLIENT_PALETTE_CACHE_SIZE;
> + dcc->image_compression = image_compression;
> + dcc->jpeg_state = jpeg_state;
> + dcc->zlib_glz_state = zlib_glz_state;
>
> return dcc;
> }
dcc->compress_settings = compress_settings;
??
> diff --git a/server/display-channel.h b/server/display-channel.h
> index cae0343..edbd4b9 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -168,6 +168,9 @@ struct Drawable {
>
> struct DisplayChannelClient {
> CommonChannelClient common;
> + SpiceImageCompression image_compression;
> + spice_wan_compression_t jpeg_state;
> + spice_wan_compression_t zlib_glz_state;
>
> int expect_init;
>
> @@ -240,7 +243,10 @@ DisplayChannelClient* dcc_new
> (DisplayCha
> uint32_t
> *common_caps,
> int
> num_common_caps,
> uint32_t
> *caps,
> - int
> num_caps);
> + int
> num_caps,
> +
> SpiceImageCompression
> image_compression,
> +
> spice_wan_compression_t
> jpeg_state,
> +
> spice_wan_compression_t
> zlib_glz_state);
> void dcc_push_monitors_config
> (DisplayChannelClient *dcc);
> void dcc_push_destroy_surface
> (DisplayChannelClient *dcc,
> uint32_t
> surface_id);
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 608229c..5a27879 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -4406,8 +4406,7 @@ static inline int
> red_compress_image(DisplayChannelClient *dcc,
> compress_send_data_t* o_comp_data)
> {
> DisplayChannel *display_channel = DCC_TO_DC(dcc);
> - SpiceImageCompression image_compression =
> - display_channel->common.worker->image_compression;
> + SpiceImageCompression image_compression = dcc->image_compression;
> int quic_compress = FALSE;
>
> if ((image_compression == SPICE_IMAGE_COMPRESSION_OFF) ||
> @@ -4811,15 +4810,14 @@ static void fill_mask(RedChannelClient *rcc,
> SpiceMarshaller *m,
> SpiceImage *mask_bitmap, Drawable *drawable)
> {
> DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
> - DisplayChannel *display = DCC_TO_DC(dcc);
>
> if (mask_bitmap && m) {
> - if (display->common.worker->image_compression !=
> SPICE_IMAGE_COMPRESSION_OFF) {
> - SpiceImageCompression save_img_comp =
> - display->common.worker->image_compression;
> - display->common.worker->image_compression =
> SPICE_IMAGE_COMPRESSION_OFF;
> + if (dcc->image_compression != SPICE_IMAGE_COMPRESSION_OFF) {
> + /* todo: pass compression argument */
> + SpiceImageCompression save_img_comp = dcc->image_compression;
> + dcc->image_compression = SPICE_IMAGE_COMPRESSION_OFF;
> fill_bits(dcc, m, mask_bitmap, drawable, FALSE);
> - display->common.worker->image_compression = save_img_comp;
> + dcc->image_compression = save_img_comp;
> } else {
> fill_bits(dcc, m, mask_bitmap, drawable, FALSE);
> }
> @@ -6706,7 +6704,7 @@ static void red_marshall_image(RedChannelClient *rcc,
> SpiceMarshaller *m, ImageI
>
> compress_send_data_t comp_send_data = {0};
>
> - comp_mode = display_channel->common.worker->image_compression;
> + comp_mode = dcc->image_compression;
>
> if (((comp_mode == SPICE_IMAGE_COMPRESSION_AUTO_LZ) ||
> (comp_mode == SPICE_IMAGE_COMPRESSION_AUTO_GLZ)) &&
> !bitmap_has_extra_stride(&bitmap)) {
> @@ -7808,10 +7806,10 @@ static int
> display_channel_handle_migrate_data(RedChannelClient *rcc, uint32_t s
>
> if (migrate_data->low_bandwidth_setting) {
> red_channel_client_ack_set_client_window(rcc,
> WIDE_CLIENT_ACK_WINDOW);
> - if (DCC_TO_WORKER(dcc)->jpeg_state == SPICE_WAN_COMPRESSION_AUTO) {
> + if (dcc->jpeg_state == SPICE_WAN_COMPRESSION_AUTO) {
> display_channel->enable_jpeg = TRUE;
> }
> - if (DCC_TO_WORKER(dcc)->zlib_glz_state ==
> SPICE_WAN_COMPRESSION_AUTO) {
> + if (dcc->zlib_glz_state == SPICE_WAN_COMPRESSION_AUTO) {
> display_channel->enable_zlib_glz_wrap = TRUE;
> }
> }
> @@ -7879,6 +7877,7 @@ static int
> display_channel_handle_preferred_compression(DisplayChannelClient *dc
> case SPICE_IMAGE_COMPRESSION_GLZ:
> case SPICE_IMAGE_COMPRESSION_OFF:
> display_channel->common.worker->image_compression =
> pc->image_compression;
> + dcc->image_compression = pc->image_compression;
> return TRUE;
> default:
> spice_warning("preferred-compression: unsupported image compression
> setting");
> @@ -8349,7 +8348,8 @@ static void handle_new_display_channel(RedWorker
> *worker, RedClient *client, Red
> display_channel = worker->display_channel;
> spice_info("add display channel client");
> dcc = dcc_new(display_channel, client, stream, migrate,
> - common_caps, num_common_caps, caps, num_caps);
> + common_caps, num_common_caps, caps, num_caps,
> + worker->image_compression, worker->jpeg_state,
> worker->zlib_glz_state);
> if (!dcc) {
> return;
> }
> @@ -8364,19 +8364,19 @@ static void handle_new_display_channel(RedWorker
> *worker, RedClient *client, Red
> DISPLAY_FREE_LIST_DEFAULT_SIZE *
> sizeof(SpiceResourceID));
> dcc->send_data.free_list.res_size = DISPLAY_FREE_LIST_DEFAULT_SIZE;
>
> - if (worker->jpeg_state == SPICE_WAN_COMPRESSION_AUTO) {
> + if (dcc->jpeg_state == SPICE_WAN_COMPRESSION_AUTO) {
> display_channel->enable_jpeg = dcc->common.is_low_bandwidth;
> } else {
> - display_channel->enable_jpeg = (worker->jpeg_state ==
> SPICE_WAN_COMPRESSION_ALWAYS);
> + display_channel->enable_jpeg = (dcc->jpeg_state ==
> SPICE_WAN_COMPRESSION_ALWAYS);
> }
>
> // todo: tune quality according to bandwidth
> display_channel->jpeg_quality = 85;
>
> - if (worker->zlib_glz_state == SPICE_WAN_COMPRESSION_AUTO) {
> + if (dcc->zlib_glz_state == SPICE_WAN_COMPRESSION_AUTO) {
> display_channel->enable_zlib_glz_wrap =
> dcc->common.is_low_bandwidth;
> } else {
> - display_channel->enable_zlib_glz_wrap = (worker->zlib_glz_state ==
> + display_channel->enable_zlib_glz_wrap = (dcc->zlib_glz_state ==
> SPICE_WAN_COMPRESSION_ALWAYS);
> }
>
> --
> 2.4.3
Code is good (now)... comment not really. We are not moving compression parameters
but adding to DCC. I'll try a new proposal:
worker: add compression parameters to dcc
This allow different dcc to have different settings from default one.
The parameters are copied initially from default settings but then they can
change independently for each client.
Even having a single client a future client is not affected by a previous
setting on the old dcc.
Note: on a future patch the
display_channel->common.worker->image_compression = pc->image_compression;
line is removed from display_channel_handle_preferred_compression.
Perhaps this change can be incorporated now.
Frediano
More information about the Spice-devel
mailing list