On Fri, Jul 29, 2011 at 05:28:35PM +0200, Christophe Fergeau wrote:
ACK.
> 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 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 6737940..efedc19 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) {
> --
> 1.7.6
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
