On Tue, 2016-01-19 at 10:16 -0500, Frediano Ziglio wrote: > > > > 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. The jpeg compression was only used with the auto_*, see: http://cgit.freedesktop.org/spice/spice/tree/server/dcc.c#n1086 I am not against changing it to use jpeg when possible, the code will be more simple > > > 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? > Do you mean returning SpiceImageCompression: SPICE_IMAGE_COMPRESSION_{OFF, QUIC, LZ, GLZ, LZ4} ? Pavel > 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