>
> SPICE expect 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, it confuses the client which burns CPU
> till it fails.
>
> This patch fixes that, but reading the whole message from the
> device (the streaming agent) and sending it over to the client.
>
> BTW, why realloc up and realloc down for every message ?
>
I don't know, before this patch it was done only for cursor shape which
should be not that often. So the question is why you want to do it
for every message?
> Signed-off-by: Uri Lublin <uril@xxxxxxxxxx>
> ---
>
> This is an alternative solution to Frediano's patch
> "stream-channel: Send the full frame in a single message" (v3)
>
> The 32MB limitation is the same as in Frediano's patch.
> Perhaps it's too big (?)
>
That's slightly different, this patch hard limit to 32mb considering
more than 32mb a protocol error, on my patch was falling back to
split.
> The BTW above should have appeared here, next time ...
> Also why use realloc at all, and not free + alloc (no memcpy) ?
>
For the shrink case, considered not so often and considering
the buffer so small a single function call can be quicker, the
allocator is free to reuse the same memory buffer.
For expanding with same considerations (not so often and small
starting buffer) more or less the same.
> ---
> server/red-stream-device.c | 40 +++++++++++++++++++++++++-------------
> 1 file changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/server/red-stream-device.c b/server/red-stream-device.c
> index df6a366f4..432632f14 100644
> --- a/server/red-stream-device.c
> +++ b/server/red-stream-device.c
> @@ -146,7 +146,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 +312,30 @@ 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;
> - }
> - // 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;
> + /* make sure we have a large enough buffer for the whole frame */
> + if (dev->msg_pos == 0 && dev->msg_len < dev->hdr.size) {
> + g_free(dev->msg);
> + dev->msg = g_malloc(dev->hdr.size);
> + dev->msg_len = dev->hdr.size;
> }
>
> - 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, reds_get_mm_time());
OT, not a regression, the time should be taken when the guest starts
sending the message, not at the end.
> + dev->hdr.size = 0;
> +
> +
> + return true;
> }
>
> /*
Note that this patch is increasing processing latency (as probably
mine).
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel