Re: [PATCH spice-server] stream-channel: Implements monitors_config

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> > On 19 Mar 2018, at 11:39, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> > 
> >> 
> >> On 03/13/2018 08:21 AM, Frediano Ziglio wrote:
> >>> Although not necessary for a single monitor DisplayChannel implementation
> >>> this make the DiisplayChannels more coherent from the client
> >>> point of view.
> >>> 
> >>> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> >>> ---
> >>>  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,
> >> 
> >> Hi Frediano,
> >> 
> >> Why do you send id=0 ?
> >> 
> >> Uri.
> >> 
> > 
> > Too much IDs :-)
> > These IDs are allocated starting from 0 and are per channel so as there
> > is only a monitor (currently) the ID should be 0.
> > So this id should not be confused with QXL ID or client/guest ID.
> 
> If Uri is confused, a comment may be in order.
> 
> Why so many IDs? Is it just to avoid an ID lookup during initialization?
> 

Not getting the lookup.

We have a global idea of the "monitor ID" which is quite confusing and
prone to different issues. The DisplayChannel (protocol) use channel IDs
(allocated per channel type starting from 0) and head IDs (allocated per
channel starting from 0). The client and vdagent use the global monitor ID.
The client craft these monitor IDs using basically the formula
"channel ID ? channel ID : head ID" and assuming the vdagent uses the same
IDs. Currently there are different problems of these assumption/schema.

> > 
> > Frediano
> > 
> >>> +            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);
> >>>      }
> >>> 

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]