Hi, On Thu, 2016-04-14 at 17:56 +0100, Frediano Ziglio wrote: > LZ image decompression was broken for 16 bpp: > - stride was computed not computed correctly (as width*4). This > caused > also a buffer underflow; > - stride in pixman is always multiple of 4 bytes (so for 16 bpp is > ALIGN(width*2, 4)) so image decompressed by lz_decode as some > missing > bytes to be fixed. > It looks good. I wonder if the code has ever worked for lz 16bpp. > The alignment code is reused from LZ4 function. > > This fix also https://bugzilla.redhat.com/show_bug.cgi?id=1285469. > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> Acked-by: Pavel Grunt <pgrunt@xxxxxxxxxx> Thanks, Pavel > --- > common/canvas_base.c | 54 +++++++++++++++++++++++++++++++----------- > ---------- > 1 file changed, 32 insertions(+), 22 deletions(-) > > diff --git a/common/canvas_base.c b/common/canvas_base.c > index fa4d373..45dd75f 100644 > --- a/common/canvas_base.c > +++ b/common/canvas_base.c > @@ -515,13 +515,30 @@ static pixman_image_t > *canvas_get_jpeg(CanvasBase *canvas, SpiceImage *image) > return surface; > } > > +static void canvas_fix_alignment(uint8_t *bits, > + int stride_encoded, int > stride_pixman, > + int height) > +{ > + if (stride_pixman > stride_encoded) { > + // Fix the row alignment > + int row; > + uint8_t *dest = bits; > + for (row = height - 1; row > 0; --row) { > + uint32_t *dest_aligned, *dest_misaligned; > + dest_aligned = (uint32_t *)(dest + stride_pixman*row); > + dest_misaligned = (uint32_t*)(dest + > stride_encoded*row); > + memmove(dest_aligned, dest_misaligned, stride_encoded); > + } > + } > +} > + > #ifdef USE_LZ4 > static pixman_image_t *canvas_get_lz4(CanvasBase *canvas, SpiceImage > *image) > { > pixman_image_t *surface = NULL; > int dec_size, enc_size, available; > int stride, stride_abs, stride_encoded; > - uint8_t *dest, *data, *data_end; > + uint8_t *dest, *data, *data_end, *bits; > int width, height, top_down; > LZ4_streamDecode_t *stream; > uint8_t spice_format; > @@ -576,6 +593,7 @@ static pixman_image_t *canvas_get_lz4(CanvasBase > *canvas, SpiceImage *image) > if (!top_down) { > dest -= (stride_abs * (height - 1)); > } > + bits = dest; > > do { > // Read next compressed block > @@ -594,20 +612,7 @@ static pixman_image_t *canvas_get_lz4(CanvasBase > *canvas, SpiceImage *image) > data += enc_size; > } while (data < data_end); > > - if (stride_abs > stride_encoded) { > - // Fix the row alignment > - int row; > - dest = (uint8_t *)pixman_image_get_data(surface); > - if (!top_down) { > - dest -= (stride_abs * (height - 1)); > - } > - for (row = height - 1; row > 0; --row) { > - uint32_t *dest_aligned, *dest_misaligned; > - dest_aligned = (uint32_t *)(dest + stride_abs*row); > - dest_misaligned = (uint32_t*)(dest + > stride_encoded*row); > - memmove(dest_aligned, dest_misaligned, stride_encoded); > - } > - } > + canvas_fix_alignment(bits, stride_encoded, stride_abs, height); > > LZ4_freeStreamDecode(stream); > return surface; > @@ -782,7 +787,6 @@ static pixman_image_t *canvas_get_lz(CanvasBase > *canvas, SpiceImage *image, > uint8_t *comp_buf = NULL; > int comp_size; > uint8_t *decomp_buf = NULL; > - uint8_t *src; > pixman_format_code_t pixman_format; > LzImageType type, as_type; > SpicePalette *palette; > @@ -790,6 +794,7 @@ static pixman_image_t *canvas_get_lz(CanvasBase > *canvas, SpiceImage *image, > int width; > int height; > int top_down; > + int stride_encoded; > int stride; > int free_palette; > > @@ -818,10 +823,12 @@ static pixman_image_t *canvas_get_lz(CanvasBase > *canvas, SpiceImage *image, > lz_decode_begin(lz_data->lz, comp_buf, comp_size, &type, > &width, &height, &n_comp_pixels, &top_down, > palette); > > + stride_encoded = width; > switch (type) { > case LZ_IMAGE_TYPE_RGBA: > as_type = LZ_IMAGE_TYPE_RGBA; > pixman_format = PIXMAN_LE_a8r8g8b8; > + stride_encoded *= 4; > break; > case LZ_IMAGE_TYPE_RGB32: > case LZ_IMAGE_TYPE_RGB24: > @@ -832,6 +839,7 @@ static pixman_image_t *canvas_get_lz(CanvasBase > *canvas, SpiceImage *image, > case LZ_IMAGE_TYPE_PLT8: > as_type = LZ_IMAGE_TYPE_RGB32; > pixman_format = PIXMAN_LE_x8r8g8b8; > + stride_encoded *= 4; > break; > case LZ_IMAGE_TYPE_A8: > as_type = LZ_IMAGE_TYPE_A8; > @@ -843,9 +851,11 @@ static pixman_image_t *canvas_get_lz(CanvasBase > *canvas, SpiceImage *image, > canvas->format == SPICE_SURFACE_FMT_32_ARGB)) { > as_type = LZ_IMAGE_TYPE_RGB32; > pixman_format = PIXMAN_LE_x8r8g8b8; > + stride_encoded *= 4; > } else { > as_type = LZ_IMAGE_TYPE_RGB16; > pixman_format = PIXMAN_x1r5g5b5; > + stride_encoded *= 2; > } > break; > default: > @@ -865,18 +875,18 @@ static pixman_image_t *canvas_get_lz(CanvasBase > *canvas, SpiceImage *image, > alloc_lz_image_surface(&lz_data->decode_data, pixman_format, > width, height, n_comp_pixels, top_down); > > - src = (uint8_t *)pixman_image_get_data(lz_data- > >decode_data.out_surface); > + stride = pixman_image_get_stride(lz_data- > >decode_data.out_surface); > + stride = abs(stride); > > - stride = (n_comp_pixels / height) * 4; > + decomp_buf = (uint8_t *)pixman_image_get_data(lz_data- > >decode_data.out_surface); > if (!top_down) { > - stride = -stride; > - decomp_buf = src + stride * (height - 1); > - } else { > - decomp_buf = src; > + decomp_buf -= stride * (height - 1); > } > > lz_decode(lz_data->lz, as_type, decomp_buf); > > + canvas_fix_alignment(decomp_buf, stride_encoded, stride, > height); > + > if (free_palette) { > free(palette); > } _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel