[Spice-devel] [RFC/POC PATCH spice-protocol 01/16] Add the StreamInfo message

[Frediano Ziglio]

> 
> 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 at redhat.com>
> > ---
> >  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?
> 

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).

> 
> > +     * 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. But potentially the guest could use
the upper bits to store some informations (beside mouse "display_id"
the value is untouched).

Frediano