On Thu, 2019-01-17 at 13:10 -0600, Jonathon Jongsma wrote:
> On Wed, 2019-01-16 at 13:52 +0100, Lukáš Hrázký wrote:
> > Adds the graphics device info from the streaming device(s) to the
> > VDAgentGraphicsDeviceInfo message sent to the vd_agent.
> >
> > Signed-off-by: Lukáš Hrázký <lhra...@redhat.com>
> > ---
> > server/red-stream-device.c | 18 +++++++++++
> > server/red-stream-device.h | 7 +++++
> > server/reds.c | 63
> > ++++++++++++++++++++++++++++++++++++--
> > 3 files changed, 86 insertions(+), 2 deletions(-)
> >
> > diff --git a/server/red-stream-device.c b/server/red-stream-device.c
> > index 6c90d77d..e7ed9f76 100644
> > --- a/server/red-stream-device.c
> > +++ b/server/red-stream-device.c
> > @@ -342,6 +342,8 @@ handle_msg_device_display_info(StreamDevice *dev,
> > SpiceCharDeviceInstance *sin)
> > dev->device_display_info.device_address,
> > dev->device_display_info.device_display_id);
> >
> > + reds_send_device_display_info(red_char_device_get_server(RED_CHA
> > R_DEVICE(dev)));
> > +
> > return true;
> > }
> >
> > @@ -799,6 +801,22 @@ stream_device_init(StreamDevice *dev)
> > dev->msg_len = sizeof(*dev->msg);
> > }
> >
> > +StreamDeviceDisplayInfo
> > *stream_device_get_device_display_info(StreamDevice *dev)
> > +{
> > + return &dev->device_display_info;
> > +}
> > +
> > +int32_t stream_device_get_stream_channel_id(StreamDevice *dev)
> > +{
> > + if (dev->stream_channel == NULL) {
> > + return -1;
> > + }
> > +
> > + int32_t channel_id;
> > + g_object_get(dev->stream_channel, "id", &channel_id, NULL);
> > + return channel_id;
> > +}
> > +
>
> Did you consider using a similar interface to the one used for the QXL
> displays? e.g. _get_device_address() / _get_device_display_ids() / etc?
>
> I know that the stream device currently (and probably always) only has
> a single display per channel, but part of me wishes the interfaces had
> some consistency. Not a big issue, just curious.
IIRC I did have a similar interface while I had it handle info for
multiple streams, but after I realized I'm not sure how multiple
streams will be implemented and dumbed it down to a single info
message, I simlified the interface as well. The interface also depends
on that. It is an interface internal to the server, so it can be
changed when multi-monitor support is implemented for streaming...
> Acked-by: Jonathon Jongsma <jjong...@redhat.com>
Thanks,
Lukas
> > static StreamDevice *
> > stream_device_new(SpiceCharDeviceInstance *sin, RedsState *reds)
> > {
> > diff --git a/server/red-stream-device.h b/server/red-stream-device.h
> > index 996be016..d7ab5e41 100644
> > --- a/server/red-stream-device.h
> > +++ b/server/red-stream-device.h
> > @@ -56,6 +56,13 @@ StreamDevice *stream_device_connect(RedsState
> > *reds, SpiceCharDeviceInstance *si
> > */
> > void stream_device_create_channel(StreamDevice *dev);
> >
> > +StreamDeviceDisplayInfo
> > *stream_device_get_device_display_info(StreamDevice *dev);
> > +
> > +/**
> > + * Returns -1 if the StreamDevice doesn't have a channel yet.
> > + */
> > +int32_t stream_device_get_stream_channel_id(StreamDevice *dev);
> > +
> > G_END_DECLS
> >
> > #endif /* STREAM_DEVICE_H */
> > diff --git a/server/reds.c b/server/reds.c
> > index 88a82419..22e81d73 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -902,14 +902,33 @@ void reds_send_device_display_info(RedsState
> > *reds)
> > }
> > g_debug("Sending device display info to the agent:");
> >
> > - size_t message_size = sizeof(VDAgentGraphicsDeviceInfo);
> > QXLInstance *qxl;
> > + RedCharDevice *dev;
> > +
> > + size_t message_size = sizeof(VDAgentGraphicsDeviceInfo);
> > +
> > + // size for the qxl device info
> > FOREACH_QXL_INSTANCE(reds, qxl) {
> > message_size +=
> > (sizeof(VDAgentDeviceDisplayInfo) +
> > strlen(red_qxl_get_device_address(qxl)) + 1) *
> > red_qxl_get_monitors_count(qxl);
> > }
> >
> > + // size for the stream device info
> > + GLIST_FOREACH(reds->char_devices, RedCharDevice, dev) {
> > + if (IS_STREAM_DEVICE(dev)) {
> > + size_t device_address_len =
> > + strlen(stream_device_get_device_display_info(STREAM_
> > DEVICE(dev))->device_address);
> > +
> > + if (device_address_len == 0) {
> > + // the device info wasn't set (yet), don't send it
> > + continue;
> > + }
> > +
> > + message_size += (sizeof(VDAgentDeviceDisplayInfo) +
> > device_address_len + 1);
> > + }
> > + }
> > +
> > RedCharDeviceWriteBuffer *char_dev_buf =
> > vdagent_new_write_buffer(reds->agent_dev,
> > VD_AGENT_GRAPHICS_DEVICE_IN
> > FO,
> > message_size,
> > @@ -925,6 +944,8 @@ void reds_send_device_display_info(RedsState
> > *reds)
> > graphics_device_info->count = 0;
> >
> > VDAgentDeviceDisplayInfo *device_display_info =
> > graphics_device_info->display_info;
> > +
> > + // add the qxl devices to the message
> > FOREACH_QXL_INSTANCE(reds, qxl) {
> > for (size_t i = 0; i < red_qxl_get_monitors_count(qxl); ++i)
> > {
> > device_display_info->channel_id = qxl->id;
> > @@ -936,7 +957,45 @@ void reds_send_device_display_info(RedsState
> > *reds)
> > device_display_info->device_address_len =
> > strlen((char*) device_display_info->device_address)
> > + 1;
> >
> > - g_debug(" channel_id: %u monitor_id: %u,
> > device_address: %s, device_display_id: %u",
> > + g_debug(" (qxl) channel_id: %u monitor_id: %u,
> > device_address: %s, device_display_id: %u",
> > + device_display_info->channel_id,
> > + device_display_info->monitor_id,
> > + device_display_info->device_address,12th
> > Congressional District of Pennsylvania
> > + device_display_info->device_display_id);
> > +
> > + device_display_info = (VDAgentDeviceDisplayInfo*)
> > ((char*) device_display_info +
> > + sizeof(VDAgentDeviceDisplayInfo) +
> > device_display_info->device_address_len);
> > +
> > + graphics_device_info->count++;
> > + }
> > + }
> > +
> > + // add the stream devices to the message
> > + GLIST_FOREACH(reds->char_devices, RedCharDevice, dev) {
> > + if (IS_STREAM_DEVICE(dev)) {
> > + StreamDevice *stream_dev = STREAM_DEVICE(dev);
> > + StreamDeviceDisplayInfo *info =
> > stream_device_get_device_display_info(stream_dev);
> > + size_t device_address_len = strlen(info-
> > > device_address);
> >
> > +
> > + if (device_address_len == 0) {
> > + // the device info wasn't set (yet), don't send it
> > + continue;
> > + }
> > +
> > + int32_t channel_id =
> > stream_device_get_stream_channel_id(stream_dev);
> > + if (channel_id == -1) {
> > + g_warning("DeviceDisplayInfo set but no stream
> > channel exists");
> > + continue;
> > + }
> > +
> > + device_display_info->channel_id = channel_id;
> > + device_display_info->monitor_id = info->stream_id;
> > + device_display_info->device_display_id = info-
> > > device_display_id;
> >
> > +
> > + strcpy((char*) device_display_info->device_address,
> > info->device_address);
> > + device_display_info->device_address_len =
> > device_address_len + 1;
> > +
> > + g_debug(" (stream) channel_id: %u monitor_id: %u,
> > device_address: %s, device_display_id: %u",
> > device_display_info->channel_id,
> > device_display_info->monitor_id,
> > device_display_info->device_address,
>
>
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
