> ---
> server/dcc.c | 38 +++++++++++++++++++++-----------------
> 1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/server/dcc.c b/server/dcc.c
> index eb5e4d1..b617b99 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -1056,6 +1056,7 @@ int dcc_compress_image(DisplayChannelClient *dcc,
> DisplayChannel *display_channel = DCC_TO_DC(dcc);
> SpiceImageCompression image_compression = dcc->image_compression;
> int quic_compress = FALSE;
> + uint32_t group_id;
>
> if ((image_compression == SPICE_IMAGE_COMPRESSION_OFF) ||
> ((src->y * src->stride) < MIN_SIZE_TO_COMPRESS)) { // TODO: change
> the size cond
> @@ -1086,7 +1087,8 @@ int dcc_compress_image(DisplayChannelClient *dcc,
> if ((src->x < MIN_DIMENSION_TO_QUIC) || (src->y <
> MIN_DIMENSION_TO_QUIC)) {
> quic_compress = FALSE;
> } else {
> - if (drawable->copy_bitmap_graduality ==
> BITMAP_GRADUAL_INVALID) {
> + if (drawable == NULL ||
> + drawable->copy_bitmap_graduality ==
> BITMAP_GRADUAL_INVALID) {
> quic_compress =
> bitmap_fmt_has_graduality(src->format) &&
> bitmap_get_graduality_level(src) ==
> BITMAP_GRADUAL_HIGH;
> } else {
> @@ -1099,6 +1101,12 @@ int dcc_compress_image(DisplayChannelClient *dcc,
> }
> }
>
> + if (drawable != NULL) {
> + group_id = drawable->group_id;
> + } else {
> + group_id =
> red_worker_get_memslot(display_channel->common.worker)->internal_groupslot_id;
> + }
> +
> if (quic_compress) {
> #ifdef COMPRESS_DEBUG
> spice_info("QUIC compress");
> @@ -1109,24 +1117,22 @@ int dcc_compress_image(DisplayChannelClient *dcc,
> (image_compression == SPICE_IMAGE_COMPRESSION_AUTO_GLZ))) {
> // if we use lz for alpha, the stride can't be extra
> if (src->format != SPICE_BITMAP_FMT_RGBA ||
> !bitmap_has_extra_stride(src)) {
> - return dcc_compress_image_jpeg(dcc, dest,
> - src, o_comp_data,
> drawable->group_id);
> + return dcc_compress_image_jpeg(dcc, dest, src, o_comp_data,
> group_id);
> }
> }
> - return dcc_compress_image_quic(dcc, dest,
> - src, o_comp_data,
> drawable->group_id);
> + return dcc_compress_image_quic(dcc, dest, src, o_comp_data,
> group_id);
> } else {
> int glz;
> int ret;
> - if ((image_compression == SPICE_IMAGE_COMPRESSION_AUTO_GLZ) ||
> - (image_compression == SPICE_IMAGE_COMPRESSION_GLZ)) {
> - glz = bitmap_fmt_has_graduality(src->format) && (
> - (src->x * src->y) < glz_enc_dictionary_get_size(
> - dcc->glz_dict->dict));
> - } else if ((image_compression == SPICE_IMAGE_COMPRESSION_AUTO_LZ) ||
> - (image_compression == SPICE_IMAGE_COMPRESSION_LZ) ||
> - (image_compression == SPICE_IMAGE_COMPRESSION_LZ4)) {
> + if ((image_compression == SPICE_IMAGE_COMPRESSION_AUTO_LZ) ||
> + (image_compression == SPICE_IMAGE_COMPRESSION_LZ) ||
> + (image_compression == SPICE_IMAGE_COMPRESSION_LZ4) ||
> + drawable == NULL) {
> glz = FALSE;
> + } else if ((image_compression == SPICE_IMAGE_COMPRESSION_AUTO_GLZ)
> ||
> + (image_compression == SPICE_IMAGE_COMPRESSION_GLZ)) {
> + glz = bitmap_fmt_has_graduality(src->format) &&
> + ((src->x * src->y) <
> glz_enc_dictionary_get_size(dcc->glz_dict->dict));
> } else {
> spice_error("invalid image compression type %u",
> image_compression);
> return FALSE;
Here the change looks bigger but you just moved the ifs to add the drawable == NULL check.
> @@ -1151,12 +1157,10 @@ int dcc_compress_image(DisplayChannelClient *dcc,
> bitmap_fmt_is_rgb(src->format) &&
> red_channel_client_test_remote_cap(&dcc->common.base,
> SPICE_DISPLAY_CAP_LZ4_COMPRESSION)) {
> - ret = dcc_compress_image_lz4(dcc, dest, src, o_comp_data,
> - drawable->group_id);
> + ret = dcc_compress_image_lz4(dcc, dest, src, o_comp_data,
> group_id);
> } else
> #endif
> - ret = dcc_compress_image_lz(dcc, dest, src, o_comp_data,
> - drawable->group_id);
> + ret = dcc_compress_image_lz(dcc, dest, src, o_comp_data,
> group_id);
> #ifdef COMPRESS_DEBUG
> spice_info("LZ LOCAL compress");
> #endif
> --
> 2.5.0
>
The patch looks good. From next patch this is done to use this function without
a drawable.
What looks ugly is the original code. If not really clear what the end results
will be. Beside all nested ifs and checks. Why for instance MIN_DIMENSION_TO_QUIC
is not checked when image_compression is SPICE_IMAGE_COMPRESSION_QUIC?
Is possible to compress palette images with Quic?
Not clear (at least to me) why glz require a Drawable.
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel