Re: [RFC PATCH spice-protocol v2 03/20] Add the StreamInfo message

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2018-08-16 at 18:26 +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@xxxxxxxxxx>
> ---
>  spice/stream-device.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/spice/stream-device.h b/spice/stream-device.h
> index 6add42b..be22e06 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,15 @@ 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, 0-based.
> +     */
> +    uint32_t guest_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.


So, here's where our work starts to overlap a bit (and why it's a bit
unfortunate that I haven't gotten around to sending out a proposal for
multi-monitor support so far, perhaps). 

The change above assumes that there will be a single output for a
single stream device. This is currently true because we only support a
single output. For a variety of reasons, I've chosen to implement
multimonitor support by multiplexing multiple outputs over a single
stream device. I'll share my current working patch for the stream
device protocol below. 

But first I'll try to explain why I chose to multiplex displays over a
single stream device. In general, I think it makes things easier. Some
examples:

 * if you use one stream device per output, you need to decide before
   you launch the vm how many outputs it will support and allocate
   those devices by passing the correct options to qemu. To add an
   additional output, you need to re-start qemu with different
   parameters.
 * We currently use a hard-coded spice port name (org.spice-
   space.stream.0). If we have a separate stream device for each
   display, we will have multiple port names. How do those port names
   get communicated from the server to the guest?
 * If you have multiple stream devices, do we launch a separate spice-
   streaming-agent process for each one? Or a single streaming agent
   that handles all of them?
 * You also have to decide how to handle odd issues such as the fact
   that the driver may be perfectly capable of enabling additional
   monitors, but we don't have sufficient stream devices for them. So
   what do you do when the vm was configured with only a single spice
   device, but the user enables an additional display from within the
   guest? Now perhaps the guest has two displays enabled, but we have
   no way of sending that second display to the client.

So there were a lot of little things like this that led me to decide
that using a single stream device was simpler and more flexible. There
may have been other reasons that I've forgotten already as well. 

The only real downside I could think of was that the stream device
protocol would have to change in a non-backward-compatible way (see
below). But since the streaming agent is still in early stages, that
doesn't seem like a huge deal. Of course, it's possible that I missed
other disadvantages, so let me know any problems you see with this
approach.

Anyway, here's the current proof-of-concept changes to the stream
device protocol that I'm working with:


diff --git a/spice/stream-device.h b/spice/stream-device.h
index 6add42b..57c768f 100644
--- a/spice/stream-device.h
+++ b/spice/stream-device.h
@@ -58,7 +58,7 @@
  */
 
 /* version of the protocol */
-#define STREAM_DEVICE_PROTOCOL 1
+#define STREAM_DEVICE_PROTOCOL 2
 
 typedef struct StreamDevHeader {
     /* should be STREAM_DEVICE_PROTOCOL */
@@ -78,8 +78,8 @@ typedef enum StreamMsgType {
     STREAM_TYPE_INVALID = 0,
     /* allows to send version information */
     STREAM_TYPE_CAPABILITIES,
-    /* send screen resolution */
-    STREAM_TYPE_FORMAT,
+    /* send guest output configuration */
+    STREAM_TYPE_OUTPUT_CONFIG,
     /* stream data */
     STREAM_TYPE_DATA,
     /* server ask to start a new stream */
@@ -115,6 +115,16 @@ typedef struct StreamMsgCapabilities {
 
 #define STREAM_MSG_CAPABILITIES_MAX_BYTES 1024
 
+typedef struct StreamMsgOutput {
+    uint8_t output_id;
+    /* screen resolution/stream size */
+    uint32_t width;
+    uint32_t height;
+    /* coordinates of the top-left corner of the screen */
+    int32_t x;
+    int32_t y;
+} StreamMsgOutput;
+
 /* Define the format of the stream, start a new stream.
  * This message is sent by the guest to the host to
  * tell the host the new stream format.
@@ -122,21 +132,21 @@ typedef struct StreamMsgCapabilities {
  * States allowed: Ready
  *   state will change to Streaming.
  */
-typedef struct StreamMsgFormat {
-    /* screen resolution/stream size */
-    uint32_t width;
-    uint32_t height;
+typedef struct StreamMsgOutputConfig {
     /* as defined in SpiceVideoCodecType enumeration */
     uint8_t codec;
-    uint8_t padding1[3];
-} StreamMsgFormat;
+    uint8_t n_outputs;
+    StreamMsgOutput outputs[0];
+} StreamMsgOutputConfig;
+
 
-/* This message contains just raw data stream.
+/* This message contains just raw data stream for the specified
output.
  * This message is sent by the guest to the host.
  *
  * States allowed: Streaming
  */
 typedef struct StreamMsgData {
+    uint8_t output_id;
     uint8_t data[0];
 } StreamMsgData;
 


Jonathon
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]