[Spice-devel] [PATCH spice v2 4/4] dcc: Rewrite dcc_image_compress
Pavel Grunt
pgrunt at redhat.com
Wed Jan 20 00:25:53 PST 2016
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 at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
More information about the Spice-devel
mailing list