[Spice-devel] [PATCH spice-server] Use desired image compression for the first image

[Pavel Grunt]

Hi Christophe,

On Mon, 2015-06-22 at 14:20 +0200, Christophe Fergeau wrote:
> Hey,
> 
> Seems fine, though it's not clear to me why this would only impact the
> first image? 
It has impact on the first image because other images are compressed by
red_compress_image(). Maybe the mechanism of choosing the compression should be
the same for both functions (or have just one function for compression ?) - I
will look into it.

> Also, do we have limitations on the kind of images quic can
> compress? This patch always uses quic when SPICE_IMAGE_COMPRESS_QUIC is
> used.
> 
There are some limitations like bitmap format, size and dimensions, but if
encoding fails, the bitmap is sent. It is true that this patch uses quic when
image-compression=quic, but before this patch the quic compression was used when
image-compression was quic, lz, lz4, glz or off.

Pavel

> Christophe
> 
> On Fri, Jun 19, 2015 at 03:44:39PM +0200, Pavel Grunt wrote:
> > red_marshall_image() allows to use other than QUIC compression only
> > when auto_lz or auto_glz image compression is set.
> > ---
> > Also the LZ4 part of code in red_marshall_image could not be triggered
> > ---
> >  server/red_worker.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index 58a7d00..8fb7218 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -8820,9 +8820,9 @@ static void red_marshall_image(RedChannelClient *rcc, 
> > SpiceMarshaller *m, ImageI
> >      SpiceBitmap bitmap;
> >      SpiceChunks *chunks;
> >      QRegion *surface_lossy_region;
> > -    int comp_succeeded;
> > +    int comp_succeeded = FALSE;
> >      int lossy_comp = FALSE;
> > -    int lz_comp = FALSE;
> > +    int quic_comp = FALSE;
> >      spice_image_compression_t comp_mode;
> >      SpiceMsgDisplayDrawCopy copy;
> >      SpiceMarshaller *src_bitmap_out, *mask_bitmap_out;
> > @@ -8891,12 +8891,11 @@ static void red_marshall_image(RedChannelClient 
> > *rcc, SpiceMarshaller *m, ImageI
> >              if (grad_level == BITMAP_GRADUAL_HIGH) {
> >                  // if we use lz for alpha, the stride can't be extra
> >                  lossy_comp = display_channel->enable_jpeg && item
> > ->can_lossy;
> > -            } else {
> > -                lz_comp = TRUE;
> > +                quic_comp = TRUE;
> >              }
> > -        } else {
> > -            lz_comp = TRUE;
> >          }
> > +    } else if (comp_mode == SPICE_IMAGE_COMPRESS_QUIC) {
> > +        quic_comp = TRUE;
> >      }
> >  
> >      if (lossy_comp) {
> > @@ -8904,7 +8903,7 @@ static void red_marshall_image(RedChannelClient *rcc, 
> > SpiceMarshaller *m, ImageI
> >                                                   &bitmap, &comp_send_data,
> >                                                   worker
> > ->mem_slots.internal_groupslot_id);
> >      } else {
> > -        if (!lz_comp) {
> > +        if (quic_comp) {
> >              comp_succeeded = red_quic_compress_image(dcc, &red_image, 
> > &bitmap,
> >                                                       &comp_send_data,
> >                                                       worker
> > ->mem_slots.internal_groupslot_id);
> > @@ -8919,6 +8918,7 @@ static void red_marshall_image(RedChannelClient *rcc, 
> > SpiceMarshaller *m, ImageI
> >                                                          worker
> > ->mem_slots.internal_groupslot_id);
> >              } else
> >  #endif
> > +            if (comp_mode != SPICE_IMAGE_COMPRESS_OFF)
> >                  comp_succeeded = red_lz_compress_image(dcc, &red_image, 
> > &bitmap,
> >                                                         &comp_send_data,
> >                                                         worker
> > ->mem_slots.internal_groupslot_id);