[Spice-devel] [PATCH 5/6] LZ4: Fix the row alignment when it is not on a 32bit boundary

[Christophe Fergeau]

Hey,

On Thu, Jan 22, 2015 at 05:21:23PM +0100, Javier Celaya wrote:
> ---
>  common/canvas_base.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/common/canvas_base.c b/common/canvas_base.c
> index c45d535..170def1 100644
> --- a/common/canvas_base.c
> +++ b/common/canvas_base.c
> @@ -563,8 +563,7 @@ static pixman_image_t *canvas_get_lz4(CanvasBase *canvas, SpiceImage *image, int
>  {
>      pixman_image_t *surface = NULL;
>      int dec_size, enc_size, available;
> -    int stride;
> -    int stride_abs;
> +    int stride, stride_abs, stride_encoded;
>      uint8_t *dest, *data, *data_end;
>      int width, height, top_down;
>      LZ4_streamDecode_t *stream;
> @@ -575,19 +574,23 @@ static pixman_image_t *canvas_get_lz4(CanvasBase *canvas, SpiceImage *image, int
>      data = image->u.lz4.data->chunk[0].data;
>      data_end = data + image->u.lz4.data->chunk[0].len;
>      width = image->descriptor.width;
> +    stride_encoded = width;
>      height = image->descriptor.height;
>      top_down = *(data++);
>      spice_format = *(data++);
>      switch (spice_format) {
>          case SPICE_BITMAP_FMT_16BIT:
>              format = PIXMAN_x1r5g5b5;
> +            stride_encoded *= 2;
>              break;
>          case SPICE_BITMAP_FMT_24BIT:
>          case SPICE_BITMAP_FMT_32BIT:
>              format = PIXMAN_x8r8g8b8;
> +            stride_encoded *= 4;
>              break;
>          case SPICE_BITMAP_FMT_RGBA:
>              format = PIXMAN_a8r8g8b8;
> +            stride_encoded *= 4;
>              break;
>          default:
>              spice_warning("Unsupported bitmap format %d with LZ4\n", spice_format);
> @@ -631,6 +634,24 @@ static pixman_image_t *canvas_get_lz4(CanvasBase *canvas, SpiceImage *image, int
>          data += enc_size;
>      } while (data < data_end);
>  
> +    if (stride_abs > stride_encoded) {

My understanding is that this will only trigger in the 24bpp case when
the next commit adds 
          case SPICE_BITMAP_FMT_24BIT:
              stride_encoded *= 3;
?
If so, it should be mentioned in the commit log that this only add some
helper code which will be used when 24bpp support is added or something
like this.

> +        // Fix the row alignment
> +        int row, col;
> +        uint32_t *dest_aligned, *dest_misaligned;
> +        dest = (uint8_t *)pixman_image_get_data(surface);
> +        if (!top_down) {
> +            dest -= (stride_abs * (height - 1));
> +        }
> +        stride_abs /= 4;
> +        dest_aligned = (uint32_t *)dest + stride_abs*height - 1;
> +        for (row = height - 1; row > 0; --row) {
> +            dest_misaligned = (uint32_t*)(dest + stride_encoded*row) + stride_abs - 1;
> +            for (col = 0; col < stride_abs; ++col) {
> +                *dest_aligned-- = *dest_misaligned--;
> +            }

Couldn't it be handled with a bunch of memmove?
for (row = height - 1; row > 0; --row) {
    dest_aligned = (uint32_t *)(dest + stride_abs*row)
    dest_misaligned = (uint32_t*)(dest + stride_encoded*row);
    memmove(dest_aligned, dest_misaligned, stride_encoded*sizeof(uint32_t));
    /* set the padding bytes to 0 if needed */
}

Christophe
-------------- 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/20150126/bfc1f759/attachment.sig>