[Spice-devel] [PATCH spice-server] stream-channel: Implements monitors_config

[Frediano Ziglio]

> 
> On Tue, 2018-03-13 at 06:21 +0000, Frediano Ziglio wrote:
> > Although not necessary for a single monitor DisplayChannel implementation
> > this make the DiisplayChannels more coherent from the client
> > point of view.
> 
> Typo: "DiisplayChannels"
> 
> I've tested this, as expected, with the heads attribute of the qxl
> device (in the xml) greater than 1, this causes an immediate round trip
> of monitors_config making the server create a second monitor in the
> guest and subsequently the streaming agent streaming two desktops next
> to each other.
> 
> Small improvement, a lot of work left to do. I would add a similar
> description of the current rather lacking state of things to the commit
> message. With that,
> 

This patch was not intended to fix this.
Do you have a suggestion for the comment I should add?

> Acked-by: Lukáš Hrázký <lhrazky at redhat.com>
> 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/stream-channel.c | 33 ++++++++++++++++++++++++++++++---
> >  1 file changed, 30 insertions(+), 3 deletions(-)
> > 
> > diff --git a/server/stream-channel.c b/server/stream-channel.c
> > index 3a3b733f..a9882d9c 100644
> > --- a/server/stream-channel.c
> > +++ b/server/stream-channel.c
> > @@ -102,6 +102,7 @@ enum {
> >      RED_PIPE_ITEM_TYPE_STREAM_DATA,
> >      RED_PIPE_ITEM_TYPE_STREAM_DESTROY,
> >      RED_PIPE_ITEM_TYPE_STREAM_ACTIVATE_REPORT,
> > +    RED_PIPE_ITEM_TYPE_MONITORS_CONFIG,
> >  };
> >  
> >  typedef struct StreamCreateItem {
> > @@ -204,6 +205,26 @@ fill_base(SpiceMarshaller *m, const StreamChannel
> > *channel)
> >      spice_marshall_DisplayBase(m, &base);
> >  }
> >  
> > +static void
> > +marshall_monitors_config(RedChannelClient *rcc, StreamChannel *channel,
> > SpiceMarshaller *m)
> > +{
> > +    struct {
> > +        SpiceMsgDisplayMonitorsConfig config;
> > +        SpiceHead head;
> > +    } msg = {
> > +        { 1, 1, },
> > +        {
> > +            0, PRIMARY_SURFACE_ID,
> > +            channel->width, channel->height,
> > +            0, 0,
> > +            0 // flags
> > +        }
> > +    };
> > +
> > +    red_channel_client_init_send_data(rcc,
> > SPICE_MSG_DISPLAY_MONITORS_CONFIG);
> > +    spice_marshall_msg_display_monitors_config(m, &msg.config);
> > +}
> > +
> >  static void
> >  stream_channel_send_item(RedChannelClient *rcc, RedPipeItem *pipe_item)
> >  {
> > @@ -229,6 +250,12 @@ stream_channel_send_item(RedChannelClient *rcc,
> > RedPipeItem *pipe_item)
> >          spice_marshall_msg_display_surface_create(m, &surface_create);
> >          break;
> >      }
> > +    case RED_PIPE_ITEM_TYPE_MONITORS_CONFIG:
> > +        if (!red_channel_client_test_remote_cap(rcc,
> > SPICE_DISPLAY_CAP_MONITORS_CONFIG)) {
> > +            return;
> > +        }
> > +        marshall_monitors_config(rcc, channel, m);
> > +        break;
> >      case RED_PIPE_ITEM_TYPE_SURFACE_DESTROY: {
> >          red_channel_client_init_send_data(rcc,
> >          SPICE_MSG_DISPLAY_SURFACE_DESTROY);
> >          SpiceMsgSurfaceDestroy surface_destroy = { PRIMARY_SURFACE_ID };
> > @@ -397,7 +424,6 @@ stream_channel_connect(RedChannel *red_channel,
> > RedClient *red_client, RedStream
> >      request_new_stream(channel, start);
> >  
> >  
> > -    // TODO set capabilities like  SPICE_DISPLAY_CAP_MONITORS_CONFIG
> >      // see guest_set_client_capabilities
> >      RedChannelClient *rcc = RED_CHANNEL_CLIENT(client);
> >      red_channel_client_push_set_ack(rcc);
> > @@ -415,6 +441,7 @@ stream_channel_connect(RedChannel *red_channel,
> > RedClient *red_client, RedStream
> >  
> >      // pass proper data
> >      red_channel_client_pipe_add_type(rcc,
> >      RED_PIPE_ITEM_TYPE_SURFACE_CREATE);
> > +    red_channel_client_pipe_add_type(rcc,
> > RED_PIPE_ITEM_TYPE_MONITORS_CONFIG);
> >      // surface data
> >      red_channel_client_pipe_add_type(rcc,
> >      RED_PIPE_ITEM_TYPE_FILL_SURFACE);
> >      // TODO monitor configs ??
> > @@ -433,8 +460,7 @@ stream_channel_constructed(GObject *object)
> >      client_cbs.connect = stream_channel_connect;
> >      red_channel_register_client_cbs(red_channel, &client_cbs, NULL);
> >  
> > -    // TODO, send monitor to support multiple monitors in the future
> > -//    red_channel_set_cap(red_channel, SPICE_DISPLAY_CAP_MONITORS_CONFIG);
> > +    red_channel_set_cap(red_channel, SPICE_DISPLAY_CAP_MONITORS_CONFIG);
> >      red_channel_set_cap(red_channel, SPICE_DISPLAY_CAP_STREAM_REPORT);
> >  
> >      reds_register_channel(reds, red_channel);
> > @@ -489,6 +515,7 @@ stream_channel_change_format(StreamChannel *channel,
> > const StreamMsgFormat *fmt)
> >          channel->width = fmt->width;
> >          channel->height = fmt->height;
> >          red_channel_pipes_add_type(red_channel,
> >          RED_PIPE_ITEM_TYPE_SURFACE_CREATE);
> > +        red_channel_pipes_add_type(red_channel,
> > RED_PIPE_ITEM_TYPE_MONITORS_CONFIG);
> >          // TODO monitors config ??
> >          red_channel_pipes_add_empty_msg(red_channel,
> >          SPICE_MSG_DISPLAY_MARK);
> >      }
>