Hi, On 07/29/2011 05:28 PM, Christophe Fergeau wrote:
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 :)
Hmm, I assume you've tested this with odd sized images? The reason I'm asking is that JPEG compression usually happens by converting from RGB to YUV color space and in the YUV colorspace the UV components are sub-sampled 2x2 (iow the UV resolution is half that of the Y/image resolution). The usual trick for odd sized images is to duplicate the last row / column to make them even sized. I could not find anything about this in the libjpeg documentation, so maybe libjpeg take cares about this itself? While on the subject of libjpeg, as this assert shows we should be defining our own jpeg error_exit handler, so that in case of an error we can simply drop the image update rather then kill the entire vm. This can be done by losing longjmp (ugly I know), see the init_libjpeg_cinfo function in: http://git.linuxtv.org/v4l-utils.git/blob/HEAD:/lib/libv4lconvert/jpeg.c Also I think there are several opportunities for more optimization with our libjpeg code. The first is to not include and parse the headers each frame, this will save both bandwidth and CPU for parsing the huffman headers and generating lookup tables, see the "Abbreviated datastreams and multiple images" section in /usr/share/doc/libjpeg-turbo-devel-1.1.0/libjpeg.txt The second is to define a new jpeg-rgb format where we use jpeg compression straight on rgb, rather then first converting it to yuv colorspace (this happens inside libjpeg, but can be overridden) this will lead to a slight increase in bandwidth usage, but also a great drop in cpu usage. Regards, Hans _______________________________________________ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
