On Wed, 2018-06-06 at 04:36 -0400, Frediano Ziglio wrote:
> >
> > On Tue, 2018-06-05 at 17:30 +0200, Lukáš Hrázký wrote:
> > > The message is meant to contain information related to the stream,
> > > e.g.
> > > now the guest (xrandr) output ID of the streamed monitor.
> > >
> > > Signed-off-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
> > > ---
> > > spice/stream-device.h | 15 +++++++++++++++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/spice/stream-device.h b/spice/stream-device.h
> > > index 6add42b..d004d0a 100644
> > > --- a/spice/stream-device.h
> > > +++ b/spice/stream-device.h
> > > @@ -90,6 +90,8 @@ typedef enum StreamMsgType {
> > > STREAM_TYPE_CURSOR_SET,
> > > /* guest cursor position */
> > > STREAM_TYPE_CURSOR_MOVE,
> > > + /* the stream information message */
> > > + STREAM_TYPE_INFO,
> > > } StreamMsgType;
> > >
> > > typedef enum StreamCapabilities {
> > > @@ -140,6 +142,19 @@ typedef struct StreamMsgData {
> > > uint8_t data[0];
> > > } StreamMsgData;
> > >
> > > +/* Message containing information about the stream, sent when a new
> > > stream is created.
> > > + */
> > > +typedef struct StreamMsgInfo {
> > > + /* The xrandr output ID (index of the output in the list) that
> > > is being
> > > + * streamed. Sequence starts from 1. Value of 0 makes no sense
> > > here, but
> >
> > Hmm, so in theory the id is determined by the guest (e.g. xorg/xrandr
> > output ids). So is it safe to specify valid values in the protocol
> > definition? What if in the future we have e.g. a wayland guest for
> > which 0 is a valid guest display ID?
As Frediano mentioned, I already do the +1 approach since the IDs on X
are a sequence starting from 0. If on Wayland the IDs were also a
sequence starting from 0, we can just do the same. I'd only have issue
with the +1 approach if 0 was a valid ID but the numbers actually
weren't a sequence but for some reason were 'scattered'. Then it would
get messy. But it's usually a good practice (:)), if you have non-
sequence IDs, to leave out 0 as a special 'Null' value.
>
> I would say there are usually 2 solutions:
> 1- add and subtract 1, if I count from 0 and you count from 0 my 2 is
> your 1;
> 2- use a different value for invalid (as the maximum possible value for
> unsigned).
> I like to test for invalid with a if (!value) so I like Lukash "0 is invalid
> index starts with 1" (solution 1).
It's definitely a point for discussion. I had to do a quite unexpected
'- 1' in the mouse motion event...
> >
> > > + * the ID is passed from server to client and back to the server
> > > and
> > > + * vd_agent in the MonitorsConfig messages. In those messages
> > > value of 0
> > > + * means the output_id is unset, meaning the monitors are
> > > regular,
> > > + * non-streamed from the streaming agent.
> > > + */
> > > + uint32_t output_id;
> > > +} StreamMsgInfo;
> > > +
> > > /* Tell to stop current stream and possibly start a new one.
> > > * This message is sent by the host to the guest.
> > > * Allows to communicate the codecs supported by the clients.
>
> uint32_t is fine for me, I hardly see having thousands of monitors
> and probably the value will be truncated. For instance the "display_id"
> field in VDAgentMouseState is 8 bit. The count in QXLMonitorsConfig
> is 16 bits (but the QXLHead id is 32, not really consistent), like
> in SpiceMsgDisplayMonitorsConfig.
Not sure how much it matters, but I'm open to suggestions to what type
to use...
> But potentially the guest could use
> the upper bits to store some informations (beside mouse "display_id"
> the value is untouched).
I'd be opposed to doing that if it's avoidable, I don't think we need
to be at such a low level here. But then again, since the protocol is
so inflexible... :D
Cheers,
Lukas
> Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel