[Spice-devel] [RFC PATCH spice-server v3 08/20] stream-device: Handle streaming data from device to channel

Jonathon Jongsma jjongsma at redhat.com
Thu Aug 24 21:33:36 UTC 2017


On Wed, 2017-08-23 at 10:14 +0100, Frediano Ziglio wrote:
> Handle stream data from device sending to the channel.
> Channel will forward to clients as proper DisplayChannel
> messaging creating and destroying the channel stream as
> needed.

This last sentence is a bit unclear. Perhaps

"The StreamChannel will forward the data to the clients using standard
DisplayChannel messages, and will create and destroy streams as
necessary".

> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/stream-channel.c | 108
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  server/stream-channel.h |   8 ++++
>  server/stream-device.c  |   4 +-
>  3 files changed, 118 insertions(+), 2 deletions(-)
> 
> diff --git a/server/stream-channel.c b/server/stream-channel.c
> index c57f7e89..4ab378a8 100644
> --- a/server/stream-channel.c
> +++ b/server/stream-channel.c
> @@ -20,11 +20,13 @@
>  #endif
>  
>  #include <common/generated_server_marshallers.h>
> +#include <spice/stream-device.h>
>  
>  #include "red-channel-client.h"
>  #include "stream-channel.h"
>  #include "reds.h"
>  #include "common-graphics-channel.h"
> +#include "display-limits.h"
>  
>  #define TYPE_STREAM_CHANNEL_CLIENT stream_channel_client_get_type()
>  
> @@ -46,6 +48,8 @@ typedef struct StreamChannelClientClass
> StreamChannelClientClass;
>   * to get buffer handling */
>  struct StreamChannelClient {
>      CommonGraphicsChannelClient parent;
> +
> +    int stream_id;
>  };
>  
>  struct StreamChannelClientClass {
> @@ -58,6 +62,10 @@ G_DEFINE_TYPE(StreamChannelClient,
> stream_channel_client, TYPE_COMMON_GRAPHICS_C
>  
>  struct StreamChannel {
>      RedChannel parent;
> +
> +    /* current video stream id, <0 if not initialized or
> +     * we are not sending a stream */
> +    int stream_id;
>  };
>  
>  struct StreamChannelClass {
> @@ -69,8 +77,22 @@ G_DEFINE_TYPE(StreamChannel, stream_channel,
> RED_TYPE_CHANNEL)
>  enum {
>      RED_PIPE_ITEM_TYPE_SURFACE_CREATE =
> RED_PIPE_ITEM_TYPE_COMMON_LAST,
>      RED_PIPE_ITEM_TYPE_FILL_SURFACE,
> +    RED_PIPE_ITEM_TYPE_STREAM_CREATE,
> +    RED_PIPE_ITEM_TYPE_STREAM_DATA,
> +    RED_PIPE_ITEM_TYPE_STREAM_DESTROY,
>  };
>  
> +typedef struct StreamCreateItem {
> +    RedPipeItem base;
> +    SpiceMsgDisplayStreamCreate stream_create;
> +} StreamCreateItem;
> +
> +typedef struct StreamDataItem {
> +    RedPipeItem base;
> +    // NOTE: this must be the last field in the structure
> +    SpiceMsgDisplayStreamData data;
> +} StreamDataItem;
> +
>  #define PRIMARY_SURFACE_ID 0
>  
>  static void
> @@ -81,6 +103,7 @@
> stream_channel_client_class_init(StreamChannelClientClass *klass)
>  static void
>  stream_channel_client_init(StreamChannelClient *client)
>  {
> +    client->stream_id = -1;
>  }
>  
>  static void
> @@ -122,6 +145,7 @@ static void
>  stream_channel_send_item(RedChannelClient *rcc, RedPipeItem
> *pipe_item)
>  {
>      SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
> +    StreamChannelClient *client = STREAM_CHANNEL_CLIENT(rcc);
>  
>      switch (pipe_item->type) {
>      case RED_PIPE_ITEM_TYPE_SURFACE_CREATE: {
> @@ -147,6 +171,32 @@ stream_channel_send_item(RedChannelClient *rcc,
> RedPipeItem *pipe_item)
>          spice_marshall_Fill(m, &fill, &brush_pat_out,
> &mask_bitmap_out);
>          break;
>      }
> +    case RED_PIPE_ITEM_TYPE_STREAM_CREATE: {
> +        StreamCreateItem *item = SPICE_UPCAST(StreamCreateItem,
> pipe_item);
> +        client->stream_id = item->stream_create.id;
> +        red_channel_client_init_send_data(rcc,
> SPICE_MSG_DISPLAY_STREAM_CREATE);
> +        spice_marshall_msg_display_stream_create(m, &item-
> >stream_create);
> +        break;
> +    }
> +    case RED_PIPE_ITEM_TYPE_STREAM_DATA: {
> +        StreamDataItem *item = SPICE_UPCAST(StreamDataItem,
> pipe_item);
> +        red_channel_client_init_send_data(rcc,
> SPICE_MSG_DISPLAY_STREAM_DATA);
> +        spice_marshall_msg_display_stream_data(m, &item->data);
> +        red_pipe_item_ref(pipe_item);
> +        spice_marshaller_add_by_ref_full(m, item->data.data, item-
> >data.data_size,
> +                                         marshaller_unref_pipe_item,
> pipe_item);
> +        break;
> +    }
> +    case RED_PIPE_ITEM_TYPE_STREAM_DESTROY: {
> +        if (client->stream_id < 0) {
> +            return;
> +        }
> +        SpiceMsgDisplayStreamDestroy stream_destroy = { client-
> >stream_id };
> +        red_channel_client_init_send_data(rcc,
> SPICE_MSG_DISPLAY_STREAM_DESTROY);
> +        spice_marshall_msg_display_stream_destroy(m,
> &stream_destroy);
> +        client->stream_id = -1;
> +        break;
> +    }
>      default:
>          spice_error("invalid pipe item type");
>      }
> @@ -255,4 +305,62 @@ stream_channel_class_init(StreamChannelClass
> *klass)
>  static void
>  stream_channel_init(StreamChannel *channel)
>  {
> +    channel->stream_id = -1;
> +}
> +
> +static RedPipeItem *
> +pipe_item_new_ref(G_GNUC_UNUSED RedChannelClient *rcc, void *data,
> G_GNUC_UNUSED int num)
> +{
> +    RedPipeItem *item = data;
> +    red_pipe_item_ref(item);
> +    return item;
> +}

How about a name like:

shared_pipe_item_ref()
pipe_item_new_shared()

?


> +
> +void
> +stream_channel_change_format(StreamChannel *channel, const
> StreamMsgFormat *fmt)
> +{
> +    RedChannel *red_channel = RED_CHANNEL(channel);
> +
> +    // send destroy old stream
> +    red_channel_pipes_add_type(red_channel,
> RED_PIPE_ITEM_TYPE_STREAM_DESTROY);
> +
> +    // TODO send new create surface if required
> +
> +    // allocate a new stream id
> +    channel->stream_id = (channel->stream_id + 1) % NUM_STREAMS;
> +
> +    // send create stream
> +    StreamCreateItem *item = spice_new0(StreamCreateItem, 1);
> +    red_pipe_item_init(&item->base,
> RED_PIPE_ITEM_TYPE_STREAM_CREATE);
> +    item->stream_create.id = channel->stream_id;
> +    item->stream_create.flags = SPICE_STREAM_FLAGS_TOP_DOWN;
> +    item->stream_create.codec_type = fmt->codec;
> +    item->stream_create.stream_width = fmt->width;
> +    item->stream_create.stream_height = fmt->height;
> +    item->stream_create.src_width = fmt->width;
> +    item->stream_create.src_height = fmt->height;
> +    item->stream_create.dest = (SpiceRect) { 0, 0, fmt->width, fmt-
> >height };
> +    item->stream_create.clip = (SpiceClip) { SPICE_CLIP_TYPE_NONE,
> NULL };
> +    red_channel_pipes_new_add(red_channel, pipe_item_new_ref, item);
> +    red_pipe_item_unref(&item->base);
> +}
> +
> +void
> +stream_channel_send_data(StreamChannel *channel, const void *data,
> size_t size, uint32_t mm_time)
> +{
> +    if (channel->stream_id < 0) {

print a warning here? Seems like sending stream data without creating a
stream is potentially a signal that something went wrong.

> +        return;
> +    }
> +
> +    RedChannel *red_channel = RED_CHANNEL(channel);
> +
> +    StreamDataItem *item = spice_malloc(sizeof(*item) + size);
> +    red_pipe_item_init(&item->base, RED_PIPE_ITEM_TYPE_STREAM_DATA);
> +    item->data.base.id = channel->stream_id;
> +    item->data.base.multi_media_time = mm_time;
> +    item->data.data_size = size;
> +    // TODO try to optimize avoiding the copy
> +    memcpy(item->data.data, data, size);
> +    red_channel_pipes_new_add(red_channel, pipe_item_new_ref, item);
> +    red_pipe_item_unref(&item->base);
>  }
> diff --git a/server/stream-channel.h b/server/stream-channel.h
> index 5691008c..6257f806 100644
> --- a/server/stream-channel.h
> +++ b/server/stream-channel.h
> @@ -48,6 +48,14 @@ GType stream_channel_get_type(void) G_GNUC_CONST;
>   */
>  StreamChannel* stream_channel_new(RedsState *server);
>  
> +struct StreamMsgFormat;
> +
> +void stream_channel_change_format(StreamChannel *channel,
> +                                  const struct StreamMsgFormat
> *fmt);
> +void stream_channel_send_data(StreamChannel *channel,
> +                              const void *data, size_t size,
> +                              uint32_t mm_time);
> +
>  G_END_DECLS
>  
>  #endif /* STREAM_CHANNEL_H_ */
> diff --git a/server/stream-device.c b/server/stream-device.c
> index 026f79c7..c1dd02d4 100644
> --- a/server/stream-device.c
> +++ b/server/stream-device.c
> @@ -149,6 +149,7 @@ handle_msg_format(StreamDevice *dev,
> SpiceCharDeviceInstance *sin)
>      }
>      fmt.width = GUINT32_FROM_LE(fmt.width);
>      fmt.height = GUINT32_FROM_LE(fmt.height);
> +    stream_channel_change_format(dev->channel, &fmt);
>      dev->hdr_pos = 0;
>  }
>  
> @@ -160,11 +161,10 @@ handle_msg_data(StreamDevice *dev,
> SpiceCharDeviceInstance *sin)
>      while (1) {
>          uint8_t buf[16 * 1024];
>          n = sif->read(sin, buf, sizeof(buf));
> -        /* TODO */
> -        spice_debug("readed %d bytes from device", n);
>          if (n <= 0) {
>              break;
>          }
> +        stream_channel_send_data(dev->channel, buf, n,
> reds_get_mm_time());
>          dev->hdr.size -= n;
>      }
>      if (dev->hdr.size == 0) {


Acked-by: Jonathon Jongsma <jjongsma at redhat.com>



More information about the Spice-devel mailing list