[Spice-commits] server/red-stream-device.c

[GitLab Mirror]

 server/red-stream-device.c |   47 +++++++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 12 deletions(-)

New commits:
commit dcc3f995d9f5575e319adcfe530c477a7c294ff3
Author: Uri Lublin <uril at redhat.com>
Date:   Sun Jul 8 16:57:08 2018 +0300

    stream-device: handle_data: send whole message
    
    SPICE expects to have each frame in a single message.
    So far the stream-device did not do that.
    That works fine for H264 streams but not for MJPEG.
    
    The client handles by itself MJPEG streams, and not via
    gstreamer, and is assuming that a message contains the
    whole frame. Since it currently not, using spice-streaming-agent
    with MJPEG plugin, confuses the client which burns CPU
    till it fails and keeps complaining:
      "GSpice-CRITICAL **: 15:53:36.984: need more input data"
    
    This patch fixes that, by reading the whole message from the
    device (the streaming agent) and sending it over to the client.
    
    Signed-off-by: Uri Lublin <uril at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/red-stream-device.c b/server/red-stream-device.c
index f280a089..6a2b6a73 100644
--- a/server/red-stream-device.c
+++ b/server/red-stream-device.c
@@ -48,6 +48,7 @@ struct StreamDevice {
     StreamChannel *stream_channel;
     CursorChannel *cursor_channel;
     SpiceTimer *close_timer;
+    uint32_t frame_mmtime;
 };
 
 struct StreamDeviceClass {
@@ -146,7 +147,11 @@ stream_device_partial_read(StreamDevice *dev, SpiceCharDeviceInstance *sin)
         }
         break;
     case STREAM_TYPE_DATA:
-        handled = handle_msg_data(dev, sin);
+        if (dev->hdr.size > 32*1024*1024) {
+            handled = handle_msg_invalid(dev, sin, "STREAM_DATA too large");
+        } else {
+            handled = handle_msg_data(dev, sin);
+        }
         break;
     case STREAM_TYPE_CURSOR_SET:
         handled = handle_msg_cursor_set(dev, sin);
@@ -308,20 +313,38 @@ handle_msg_data(StreamDevice *dev, SpiceCharDeviceInstance *sin)
         spice_assert(dev->hdr.type == STREAM_TYPE_DATA);
     }
 
-    while (1) {
-        uint8_t buf[16 * 1024];
-        n = sif->read(sin, buf, MIN(sizeof(buf), dev->hdr.size));
-        if (n <= 0) {
-            break;
+    /* make sure we have a large enough buffer for the whole frame */
+    /* ---
+     * TODO: Now that we copy partial data into the buffer, for each frame
+     * the buffer is allocated and freed (look for g_realloc in
+     * stream_device_partial_read).
+     * Probably better to just keep the larger buffer.
+     */
+    if (dev->msg_pos == 0) {
+        dev->frame_mmtime = reds_get_mm_time();
+        if (dev->msg_len < dev->hdr.size) {
+            g_free(dev->msg);
+            dev->msg = g_malloc(dev->hdr.size);
+            dev->msg_len = dev->hdr.size;
         }
-        // TODO collect all message ??
-        // up: we send a single frame together
-        // down: guest can cause a crash
-        stream_channel_send_data(dev->stream_channel, buf, n, reds_get_mm_time());
-        dev->hdr.size -= n;
     }
 
-    return dev->hdr.size == 0;
+    /* read from device */
+    n = sif->read(sin, dev->msg->buf + dev->msg_pos, dev->hdr.size - dev->msg_pos);
+    if (n <= 0) {
+        return false;
+    }
+
+    dev->msg_pos += n;
+    if (dev->msg_pos != dev->hdr.size) { /* some bytes are still missing */
+        return false;
+    }
+
+    /* The whole frame was read from the device, send it */
+    stream_channel_send_data(dev->stream_channel, dev->msg->buf, dev->hdr.size,
+                             dev->frame_mmtime);
+
+    return true;
 }
 
 /*