[Spice-devel] JPEG error - Application transferred too few scanlines

Submitted by Christophe Fergeau on July 29, 2011, 12:08 a.m.

Details

Message ID 20110728170857.GG2545@teriyaki.cdg.redhat.com
State New, archived
Headers show

Not browsing as part of any series.

Commit Message

Christophe Fergeau July 29, 2011, 12:08 a.m.
Hey,

On Wed, Jul 27, 2011 at 06:11:32PM +0200, Christophe Fergeau wrote:
> Yep, I broke that in master, it's caused by the mjpeg changes, the "height"
> used when calling mjpeg_encoder_new is 1 byte less than the height which is
> then used in the loop around mjpeg_encoder_encode_scanline.

I went further debugging that, image sizes get rounded to the next even
number. I didn't notice that when I wrote the code, so the loop sending the
image to encode line by line to libjpeg uses the non-rounded height for
odd-sized images. And then libjpeg is right in complaining that one line is
missing :) The (ugly) attached  patch seems to be working as a workaround,
but I'll come up with a better fix.

Christophe
commit 74eb7e06184baf2b671c540225645d88f01d8abd
Author: Christophe Fergeau <cfergeau@redhat.com>
Date:   Thu Jul 28 13:30:24 2011 +0200

    fix

Patch hide | download patch | download mbox

diff --git a/server/red_worker.c b/server/red_worker.c
index 6737940..0ae53a7 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -7329,11 +7329,12 @@  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;
+    uint8_t *src_line;
 
-    for (i = 0; i < image_height; i++) {
-        uint8_t *src_line =
+    for (i = 0; i < stream_height; i++) {
+        src_line =
             (uint8_t *)red_get_image_line(worker, chunks, &offset, &chunk, image_stride);
 
         if (!src_line) {
@@ -7341,10 +7342,14 @@  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;
     }
 
+    if ((stream_height > 0) && (stream_height != SPICE_ALIGN(src->bottom - src->top, 2)))
+        if (mjpeg_encoder_encode_scanline(stream->mjpeg_encoder, src_line, stream_width) == 0)
+            return FALSE;
+
     return TRUE;
 }
 

Comments

Ok thanks.
I have another question. I want to try to separate the streaming data
and create a sperate channel for this kind of data. Do you think that
is better to work with the new client or with spicec?

2011/7/28 Christophe Fergeau <cfergeau@redhat.com>:
> Hey,
>
> On Wed, Jul 27, 2011 at 06:11:32PM +0200, Christophe Fergeau wrote:
>> Yep, I broke that in master, it's caused by the mjpeg changes, the "height"
>> used when calling mjpeg_encoder_new is 1 byte less than the height which is
>> then used in the loop around mjpeg_encoder_encode_scanline.
>
> I went further debugging that, image sizes get rounded to the next even
> number. I didn't notice that when I wrote the code, so the loop sending the
> image to encode line by line to libjpeg uses the non-rounded height for
> odd-sized images. And then libjpeg is right in complaining that one line is
> missing :) The (ugly) attached  patch seems to be working as a workaround,
> but I'll come up with a better fix.
>
> Christophe
>
On Mon, Aug 01, 2011 at 02:15:07PM +0200, Andrea Celestino wrote:
> Ok thanks.
> I have another question. I want to try to separate the streaming data
> and create a sperate channel for this kind of data. Do you think that
> is better to work with the new client or with spicec?
> 

Probably better with the new client.

> 2011/7/28 Christophe Fergeau <cfergeau@redhat.com>:
> > Hey,
> >
> > On Wed, Jul 27, 2011 at 06:11:32PM +0200, Christophe Fergeau wrote:
> >> Yep, I broke that in master, it's caused by the mjpeg changes, the "height"
> >> used when calling mjpeg_encoder_new is 1 byte less than the height which is
> >> then used in the loop around mjpeg_encoder_encode_scanline.
> >
> > I went further debugging that, image sizes get rounded to the next even
> > number. I didn't notice that when I wrote the code, so the loop sending the
> > image to encode line by line to libjpeg uses the non-rounded height for
> > odd-sized images. And then libjpeg is right in complaining that one line is
> > missing :) The (ugly) attached  patch seems to be working as a workaround,
> > but I'll come up with a better fix.
> >
> > Christophe
> >
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel