[Spice-devel] [PATCH spice-server] worker: remove compression parameters

[Pavel Grunt]

Hi Frediano,

On Tue, 2015-11-10 at 09:28 -0500, Frediano Ziglio wrote:
> > 
> > ---
> >  server/red_worker.c | 20 ++++++++------------
> >  1 file changed, 8 insertions(+), 12 deletions(-)
> > 
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index 111f8ba..85e0ebd 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -323,10 +323,6 @@ typedef struct RedWorker {
> >  
> >      RedMemSlotInfo mem_slots;
> >  
> > -    SpiceImageCompression image_compression;
> > -    spice_wan_compression_t jpeg_state;
> > -    spice_wan_compression_t zlib_glz_state;
> > -
> >      QuicData quic_data;
> >      QuicContext *quic;
> >  
> > @@ -8074,7 +8070,6 @@ static int
> > display_channel_handle_stream_report(DisplayChannelClient *dcc,
> >  
> >  static int
> > display_channel_handle_preferred_compression(DisplayChannelClient
> >  *dcc,
> >          SpiceMsgcDisplayPreferredCompression *pc) {
> > -    DisplayChannel *display_channel = DCC_TO_DC(dcc);
> >      switch (pc->image_compression) {
> >      case SPICE_IMAGE_COMPRESSION_AUTO_LZ:
> >      case SPICE_IMAGE_COMPRESSION_AUTO_GLZ:
> > @@ -8085,7 +8080,7 @@ static int
> > display_channel_handle_preferred_compression(DisplayChannelClient *dc
> >      case SPICE_IMAGE_COMPRESSION_LZ:
> >      case SPICE_IMAGE_COMPRESSION_GLZ:
> >      case SPICE_IMAGE_COMPRESSION_OFF:
> > -        display_channel->common.worker->image_compression =
> > pc->image_compression;
> > +        dcc->image_compression = pc->image_compression;
> >          return TRUE;
> >      default:
> >          spice_warning("preferred-compression: unsupported image compression
> >          setting");
> > @@ -8552,7 +8547,7 @@ static void handle_new_display_channel(RedWorker
> > *worker, RedClient *client, Red
> >      spice_info("add display channel client");
> >      dcc = dcc_new(display_channel, client, stream, migrate,
> >                    common_caps, num_common_caps, caps, num_caps,
> > -                  worker->image_compression, worker->jpeg_state,
> > worker->zlib_glz_state);
> > +                  image_compression, jpeg_state, zlib_glz_state);
> >      if (!dcc) {
> >          return;
> >      }
> 
> I think this uses just globals. Could be a problem if the different cards
> (like in Windows for multi monitor) wants to use different settings
Yes, using globals is not good, but I think there is a patch removing their
usage. Their values comes from the qemu calling
spice_server_set_*_compression().

Is it currently possible to have different compressions for each worker? Should
it be possible?

> 
> > @@ -9220,9 +9215,13 @@ static void handle_dev_set_compression(void *opaque,
> > void *payload)
> >  {
> >      RedWorkerMessageSetCompression *msg = payload;
> >      RedWorker *worker = opaque;
> > +    DisplayChannelClient *dcc;
> > +    RingItem *item, *next;
> >  
> > -    worker->image_compression = msg->image_compression;
> > -    switch (worker->image_compression) {
> > +    FOREACH_DCC(worker->display_channel, item, next, dcc) {
> > +        dcc->image_compression = msg->image_compression;
> > +    }
> 
> I think this change the behavior a bit.
> Previously the setting was used for newer created DCCs while
> now affects all present DCCs.

I consider this patch as follow up for the "worker: move compression parameters
to dcc". Before that patch the image compression was based on the RedWorker
image_compression parameter. However "worker: move compression parameters to
dcc" changed it (the compression depends on dcc image compression parameter).

Pavel

> 
> > +    switch (msg->image_compression) {
> >      case SPICE_IMAGE_COMPRESSION_AUTO_LZ:
> >          spice_info("ic auto_lz");
> >          break;
> > @@ -9589,9 +9588,6 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> > RedDispatcher *red_dispatcher)
> >      if (worker->record_fd) {
> >          dispatcher_register_universal_handler(dispatcher,
> >          worker_dispatcher_record);
> >      }
> > -    worker->image_compression = image_compression;
> > -    worker->jpeg_state = jpeg_state;
> > -    worker->zlib_glz_state = zlib_glz_state;
> >      worker->driver_cap_monitors_config = 0;
> >      stat_init(&worker->add_stat, add_stat_name);
> >      stat_init(&worker->exclude_stat, exclude_stat_name);
> > --
> > 2.5.0
> > 
> 
> Frediano