>
> 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@xxxxxxxxxx>
> > ---
> > 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()
>
> ?
>
I liked the suggestion to add a function to RedChannel.
Too many red_channel_add_whatever :-)
>
> > +
> > +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.
>
I put a comment now.
This is not really an error, can happen if we send a stop but
the guest didn't handle it or was still sending data.
> > + 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@xxxxxxxxxx>
>
>
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel