Re: [Spice-devel] [PATCH spice-protocol 1/8] Add the VDAgentGraphicsDeviceInfo message
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ý
> > ---
> > 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,
> > };
> >
>
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol 1/8] Add the VDAgentGraphicsDeviceInfo message
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ý
> ---
> 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.
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,
> };
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol 1/8] Add the VDAgentGraphicsDeviceInfo message
On Wed, 2019-01-09 at 06:30 -0500, Frediano Ziglio wrote:
> >
> > On Wed, 2019-01-09 at 02:41 -0500, Frediano Ziglio 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ý
> > > > ---
> > > > 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;
> > >
> > > Other parts of the SPICE protocol use 8 bit for these.
> > > Do you want to extend them in the future?
> > > Maybe better to put some comment on possible limits?
> >
> > channel_id is 8 bits on the client <-> server line, monitor_id is 32
>
> Yes, confused for monitor_id.
>
> > bits though. What comment on limits? You need to use existing valid
> > IDs, otherwise it won't work. I don't see any other limitation.
> >
> > I can change them (or just channel_id) to 8 bits if you want... I'd
> > just leave them like this myself though.
> >
>
> or a comment like "// currently limited to 255".
Ok, but I find no value in a comment like this. The fact that a valid
ID will atm. never be greater than 255 does not really impose a limit
here. If you are putting an ID greater than 255 in here, you are
putting a non-existent ID, but that's all. There are always non-
existent IDs that will not work. No difference.
> > > > +uint32_t device_display_id;
> > > > +uint32_t device_address_len;
> > > > +uint8_t device_address[0]; // a zero-terminated string
> > >
> > > Similar comments for spice-protocol for these last 2 fields.
> >
> > Not sure what you mean by this...
> >
>
> Comments on patch 2/8.
About zero-termination? The length is actually needed here.
> > Cheers,
> > Lukas
> >
> > > > +} VDAgentDeviceDisplayInfo;
> > > > +
> > > > +typedef struct SPICE_ATTR_PACKED VDAgentGraphicsDeviceInfo {
> > > > +uint32_t count;
> > > > +VDAgentDeviceDisplayInfo device_info[0];
> > > > +} 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,
> > > > };
> > > >
> > >
> > > Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol 1/8] Add the VDAgentGraphicsDeviceInfo message
>
> On Wed, 2019-01-09 at 02:41 -0500, Frediano Ziglio 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ý
> > > ---
> > > 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;
> >
> > Other parts of the SPICE protocol use 8 bit for these.
> > Do you want to extend them in the future?
> > Maybe better to put some comment on possible limits?
>
> channel_id is 8 bits on the client <-> server line, monitor_id is 32
Yes, confused for monitor_id.
> bits though. What comment on limits? You need to use existing valid
> IDs, otherwise it won't work. I don't see any other limitation.
>
> I can change them (or just channel_id) to 8 bits if you want... I'd
> just leave them like this myself though.
>
or a comment like "// currently limited to 255".
> > > +uint32_t device_display_id;
> > > +uint32_t device_address_len;
> > > +uint8_t device_address[0]; // a zero-terminated string
> >
> > Similar comments for spice-protocol for these last 2 fields.
>
> Not sure what you mean by this...
>
Comments on patch 2/8.
> Cheers,
> Lukas
>
> > > +} VDAgentDeviceDisplayInfo;
> > > +
> > > +typedef struct SPICE_ATTR_PACKED VDAgentGraphicsDeviceInfo {
> > > +uint32_t count;
> > > +VDAgentDeviceDisplayInfo device_info[0];
> > > +} 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,
> > > };
> > >
> >
> > Frediano
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol 1/8] Add the VDAgentGraphicsDeviceInfo message
On Wed, 2019-01-09 at 02:41 -0500, Frediano Ziglio 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ý
> > ---
> > 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;
>
> Other parts of the SPICE protocol use 8 bit for these.
> Do you want to extend them in the future?
> Maybe better to put some comment on possible limits?
channel_id is 8 bits on the client <-> server line, monitor_id is 32
bits though. What comment on limits? You need to use existing valid
IDs, otherwise it won't work. I don't see any other limitation.
I can change them (or just channel_id) to 8 bits if you want... I'd
just leave them like this myself though.
> > +uint32_t device_display_id;
> > +uint32_t device_address_len;
> > +uint8_t device_address[0]; // a zero-terminated string
>
> Similar comments for spice-protocol for these last 2 fields.
Not sure what you mean by this...
Cheers,
Lukas
> > +} VDAgentDeviceDisplayInfo;
> > +
> > +typedef struct SPICE_ATTR_PACKED VDAgentGraphicsDeviceInfo {
> > +uint32_t count;
> > +VDAgentDeviceDisplayInfo device_info[0];
> > +} 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,
> > };
> >
>
> Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol 1/8] Add the VDAgentGraphicsDeviceInfo message
>
> 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ý
> ---
> 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;
Other parts of the SPICE protocol use 8 bit for these.
Do you want to extend them in the future?
Maybe better to put some comment on possible limits?
> +uint32_t device_display_id;
> +uint32_t device_address_len;
> +uint8_t device_address[0]; // a zero-terminated string
Similar comments for spice-protocol for these last 2 fields.
> +} VDAgentDeviceDisplayInfo;
> +
> +typedef struct SPICE_ATTR_PACKED VDAgentGraphicsDeviceInfo {
> +uint32_t count;
> +VDAgentDeviceDisplayInfo device_info[0];
> +} 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,
> };
>
Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-protocol 1/8] Add the VDAgentGraphicsDeviceInfo message
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ý
---
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];
+} 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,
};
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
