[Spice-devel] [PATCH] Fix LZ4 supported image formats.
Christophe Fergeau
cfergeau at redhat.com
Wed Jan 21 08:05:00 PST 2015
Hey,
Sorry for the late review...
On Thu, Jan 15, 2015 at 12:50:21PM +0100, Javier Celaya wrote:
> This patch limits the LZ4 algorithm to RGB formats. Other formats are
> compressed with LZ. It also sends the top_down flag and the original
> format to the client, so that it can create the right kind of pixman
> surface.
Looking at the other compression formats, it seems they don't send a
top down flag, but instead 'revert' the image before sending it, any
reason this is not being done here?
This patch breaks the wire protocol which was used in spice-gtk 0.27,
which we usually don't do, but since what was in the release was
crashing when being used, I'd say this is fine to change.
This commit could have easily been split in different ones (limit lz4 to
RGB formats, send the original format to the client, ...
Apart from these comments, this looks good.
Christophe
> ---
> server/lz4_encoder.c | 21 +++++++++++----------
> server/lz4_encoder.h | 7 +++----
> server/red_worker.c | 21 +++++++--------------
> 3 files changed, 21 insertions(+), 28 deletions(-)
>
> diff --git a/server/lz4_encoder.c b/server/lz4_encoder.c
> index 531ab4b..bb8e98a 100644
> --- a/server/lz4_encoder.c
> +++ b/server/lz4_encoder.c
> @@ -49,22 +49,23 @@ void lz4_encoder_destroy(Lz4EncoderContext* encoder)
> free(encoder);
> }
>
> -int lz4_encode(Lz4EncoderContext *lz4, int height, int stride,
> - uint8_t *io_ptr, unsigned int num_io_bytes)
> +int lz4_encode(Lz4EncoderContext *lz4, int height, int stride, uint8_t *io_ptr,
> + unsigned int num_io_bytes, int top_down, uint8_t format)
> {
> Lz4Encoder *enc = (Lz4Encoder *)lz4;
> uint8_t *lines;
> int num_lines = 0;
> int total_lines = 0;
> - int in_size, enc_size, out_size = 0, already_copied;
> - int stride_abs = abs(stride);
> + int in_size, enc_size, out_size, already_copied;
> uint8_t *in_buf, *compressed_lines;
> uint8_t *out_buf = io_ptr;
> LZ4_stream_t *stream = LZ4_createStream();
>
> - // Encode direction
> - *(out_buf++) = stride < 0 ? 1 : 0;
> - num_io_bytes--;
> + // Encode direction and format
> + *(out_buf++) = top_down ? 1 : 0;
> + *(out_buf++) = format;
> + out_size = 2;
> + num_io_bytes -= 2;
>
> do {
> num_lines = enc->usr->more_lines(enc->usr, &lines);
> @@ -73,9 +74,9 @@ int lz4_encode(Lz4EncoderContext *lz4, int height, int stride,
> LZ4_freeStream(stream);
> return 0;
> }
> - in_buf = stride < 0 ? lines - (stride_abs * (num_lines - 1)) : lines;
> - lines += stride * num_lines;
> - in_size = stride_abs * num_lines;
> + in_buf = lines;
> + in_size = stride * num_lines;
> + lines += in_size;
> compressed_lines = (uint8_t *) malloc(LZ4_compressBound(in_size) + 4);
> enc_size = LZ4_compress_continue(stream, (const char *) in_buf,
> (char *) compressed_lines + 4, in_size);
> diff --git a/server/lz4_encoder.h b/server/lz4_encoder.h
> index f3359c0..f1de12a 100644
> --- a/server/lz4_encoder.h
> +++ b/server/lz4_encoder.h
> @@ -43,8 +43,7 @@ struct Lz4EncoderUsrContext {
> Lz4EncoderContext* lz4_encoder_create(Lz4EncoderUsrContext *usr);
> void lz4_encoder_destroy(Lz4EncoderContext *encoder);
>
> -/* returns the total size of the encoded data. Images must be supplied from the
> - top line to the bottom */
> -int lz4_encode(Lz4EncoderContext *lz4, int height, int stride,
> - uint8_t *io_ptr, unsigned int num_io_bytes);
> +/* returns the total size of the encoded data. */
> +int lz4_encode(Lz4EncoderContext *lz4, int height, int stride, uint8_t *io_ptr,
> + unsigned int num_io_bytes, int top_down, uint8_t format);
> #endif
> diff --git a/server/red_worker.c b/server/red_worker.c
> index a18987a..16411a7 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -6422,7 +6422,6 @@ static int red_lz4_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
> Lz4Data *lz4_data = &worker->lz4_data;
> Lz4EncoderContext *lz4 = worker->lz4;
> int lz4_size = 0;
> - int stride;
>
> #ifdef COMPRESS_STAT
> stat_time_t start_time = stat_now();
> @@ -6454,20 +6453,12 @@ static int red_lz4_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
>
> lz4_data->data.u.lines_data.chunks = src->data;
> lz4_data->data.u.lines_data.stride = src->stride;
> + lz4_data->data.u.lines_data.next = 0;
> + lz4_data->data.u.lines_data.reverse = 0;
> lz4_data->usr.more_lines = lz4_usr_more_lines;
> -
> - if ((src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN)) {
> - lz4_data->data.u.lines_data.next = 0;
> - lz4_data->data.u.lines_data.reverse = 0;
> - stride = src->stride;
> - } else {
> - lz4_data->data.u.lines_data.next = src->data->num_chunks - 1;
> - lz4_data->data.u.lines_data.reverse = 1;
> - stride = -src->stride;
> - }
> -
> - lz4_size = lz4_encode(lz4, src->y, stride, (uint8_t*)lz4_data->data.bufs_head->buf,
> - sizeof(lz4_data->data.bufs_head->buf));
> + lz4_size = lz4_encode(lz4, src->y, src->stride, (uint8_t*)lz4_data->data.bufs_head->buf,
> + sizeof(lz4_data->data.bufs_head->buf),
> + src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN, src->format);
>
> // the compressed buffer is bigger than the original data
> if (lz4_size > (src->y * src->stride)) {
> @@ -6678,6 +6669,7 @@ static inline int red_compress_image(DisplayChannelClient *dcc,
> if (!glz) {
> #ifdef USE_LZ4
> if (image_compression == SPICE_IMAGE_COMPRESS_LZ4 &&
> + bitmap_fmt_is_rgb(src->format) &&
> red_channel_client_test_remote_cap(&dcc->common.base,
> SPICE_DISPLAY_CAP_LZ4_COMPRESSION)) {
> ret = red_lz4_compress_image(dcc, dest, src, o_comp_data,
> @@ -8918,6 +8910,7 @@ static void red_marshall_image(RedChannelClient *rcc, SpiceMarshaller *m, ImageI
> } else {
> #ifdef USE_LZ4
> if (comp_mode == SPICE_IMAGE_COMPRESS_LZ4 &&
> + bitmap_fmt_is_rgb(bitmap.format) &&
> red_channel_client_test_remote_cap(&dcc->common.base,
> SPICE_DISPLAY_CAP_LZ4_COMPRESSION)) {
> comp_succeeded = red_lz4_compress_image(dcc, &red_image, &bitmap,
> --
> 1.9.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150121/ab896fb0/attachment.sig>
More information about the Spice-devel
mailing list