[Spice-commits] server/jpeg_encoder.c server/mjpeg_encoder.c

[Frediano Ziglio]

 server/jpeg_encoder.c  |   16 ++++++++--------
 server/mjpeg_encoder.c |   10 ++++++----
 2 files changed, 14 insertions(+), 12 deletions(-)

New commits:
commit bdeef8b292ab2e3edea8e8ab9683d56d59d29953
Author: Victor Toso <victortoso at redhat.com>
Date:   Fri Aug 14 18:24:27 2015 +0200

    mjpeg and jpeg encoder: fix alignment warnings
    
    As the input line could be uint8_t*, uint16_t* or uint32_t*, changing
    the default from uint8_t* to void* seems the correct choice to deal with
    upcasting warnings.
    
    Regarding chunks->data allocation, I quote Frediano explantion:
    "Lines came from spice_bitmap_get_line. This function assume that bitmap
    data is split among chunks each containing some lines
    (always full lines). If chunk->data is allocated using malloc or similar
    SHOULD (not 100% sure) be 4 bytes aligned so in our cases
    (8, 16, 24 or 32 bit images) should be aligned enough.
    
    All the casts unfortunately came from the fact we compute based on
    pixel bytes to make it generic so we use uint8_t*."
    
    and
    
    "Looking at code looks like these chunks came from the virtual machine.
    So the question is... why should the virtual machine give use some
    not-pixel align data?
    I would put a large comment to state that we assume VM send aligned
    data, would be stupid for the VM to not align it!"
    
    clang output:
    jpeg_encoder.c:109:26: error: cast from 'uint8_t *'
    (aka 'unsigned char *') to 'uint16_t *' (aka 'unsigned short *')
    increases required alignment from 1 to 2 [-Werror,-Wcast-align]
      uint16_t *src_line = (uint16_t *)line;
                           ^~~~~~~~~~~~~~~~
    
    jpeg_encoder.c:144:26: error: cast from 'uint8_t *'
    (aka 'unsigned char *') to 'uint32_t *' (aka 'unsigned int *')
    increases required alignment from 1 to 4 [-Werror,-Wcast-align]
      uint32_t *src_line = (uint32_t *)line;
                           ^~~~~~~~~~~~~~~~
    
    mjpeg_encoder.c:260:23: error: cast from 'uint8_t *'
    (aka 'unsigned char *') to 'uint16_t *' (aka 'unsigned short *')
    increases required alignment from 1 to 2 [-Werror,-Wcast-align]
      uint16_t pixel = *(uint16_t *)src;
                        ^~~~~~~~~~~~~~~

diff --git a/server/jpeg_encoder.c b/server/jpeg_encoder.c
index 01732ff..0296e9b 100644
--- a/server/jpeg_encoder.c
+++ b/server/jpeg_encoder.c
@@ -35,7 +35,7 @@ typedef struct JpegEncoder {
         int height;
         int stride;
         unsigned int out_size;
-        void (*convert_line_to_RGB24) (uint8_t *line, int width, uint8_t **out_line);
+        void (*convert_line_to_RGB24) (void *line, int width, uint8_t **out_line);
     } cur_image;
 } JpegEncoder;
 
@@ -104,9 +104,9 @@ void jpeg_encoder_destroy(JpegEncoderContext* encoder)
     free(encoder);
 }
 
-static void convert_RGB16_to_RGB24(uint8_t *line, int width, uint8_t **out_line)
+static void convert_RGB16_to_RGB24(void *line, int width, uint8_t **out_line)
 {
-    uint16_t *src_line = (uint16_t *)line;
+    uint16_t *src_line = line;
     uint8_t *out_pix;
     int x;
 
@@ -122,11 +122,11 @@ static void convert_RGB16_to_RGB24(uint8_t *line, int width, uint8_t **out_line)
    }
 }
 
-static void convert_BGR24_to_RGB24(uint8_t *line, int width, uint8_t **out_line)
+static void convert_BGR24_to_RGB24(void *in_line, int width, uint8_t **out_line)
 {
     int x;
     uint8_t *out_pix;
-
+    uint8_t *line = in_line;
     spice_assert(out_line && *out_line);
 
     out_pix = *out_line;
@@ -139,9 +139,9 @@ static void convert_BGR24_to_RGB24(uint8_t *line, int width, uint8_t **out_line)
     }
 }
 
-static void convert_BGRX32_to_RGB24(uint8_t *line, int width, uint8_t **out_line)
+static void convert_BGRX32_to_RGB24(void *line, int width, uint8_t **out_line)
 {
-    uint32_t *src_line = (uint32_t *)line;
+    uint32_t *src_line = line;
     uint8_t *out_pix;
     int x;
 
@@ -157,7 +157,7 @@ static void convert_BGRX32_to_RGB24(uint8_t *line, int width, uint8_t **out_line
     }
 }
 
-static void convert_RGB24_to_RGB24(uint8_t *line, int width, uint8_t **out_line)
+static void convert_RGB24_to_RGB24(void *line, int width, uint8_t **out_line)
 {
     *out_line = line;
 }
diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 9a41ef3..4b803a9 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -162,7 +162,7 @@ struct MJpegEncoder {
     struct jpeg_error_mgr jerr;
 
     unsigned int bytes_per_pixel; /* bytes per pixel of the input buffer */
-    void (*pixel_converter)(uint8_t *src, uint8_t *dest);
+    void (*pixel_converter)(void *src, uint8_t *dest);
 
     MJpegEncoderRateControl rate_control;
     MJpegEncoderRateControlCbs cbs;
@@ -238,15 +238,16 @@ uint8_t mjpeg_encoder_get_bytes_per_pixel(MJpegEncoder *encoder)
 
 #ifndef JCS_EXTENSIONS
 /* Pixel conversion routines */
-static void pixel_rgb24bpp_to_24(uint8_t *src, uint8_t *dest)
+static void pixel_rgb24bpp_to_24(void *src_ptr, uint8_t *dest)
 {
+    uint8_t *src = src_ptr;
     /* libjpegs stores rgb, spice/win32 stores bgr */
     *dest++ = src[2]; /* red */
     *dest++ = src[1]; /* green */
     *dest++ = src[0]; /* blue */
 }
 
-static void pixel_rgb32bpp_to_24(uint8_t *src, uint8_t *dest)
+static void pixel_rgb32bpp_to_24(void *src, uint8_t *dest)
 {
     uint32_t pixel = *(uint32_t *)src;
     *dest++ = (pixel >> 16) & 0xff;
@@ -255,7 +256,7 @@ static void pixel_rgb32bpp_to_24(uint8_t *src, uint8_t *dest)
 }
 #endif
 
-static void pixel_rgb16bpp_to_24(uint8_t *src, uint8_t *dest)
+static void pixel_rgb16bpp_to_24(void *src, uint8_t *dest)
 {
     uint16_t pixel = *(uint16_t *)src;
     *dest++ = ((pixel >> 7) & 0xf8) | ((pixel >> 12) & 0x7);
@@ -842,6 +843,7 @@ int mjpeg_encoder_encode_scanline(MJpegEncoder *encoder, uint8_t *src_pixels,
     if (encoder->pixel_converter) {
         unsigned int x;
         for (x = 0; x < image_width; x++) {
+            /* src_pixels is expected to be 4 bytes aligned */
             encoder->pixel_converter(src_pixels, row);
             row += 3;
             src_pixels += encoder->bytes_per_pixel;