[Spice-devel] [spice-server PATCH v1 4/12] mjpeg_encoder: fix alignment warnings (pixel_converter)

[Frediano Ziglio]

Not sure about it. 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* but clang complaints (well, that's why there are explicit casts, to make compiler aware we know we are doing the right thing).

Frediano

> 
> Hey,
> 
> In this case, we are going to pass unaligned data to pixel_converter(),
> so I'd say the warning is legit and we are just papering over it:
> 
> uint8_t *row;
> for (x = 0; x < image_width; x++) {
>     encoder->pixel_converter(src_pixels, row);
>     row += 3;
>     src_pixels += encoder->bytes_per_pixel;
> }
> 
> Christophe
> 
> On Wed, Aug 05, 2015 at 02:23:18PM +0200, Victor Toso wrote:
> > As the input line could be uint8_t*, uint16_t*, uint32_t*, changing the
> > default from uint8_t* to void* to deal with upcasting warnings.
> > 
> > from clang:
> > 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;
> >                     ^~~~~~~~~~~~~~~
> > ---
> >  server/mjpeg_encoder.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
> > index 9a41ef3..1d1f167 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);
> > --
> > 2.4.3
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>