Re: [RFC PATCH v2 00/20] Monitor ID rework

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

 



> On Mon, Aug 27, 2018 at 03:34:54PM +0200, Lukáš Hrázký wrote:
> > On Mon, 2018-08-27 at 14:27 +0200, Gerd Hoffmann wrote:
> > >   Hi,
> > > 
> > > > 1. The IDs sent from the client in VDAgentMonitorsConfig and
> > > > MousePosition messages are equal to either `channel_id + monitor_id` or
> > > > `channel_id ? channel_id : monitor_id`. This is under the assumption
> > > > that there is either only one display_channel or more display channels
> > > > each with only a single monitor. Under this assumption the
> > > > aforementioned expressions are equivalent. It is not a reliable way to
> > > > identify monitors though.
> > > 
> > > Can we define this to be "channel_id + monitor_id", then use sparse
> > > channel ids?  qxl has room for up to 16 monitors in the protocol
> > > messages.  channel_id is 8 bit.  So we could support up to 16 display
> > > devices with up to 16 outputs each if we enumerate the display channels
> > > 0, 16, 32 instead of 0, 1, 2.
> > 
> > While I suppose technically possible, a couple issues:
> > 
> > 1. It is AFAICS not backwards compatible anyway (you don't change the
> > protocol itself, but you change the content):
> > 
> > channel_id | monitor_id | old display_id | your display_id
> > ----------------------------------------------------------
> >          0 |          0 |              0 |               0
> >          1 |          0 |              1 |              16
> >          2 |          0 |              2 |              32
> > ...
> 
> No.  The idea is to change the channel id, not the math to calculate
> the display id.
> 
> current scheme:
> 
>    channel  monitor  display
>      0         0       0
>      0         1       1    <== Oops, two displays
>      1         0       1    <== with the same ID.
>      1         1       2
> 
> suggested scheme:
> 
>    channel  monitor  display
>      0         0       0
>      0         1       1
>     16         0      16
>     16         1      17
> 

What you are proposing is basically, instead of

struct {
  ...
  uint8_t display_id;
  ...
};

to have

struct {
  ...
  uint8_t channel_id:4;
  uint8_t monitor_id:4;
  ...
};

at this point however both the client and server have to be changed,
this new schema is just introducing a limitation (16 channels and 16
monitors) which you won't have using 2 different fields.
And to keep the spice-gtk API unchanged you would have to keep the
old display_id.

> Maybe it is better to make the size of the gaps depend on the number of
> monitors a channel has, i.e. if channel 0 is a qxl device with up to
> four outputs configured we skip channels 1+2+3 and the next display
> device gets channel 4.  We don't have holes in the display numbering
> then.
> 

However you don't know the number of monitors (client) till you
receive a specific message so at the beginning you don't know
the information you need (on the client) and to fix this you would
have to change the protocol to send it earlier.

> > 2. I don't want to do this, while it's better than simple channel_id +
> > monitor_id, it's still unnecessary to encode the numbers this way, two
> > separate fields are much cleaner.
> 
> Sure, two fields are cleaner.
> 
> > (and with it not saving us
> > compatibility trouble...)
> 
> Well, I hope it *does* save us the compatibility trouble, that is the
> whole point.  If if doesn't work out because there assumtions in the
> code base about the way channels are numbered, then scratch the idea.
> 

No, there are not assumption on channel numbering. But there are on
the channel/monitor combination.

> > (on a side note, I'm still assimilating the information you shared in
> > the other emails and thinking about ways to move forward, that's why
> > I'm slow on the replies...)
> 
> Sure.  Take your time and feel free to ask if you need more information.
> 
> cheers,
>   Gerd
> 

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




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