> > Rules are now: > > Compression type: > off -> uncompressed > quic -> quic if possible else off I think original code always fall back to jpeg (if possible). Actually was more an use jpeg if possible and enabled instead of a fall back. > lz -> lz if possible else off > glz -> glz if possible else lz else off > auto_lz -> lz if possible else jpeg else quic else off > auto_glz -> glz if possible else lz else jpeg else quic else off > lz4 -> lz4 if possible else lz else off > --- > server/dcc.c | 170 > +++++++++++++++++++++++++++++------------------------------ > 1 file changed, 84 insertions(+), 86 deletions(-) > > diff --git a/server/dcc.c b/server/dcc.c > index 9b65031..8a0914d 100644 > --- a/server/dcc.c > +++ b/server/dcc.c > @@ -1028,6 +1028,38 @@ static int > dcc_compress_image_quic(DisplayChannelClient *dcc, SpiceImage *dest, > } > > #define MIN_SIZE_TO_COMPRESS 54 > +static bool can_compress_bitmap(SpiceBitmap *bitmap, SpiceImageCompression > image_compression) > +{ Would not be better to return a SpiceImageCompression with the specific compression chosen? Frediano > + spice_return_val_if_fail(image_compression > > SPICE_IMAGE_COMPRESSION_INVALID && > + image_compression < > SPICE_IMAGE_COMPRESSION_ENUM_END, FALSE); > + > + /* > + quic doesn't handle: > + (1) palette > + lz doesn't handle: > + (1) bitmaps with strides that are larger than the width of the image > in bytes > + (2) unstable bitmaps > + */ > + if (bitmap->y * bitmap->stride < MIN_SIZE_TO_COMPRESS) { // TODO: change > the size cond > + return FALSE; > + } > + if (image_compression == SPICE_IMAGE_COMPRESSION_OFF) { > + return FALSE; > + } > + if (image_compression == SPICE_IMAGE_COMPRESSION_QUIC) { > + return !bitmap_fmt_is_plt(bitmap->format); > + } > + if (image_compression == SPICE_IMAGE_COMPRESSION_LZ || > + image_compression == SPICE_IMAGE_COMPRESSION_LZ4 || > + image_compression == SPICE_IMAGE_COMPRESSION_GLZ || > + bitmap_fmt_is_plt(bitmap->format)) { > + return !bitmap_has_extra_stride(bitmap) && > + !(bitmap->data->flags & SPICE_CHUNKS_FLAGS_UNSTABLE); > + } > + > + return TRUE; > +} > + > #define MIN_DIMENSION_TO_QUIC 3 > int dcc_compress_image(DisplayChannelClient *dcc, > SpiceImage *dest, SpiceBitmap *src, Drawable > *drawable, > @@ -1037,49 +1069,29 @@ int dcc_compress_image(DisplayChannelClient *dcc, > DisplayChannel *display_channel = DCC_TO_DC(dcc); > SpiceImageCompression image_compression = dcc->image_compression; > int quic_compress = FALSE; > + int jpeg_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 > + if (!can_compress_bitmap(src, image_compression)) { > return FALSE; > - } else if (image_compression == SPICE_IMAGE_COMPRESSION_QUIC) { > - if (bitmap_fmt_is_plt(src->format)) { > - return FALSE; > - } else { > - quic_compress = TRUE; > - } > - } else { > - /* > - lz doesn't handle (1) bitmaps with strides that are larger than > the width > - of the image in bytes (2) unstable bitmaps > - */ > - if (bitmap_has_extra_stride(src) || (src->data->flags & > SPICE_CHUNKS_FLAGS_UNSTABLE)) { > - if ((image_compression == SPICE_IMAGE_COMPRESSION_LZ) || > - (image_compression == SPICE_IMAGE_COMPRESSION_GLZ) || > - (image_compression == SPICE_IMAGE_COMPRESSION_LZ4) || > - bitmap_fmt_is_plt(src->format)) { > - return FALSE; > - } else { > - quic_compress = TRUE; > - } > - } else { > - if ((image_compression == SPICE_IMAGE_COMPRESSION_AUTO_LZ) || > - (image_compression == SPICE_IMAGE_COMPRESSION_AUTO_GLZ)) { > - if ((src->x < MIN_DIMENSION_TO_QUIC) || (src->y < > MIN_DIMENSION_TO_QUIC)) { > - quic_compress = FALSE; > - } else { > - if (drawable == NULL || > - drawable->copy_bitmap_graduality == > BITMAP_GRADUAL_INVALID) { > - quic_compress = > bitmap_fmt_has_graduality(src->format) && > + } > + if (image_compression == SPICE_IMAGE_COMPRESSION_QUIC) { > + quic_compress = TRUE; > + } else if ((image_compression == SPICE_IMAGE_COMPRESSION_AUTO_LZ || > + image_compression == SPICE_IMAGE_COMPRESSION_AUTO_GLZ) && > + !bitmap_fmt_is_plt(src->format) && > + src->x >= MIN_DIMENSION_TO_QUIC && src->y >= > MIN_DIMENSION_TO_QUIC) { > + 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 { > - quic_compress = (drawable->copy_bitmap_graduality == > BITMAP_GRADUAL_HIGH); > - } > - } > - } else { > - quic_compress = FALSE; > - } > + } else { > + quic_compress = bitmap_has_extra_stride(src) || > + (src->data->flags & SPICE_CHUNKS_FLAGS_UNSTABLE) > || > + drawable->copy_bitmap_graduality == > BITMAP_GRADUAL_HIGH; > } > + jpeg_compress = can_lossy && display_channel->enable_jpeg && > + (src->format != SPICE_BITMAP_FMT_RGBA || > !bitmap_has_extra_stride(src)); > } > > if (drawable != NULL) { > @@ -1089,70 +1101,56 @@ int dcc_compress_image(DisplayChannelClient *dcc, > } > > if (quic_compress) { > + if (jpeg_compress) { > #ifdef COMPRESS_DEBUG > - spice_info("QUIC compress"); > + spice_info("JPEG compress"); > #endif > - // if bitmaps is picture-like, compress it using jpeg > - if (can_lossy && display_channel->enable_jpeg && > - ((image_compression == SPICE_IMAGE_COMPRESSION_AUTO_LZ) || > - (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, > group_id); > - } > + /* bitmap is picture-like, compress it using jpeg */ > + return dcc_compress_image_jpeg(dcc, dest, src, o_comp_data, > group_id); > } > +#ifdef COMPRESS_DEBUG > + spice_info("QUIC compress"); > +#endif > 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_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; > - } > + } > > - if (glz) { > + if ((image_compression == SPICE_IMAGE_COMPRESSION_AUTO_GLZ) || > + (image_compression == SPICE_IMAGE_COMPRESSION_GLZ)) { > + if (drawable != NULL && > + bitmap_fmt_has_graduality(src->format) && > + (src->x * src->y) < > glz_enc_dictionary_get_size(dcc->glz_dict->dict)) { > + int ret, frozen; > /* using the global dictionary only if it is not frozen */ > pthread_rwlock_rdlock(&dcc->glz_dict->encode_lock); > - if (!dcc->glz_dict->migrate_freeze) { > - ret = dcc_compress_image_glz(dcc, > - dest, src, > - drawable, o_comp_data); > - } else { > - glz = FALSE; > + frozen = dcc->glz_dict->migrate_freeze; > + if (!frozen) { > +#ifdef COMPRESS_DEBUG > + spice_info("LZ global compress fmt=%d", src->format); > +#endif > + ret = dcc_compress_image_glz(dcc, dest, src, drawable, > o_comp_data); > } > pthread_rwlock_unlock(&dcc->glz_dict->encode_lock); > + if (!frozen) { > + return ret; > + } > } > + } > > - if (!glz) { > #ifdef USE_LZ4 > - if (image_compression == SPICE_IMAGE_COMPRESSION_LZ4 && > - 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, > group_id); > - } else > -#endif > - ret = dcc_compress_image_lz(dcc, dest, src, o_comp_data, > group_id); > + if (image_compression == SPICE_IMAGE_COMPRESSION_LZ4 && > + bitmap_fmt_is_rgb(src->format) && > + red_channel_client_test_remote_cap(&dcc->common.base, > SPICE_DISPLAY_CAP_LZ4_COMPRESSION)) { > #ifdef COMPRESS_DEBUG > - spice_info("LZ LOCAL compress"); > + spice_info("LZ4 compress"); > #endif > - } > + return dcc_compress_image_lz4(dcc, dest, src, o_comp_data, > group_id); > + } > +#endif > + > #ifdef COMPRESS_DEBUG > - else { > - spice_info("LZ global compress fmt=%d", src->format); > - } > + spice_info("LZ LOCAL compress"); > #endif > - return ret; > - } > + return dcc_compress_image_lz(dcc, dest, src, o_comp_data, group_id); > } > > #define CLIENT_PALETTE_CACHE > -- > 2.5.0 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel