Re: Patch to support LZ4 compression algorithm‏‏

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



El Lunes, 12 de enero de 2015 17:35:27 Javier Celaya escribió:
> In a moment, I will send the patches that fix these problems.

These are the patches. One patch is for spice-server, the other two are for 
spice-common. Can someone review and apply them, please?

Thanks in advance
>From 1668d27f7b0016e54fd494ea511c8257b7fd05dc Mon Sep 17 00:00:00 2001
From: Javier Celaya <javier.celaya@xxxxxxxxx>
Date: Mon, 12 Jan 2015 17:14:28 +0100
Subject: [PATCH 1/2] Fix LZ4 output buffer size.

---
 common/canvas_base.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/common/canvas_base.c b/common/canvas_base.c
index ddcbe32..bb7a424 100644
--- a/common/canvas_base.c
+++ b/common/canvas_base.c
@@ -550,7 +550,7 @@ static pixman_image_t *canvas_get_jpeg(CanvasBase *canvas, SpiceImage *image, in
 static pixman_image_t *canvas_get_lz4(CanvasBase *canvas, SpiceImage *image, int invers)
 {
     pixman_image_t *surface = NULL;
-    int dec_size, enc_size;
+    int dec_size, enc_size, available;
     int stride;
     int stride_abs;
     uint8_t *dest, *data, *data_end;
@@ -579,6 +579,7 @@ static pixman_image_t *canvas_get_lz4(CanvasBase *canvas, SpiceImage *image, int
     dest = (uint8_t *)pixman_image_get_data(surface);
     stride = pixman_image_get_stride(surface);
     stride_abs = abs(stride);
+    available = height * stride_abs;
     if (direction == 1) {
         dest -= (stride_abs * (height - 1));
     }
@@ -588,7 +589,7 @@ static pixman_image_t *canvas_get_lz4(CanvasBase *canvas, SpiceImage *image, int
         enc_size = ntohl(*((uint32_t *)data));
         data += 4;
         dec_size = LZ4_decompress_safe_continue(stream, (const char *) data,
-                                                (char *) dest, enc_size, height * stride_abs);
+                                                (char *) dest, enc_size, available);
         if (dec_size <= 0) {
             spice_warning("Error decoding LZ4 block\n");
             pixman_image_unref(surface);
@@ -596,6 +597,7 @@ static pixman_image_t *canvas_get_lz4(CanvasBase *canvas, SpiceImage *image, int
             break;
         }
         dest += dec_size;
+        available -= dec_size;
         data += enc_size;
     } while (data < data_end);
 
-- 
1.9.3

>From 2caa51d4297ff16801e6a8b7243be9784b97e0fe Mon Sep 17 00:00:00 2001
From: Javier Celaya <javier.celaya@xxxxxxxxx>
Date: Mon, 12 Jan 2015 17:22:27 +0100
Subject: [PATCH 2/2] Fix LZ4 supported image formats.

- Read the original image format from the LZ4 stream. Only RGB formats
  are supported.
- Expand 24bit to 32bit image format.
---
 common/canvas_base.c | 48 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/common/canvas_base.c b/common/canvas_base.c
index bb7a424..d1a714c 100644
--- a/common/canvas_base.c
+++ b/common/canvas_base.c
@@ -554,22 +554,42 @@ static pixman_image_t *canvas_get_lz4(CanvasBase *canvas, SpiceImage *image, int
     int stride;
     int stride_abs;
     uint8_t *dest, *data, *data_end;
-    int width, height, direction;
+    int width, height, top_down;
     LZ4_streamDecode_t *stream;
+    uint8_t spice_format;
+    pixman_format_code_t format;
+    uint32_t * dest32, xrgb;
+    int i32, i24;
 
     spice_chunks_linearize(image->u.lz4.data);
     data = image->u.lz4.data->chunk[0].data;
     data_end = data + image->u.lz4.data->chunk[0].len;
     width = image->descriptor.width;
     height = image->descriptor.height;
-    direction = *(data++);
+    top_down = *(data++);
+    spice_format = *(data++);
+    switch (spice_format) {
+        case SPICE_BITMAP_FMT_16BIT:
+            format =  PIXMAN_x1r5g5b5;
+            break;
+        case SPICE_BITMAP_FMT_24BIT:
+        case SPICE_BITMAP_FMT_32BIT:
+            format =  PIXMAN_x8r8g8b8;
+            break;
+        case SPICE_BITMAP_FMT_RGBA:
+            format =  PIXMAN_a8r8g8b8;
+            break;
+        default:
+            spice_error("Unsupported bitmap format %d with LZ4\n", spice_format);
+            return NULL;
+    }
 
     surface = surface_create(
 #ifdef WIN32
                              canvas->dc,
 #endif
-                             PIXMAN_a8r8g8b8,
-                             width, height, direction == 0);
+                             format,
+                             width, height, top_down);
     if (surface == NULL) {
         spice_warning("create surface failed");
         return NULL;
@@ -580,7 +600,7 @@ static pixman_image_t *canvas_get_lz4(CanvasBase *canvas, SpiceImage *image, int
     stride = pixman_image_get_stride(surface);
     stride_abs = abs(stride);
     available = height * stride_abs;
-    if (direction == 1) {
+    if (!top_down) {
         dest -= (stride_abs * (height - 1));
     }
 
@@ -602,6 +622,24 @@ static pixman_image_t *canvas_get_lz4(CanvasBase *canvas, SpiceImage *image, int
     } while (data < data_end);
 
     LZ4_freeStreamDecode(stream);
+
+    if (spice_format == SPICE_BITMAP_FMT_24BIT) {
+        // Expand 24 to 32 bits
+        dest = (uint8_t *)pixman_image_get_data(surface);
+        if (!top_down) {
+            dest -= (stride_abs * (height - 1));
+        }
+        dest32 = (uint32_t *)dest;
+        i32 = height * width;
+        i24 = (height * stride_abs * 3) / 4;
+        while (i32 > 0) {
+            xrgb = dest[--i24] << 16;  // R
+            xrgb += dest[--i24] << 8;  // G
+            xrgb += dest[--i24];       // B
+            dest32[--i32] = xrgb;
+        }
+    }
+
     return surface;
 }
 #endif
-- 
1.9.3

>From 4efbc83d1ffa8c098b6496fe58ca74e79ad2b6c2 Mon Sep 17 00:00:00 2001
From: Javier Celaya <javier.celaya@xxxxxxxxx>
Date: Mon, 12 Jan 2015 17:41:53 +0100
Subject: [PATCH] Fix LZ4 supported image formats.

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.
---
 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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]