From 4b15555fbd9aab3726b2bb7ee0c878d4fcd04410 Mon Sep 17 00:00:00 2001
From: Christophe Fergeau <cfergeau@redhat.com>
Date: Thu, 28 Jul 2011 13:30:24 +0200
Subject: [PATCH] mjpeg: fix libjpeg assertion

After the changes to add libjpeg-turbo support to spice-server mjpeg
compression code, it's relatively easy to hit an assertion from
libjpeg in spice-server about "too few scanlines transferred" when
the mjpeg streaming code triggers. This assertion brings down qemu,
which is bad :)

This is because when we first initialize the mjpeg encoder, we do:

stream_width = SPICE_ALIGN(src_rect->right - src_rect->left, 2);
stream_height = SPICE_ALIGN(src_rect->bottom - src_rect->top, 2);

stream->mjpeg_encoder = mjpeg_encoder_new(stream_width, stream_height);

and then when we iterate over the image scanlines to feed them to
libjpeg, we do:

const int image_height = src->bottom - src->top;
const int image_width = src->right - src->left;

for (i = 0; i < image_height; i++) {
    mjpeg_encoder_encode_scanline(...);
}
mjpeg_encoder_end_frame(...);

When stream_height is odd, the mjpeg_encoder will be created with
an height that is 1 more than the number of lines we encode. Then
libjpeg asserts when we tell it we're done with the compression
while it's still waiting for one more scanline.

Looking through git history, this rounding seems to be an artifact
from when we were using ffmpeg for the mjpeg encoding. Since
spicec and spicy (the latter needs a few fixes) can handle streams
with odd height/width, the best way to solve this issue is to stop
rounding up the height and width of the streams we create. This
even saves some bandwidth :)
---
 server/red_worker.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/server/red_worker.c b/server/red_worker.c
index 67379402..efedc197 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -2417,8 +2417,8 @@ static void red_create_stream(RedWorker *worker, Drawable *drawable)
 
     ASSERT(drawable->red_drawable->type == QXL_DRAW_COPY);
     src_rect = &drawable->red_drawable->u.copy.src_area;
-    stream_width = SPICE_ALIGN(src_rect->right - src_rect->left, 2);
-    stream_height = SPICE_ALIGN(src_rect->bottom - src_rect->top, 2);
+    stream_width = src_rect->right - src_rect->left;
+    stream_height = src_rect->bottom - src_rect->top;
 
     stream->mjpeg_encoder = mjpeg_encoder_new(stream_width, stream_height);
 
@@ -7329,10 +7329,10 @@ static int encode_frame (RedWorker *worker, const SpiceRect *src,
         red_get_image_line(worker, chunks, &offset, &chunk, image_stride);
     }
 
-    const int image_height = src->bottom - src->top;
-    const int image_width = src->right - src->left;
+    const unsigned int stream_height = src->bottom - src->top;
+    const unsigned int stream_width = src->right - src->left;
 
-    for (i = 0; i < image_height; i++) {
+    for (i = 0; i < stream_height; i++) {
         uint8_t *src_line =
             (uint8_t *)red_get_image_line(worker, chunks, &offset, &chunk, image_stride);
 
@@ -7341,7 +7341,7 @@ static int encode_frame (RedWorker *worker, const SpiceRect *src,
         }
 
         src_line += src->left * mjpeg_encoder_get_bytes_per_pixel(stream->mjpeg_encoder);
-        if (mjpeg_encoder_encode_scanline(stream->mjpeg_encoder, src_line, image_width) == 0)
+        if (mjpeg_encoder_encode_scanline(stream->mjpeg_encoder, src_line, stream_width) == 0)
             return FALSE;
     }
 
@@ -7705,8 +7705,8 @@ static void red_display_marshall_stream_start(DisplayChannel *display_channel,
 
     stream_create.src_width = stream->width;
     stream_create.src_height = stream->height;
-    stream_create.stream_width = SPICE_ALIGN(stream_create.src_width, 2);
-    stream_create.stream_height = SPICE_ALIGN(stream_create.src_height, 2);
+    stream_create.stream_width = stream_create.src_width;
+    stream_create.stream_height = stream_create.src_height;
     stream_create.dest = stream->dest_area;
 
     if (stream->current) {