[Spice-devel] [PATCH spice-server 8/8] stream-channel: Send the full frame in a single message
Frediano Ziglio
fziglio at redhat.com
Sun Feb 18 18:58:14 UTC 2018
The current implementation of server and client assume that a single
data message contains an encoded frame.
This is not a problem for most encoding but for MJPEG this causes
the client to fail decoding.
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
server/stream-channel.c | 97 ++++++++++++++++++++++++++++++++++++++++++-------
server/stream-channel.h | 12 ++++++
server/stream-device.c | 7 ++--
3 files changed, 100 insertions(+), 16 deletions(-)
diff --git a/server/stream-channel.c b/server/stream-channel.c
index 88f859f6..79addec0 100644
--- a/server/stream-channel.c
+++ b/server/stream-channel.c
@@ -44,6 +44,7 @@
typedef struct StreamChannelClient StreamChannelClient;
typedef struct StreamChannelClientClass StreamChannelClientClass;
+typedef struct StreamDataItem StreamDataItem;
/* we need to inherit from CommonGraphicsChannelClient
* to get buffer handling */
@@ -74,6 +75,10 @@ struct StreamChannel {
StreamQueueStat queue_stat;
+ /* pending partial data item */
+ StreamDataItem *data_item;
+ uint32_t data_item_pos;
+
/* callback to notify when a stream should be started or stopped */
stream_channel_start_proc start_cb;
void *start_opaque;
@@ -104,12 +109,12 @@ typedef struct StreamCreateItem {
SpiceMsgDisplayStreamCreate stream_create;
} StreamCreateItem;
-typedef struct StreamDataItem {
+struct StreamDataItem {
RedPipeItem base;
StreamChannel *channel;
// NOTE: this must be the last field in the structure
SpiceMsgDisplayStreamData data;
-} StreamDataItem;
+};
#define PRIMARY_SURFACE_ID 0
@@ -129,6 +134,18 @@ stream_channel_client_init(StreamChannelClient *client)
client->stream_id = -1;
}
+static void
+stream_channel_unref_data_item(StreamChannel *channel)
+{
+ if (channel->data_item) {
+ // this is required in order to update statistics correctly
+ channel->data_item->data.data_size = 0;
+ red_pipe_item_unref(&channel->data_item->base);
+ channel->data_item = NULL;
+ channel->data_item_pos = 0;
+ }
+}
+
static void
request_new_stream(StreamChannel *channel, StreamMsgStartStop *start)
{
@@ -152,6 +169,7 @@ stream_channel_client_on_disconnect(RedChannelClient *rcc)
channel->stream_id = -1;
channel->width = 0;
channel->height = 0;
+ stream_channel_unref_data_item(channel);
// send stream stop to device
StreamMsgStartStop stop = { 0, };
@@ -424,6 +442,16 @@ stream_channel_constructed(GObject *object)
reds_register_channel(reds, red_channel);
}
+static void
+stream_channel_finalize(GObject *object)
+{
+ StreamChannel *channel = STREAM_CHANNEL(object);
+
+ stream_channel_unref_data_item(channel);
+
+ G_OBJECT_CLASS(stream_channel_parent_class)->finalize(object);
+}
+
static void
stream_channel_class_init(StreamChannelClass *klass)
{
@@ -431,6 +459,7 @@ stream_channel_class_init(StreamChannelClass *klass)
RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
object_class->constructed = stream_channel_constructed;
+ object_class->finalize = stream_channel_finalize;
channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_DISPLAY, NULL);
channel_class->handle_message = handle_message;
@@ -508,6 +537,39 @@ data_item_free(RedPipeItem *base)
g_free(pipe_item);
}
+static StreamDataItem*
+stream_channel_new_data_item(StreamChannel *channel, size_t size, uint32_t mm_time)
+{
+ stream_channel_unref_data_item(channel);
+
+ StreamDataItem *item = g_malloc(sizeof(*item) + size);
+ red_pipe_item_init_full(&item->base, RED_PIPE_ITEM_TYPE_STREAM_DATA,
+ data_item_free);
+ item->data.base.id = channel->stream_id;
+ item->data.base.multi_media_time = mm_time;
+ item->data.data_size = size;
+ item->channel = channel;
+
+ channel->data_item = item;
+ channel->data_item_pos = 0;
+
+ return item;
+}
+
+void
+stream_channel_start_data(StreamChannel *channel, size_t size, uint32_t mm_time)
+{
+ // see stream_channel_send_data comment
+ if (channel->stream_id < 0) {
+ return;
+ }
+
+ // TODO this collects all chunks in a single message
+ // up: we send a single frame together (more compatible)
+ // down: guest can cause a crash due to DoS. As a safe measure we limit the maximum message
+ stream_channel_new_data_item(channel, MIN(size, 32*1024*1024), mm_time);
+}
+
void
stream_channel_send_data(StreamChannel *channel, const void *data, size_t size, uint32_t mm_time)
{
@@ -520,17 +582,25 @@ stream_channel_send_data(StreamChannel *channel, const void *data, size_t size,
RedChannel *red_channel = RED_CHANNEL(channel);
- StreamDataItem *item = g_malloc(sizeof(*item) + size);
- red_pipe_item_init_full(&item->base, RED_PIPE_ITEM_TYPE_STREAM_DATA,
- data_item_free);
- item->data.base.id = channel->stream_id;
- item->data.base.multi_media_time = mm_time;
- item->data.data_size = size;
- item->channel = channel;
- stream_channel_update_queue_stat(channel, 1, size);
- // TODO try to optimize avoiding the copy
- memcpy(item->data.data, data, size);
- red_channel_pipes_add(red_channel, &item->base);
+ while (size) {
+ StreamDataItem *item = channel->data_item;
+
+ if (!item) {
+ item = stream_channel_new_data_item(channel, size, mm_time);
+ }
+
+ size_t copy_size = item->data.data_size - channel->data_item_pos;
+ copy_size = MIN(copy_size, size);
+ // TODO try to optimize avoiding the copy
+ memcpy(item->data.data + channel->data_item_pos, data, copy_size);
+ size -= copy_size;
+ channel->data_item_pos += copy_size;
+ if (channel->data_item_pos == item->data.data_size) {
+ channel->data_item = NULL;
+ stream_channel_update_queue_stat(channel, 1, item->data.data_size);
+ red_channel_pipes_add(red_channel, &item->base);
+ }
+ }
}
void
@@ -570,6 +640,7 @@ stream_channel_reset(StreamChannel *channel)
channel->stream_id = -1;
channel->width = 0;
channel->height = 0;
+ stream_channel_unref_data_item(channel);
if (!red_channel_is_connected(red_channel)) {
return;
diff --git a/server/stream-channel.h b/server/stream-channel.h
index e8bec80b..18a1bdea 100644
--- a/server/stream-channel.h
+++ b/server/stream-channel.h
@@ -60,6 +60,18 @@ struct StreamMsgStartStop;
void stream_channel_change_format(StreamChannel *channel,
const struct StreamMsgFormat *fmt);
+
+/**
+ * Tell the channel that a new data packet is starting.
+ * This can be used to group all chunks together.
+ */
+void stream_channel_start_data(StreamChannel *channel,
+ size_t size,
+ uint32_t mm_time);
+
+/**
+ * Send to channel a chunk of data.
+ */
void stream_channel_send_data(StreamChannel *channel,
const void *data, size_t size,
uint32_t mm_time);
diff --git a/server/stream-device.c b/server/stream-device.c
index ddac0ca9..b206b116 100644
--- a/server/stream-device.c
+++ b/server/stream-device.c
@@ -285,11 +285,12 @@ handle_msg_data(StreamDevice *dev, SpiceCharDeviceInstance *sin)
if (n <= 0) {
break;
}
- // TODO collect all message ??
- // up: we send a single frame together
- // down: guest can cause a crash
+ if (dev->msg_pos == 0) {
+ stream_channel_start_data(dev->stream_channel, dev->hdr.size, reds_get_mm_time());
+ }
stream_channel_send_data(dev->stream_channel, buf, n, reds_get_mm_time());
dev->hdr.size -= n;
+ dev->msg_pos += n;
}
return dev->hdr.size == 0;
--
2.14.3
More information about the Spice-devel
mailing list