[Spice-devel] [PATCH spice v2 4/4] dcc: Rewrite dcc_image_compress
Frediano Ziglio
fziglio at redhat.com
Tue Jan 19 07:16:32 PST 2016
>
> 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
More information about the Spice-devel
mailing list