[Spice-devel] [RFC/POC PATCH 00/16] add output_id to monitors_config

[Marc-André Lureau]

Hi

On Tue, Jun 5, 2018 at 5:30 PM, Lukáš Hrázký <lhrazky at redhat.com> wrote:
> Hi everyone,
>
> following is my attempt at solving the ID issues with monitors_config
> and streaming. The concept is as follows:
>

Before introducing a new solution, could you describe the "issues with
monitors_config and streaming"? I am not following closely enough the
mailing list probably, so a recap would be welcome. I'd like to
understand the shortcomings of the current messages, and see if we are
on the same page about it. Thanks!

> 1. The streaming-agent sends a new StreamInfo message when it starts
> streaming. The message contains the output_id of the streamed monitor.
> The actual value is the index in the list of xrandr outputs. Basically,
> the number should uniquely identify a monitor in the guest context under
> X.
>
> 2. The server copies the number into the SpiceMsgDisplayMonitorsConfig
> message and sends it to the client as part of the monitors_config
> exchange.
>
> 3. The client stores the output_id in it's list of monitor_configs. Here
> I had to make significant changes, because the monitors_config code in
> spice-gtk is very loosely written and also exposes quite a few
> unnecessary details to the client app. The client sends the output_id
> back to the server as part of the monitors_config messages
> (VDAgentMonitorsConfigV2) and also uses it for the mouse motion event,
> which also contains a display ID interpreted by the vd_agent. In the
> end, the API/ABI towards the client app should remain unchanged.
>
> 4. The server passes the output_id in above-mentioned messages to the
> vd_agent. The output_id is meaningless in the server context.
> (Currently, it doesn't pass the monitors_config messages if there is a
> QXL device that supports it, though. Needs more work.)
>
> 5. vd_agent:
>   a) For mouse input, the output_id was passed in the original message,
>   so no change needed here, it works.
>
>   b) If the server sends monitors_config to the guest, the vdagent will
>   prefer to use monitors_configs with the output_ids set, if such are
>   present. In that case, it ignores monitors_configs with the output_id
>   unset. If no output_ids are present, it should behave as it used to.
>
> A couple of things to note:
> - While I did copy the VDAgentMonitorsConfig to a different message for
> backwards compatibility, I didn't do the same for
> SpiceMsgDisplayMonitorsConfig, it remains to be done.
>
> - I didn't introduce any capabilities to handle the compatibility, also
> remains to be done. Hopefully it is also clear it will be quite a
> non-trivial job to do that :( Ain't gonna make the code prettier either.
>
> For your convenience, you can also pull the branches below:
> https://gitlab.freedesktop.org/lukash/spice-protocol/tree/monitors-config-poc
> https://gitlab.freedesktop.org/lukash/spice-common/tree/monitors-config-poc
> https://gitlab.com/lhrazky/spice-streaming-agent/tree/monitors-config-poc
> https://gitlab.freedesktop.org/lukash/spice/tree/monitors-config-poc
> https://gitlab.freedesktop.org/lukash/spice-gtk/tree/monitors-config-poc
> https://gitlab.freedesktop.org/lukash/vd_agent/tree/monitors-config-poc
>
> All in all, it's big, complex and invasive. Note I did review the
> emergency instructional video [1] and am therefore ready for any
> bombardment you'll be sending my way :D (Don't hesitate to contact me
> with questions either)
>
> Last minute note: I've noticed some of the patches are missing
> Signed-off-by line, since they are not for merging, should not be an
> issue...
>
>
> Lukáš Hrázký (16):
> spice-protocol
>   Add the StreamInfo message
>   Create a version 2 of the VDAgentMonitorsConfig message
> spice-common
>   add output_id to SpiceMsgDisplayMonitorsConfig
> spice-streaming-agent
>   Send a StreamInfo message when starting to stream
> spice-server
>   Handle the StreamInfo message from the streaming agent
>   Use VDAgentMonitorsConfigV2 that contains the output_id field
> spice-gtk
>   Rework the handling of monitors_config
>   Remove the n_display_channels check when sending monitors_config
>   Use an 'enabled' flag instead of status enum in monitors_config
>   Use VDAgentMonitorsConfigV2 as the message for monitors_config
>   Add output_id to the monitors_config
>   Use the new output_id as display ID for the mouse motion event
> vd_agent
>   vdagent: Log the diddly doo X11 error
>   Improve/add some logging messages
>   Use VDAgentMonitorsConfigV2 instead of VDAgentMonitorsConfig
>   vdagent: Use output_id from VDAgentMonitorsConfigV2
>
> [1] https://www.youtube.com/watch?v=IKqXu-5jw60
>
> --
> 2.17.1
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



-- 
Marc-André Lureau