[Spice-devel] [PATCH spice-protocol 1/8] Add the VDAgentGraphicsDeviceInfo message

[Lukáš Hrázký]

Hi,

On Thu, 2019-01-10 at 14:13 -0600, Jonathon Jongsma wrote:
> On Tue, 2019-01-08 at 16:28 +0100, Lukáš Hrázký wrote:
> > The message serves for passing the device address and device display
> > ID
> > information for all display channels from SPICE server to the
> > vd_agent.
> > 
> > Signed-off-by: Lukáš Hrázký <lhrazky at redhat.com>
> > ---
> >  spice/vd_agent.h | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/spice/vd_agent.h b/spice/vd_agent.h
> > index dda7044..5e618b7 100644
> > --- a/spice/vd_agent.h
> > +++ b/spice/vd_agent.h
> > @@ -91,6 +91,7 @@ enum {
> >      VD_AGENT_CLIENT_DISCONNECTED,
> >      VD_AGENT_MAX_CLIPBOARD,
> >      VD_AGENT_AUDIO_VOLUME_SYNC,
> > +    VD_AGENT_GRAPHICS_DEVICE_INFO,
> >      VD_AGENT_END_MESSAGE,
> >  };
> >  
> > @@ -248,6 +249,19 @@ typedef struct SPICE_ATTR_PACKED
> > VDAgentAudioVolumeSync {
> >      uint16_t volume[0];
> >  } VDAgentAudioVolumeSync;
> >  
> > +typedef struct SPICE_ATTR_PACKED VDAgentDeviceDisplayInfo {
> > +    uint32_t channel_id;
> > +    uint32_t monitor_id;
> > +    uint32_t device_display_id;
> > +    uint32_t device_address_len;
> > +    uint8_t device_address[0];  // a zero-terminated string
> > +} VDAgentDeviceDisplayInfo;
> > +
> > +typedef struct SPICE_ATTR_PACKED VDAgentGraphicsDeviceInfo {
> > +    uint32_t count;
> > +    VDAgentDeviceDisplayInfo device_info[0];
> 
> Just had a thought. I wonder if we should change this member name.
> Given that the type VDAgentGraphicsDeviceInfo ends with the phrase
> "DeviceInfo", a variable of this type is fairly likely to be named
> something like "device_info". In that case, we'd have code like
> device_info.device_info[0], which is a little awkward. 
> 
> Since the type of the member ends with "DisplayInfo", maybe it could be
> something like this instead?
> 
>  VDAgentDeviceDisplayInfo display_info[0]
> 
> Or device_display_info? though that gets a bit too long. If you prefer
> to keep it device_info, I won't argue.

Yes, display_info makes more sense and I think it was actually my
initial intention, but I got confused with the names myself (renaming
it a couple of times as well...). I'll change it.

Thanks,
Lukas

> Jonathon
> 
> > +} VDAgentGraphicsDeviceInfo;
> > +
> >  enum {
> >      VD_AGENT_CAP_MOUSE_STATE = 0,
> >      VD_AGENT_CAP_MONITORS_CONFIG,
> > @@ -264,6 +278,7 @@ enum {
> >      VD_AGENT_CAP_MONITORS_CONFIG_POSITION,
> >      VD_AGENT_CAP_FILE_XFER_DISABLED,
> >      VD_AGENT_CAP_FILE_XFER_DETAILED_ERRORS,
> > +    VD_AGENT_CAP_GRAPHICS_DEVICE_INFO,
> >      VD_AGENT_END_CAP,
> >  };
> >  
> 
>