Re: [Qemu-devel] VNC cross-endian failures

[Troy Benjegerdes]

Much better.

I did have one bogon after running xvncview on a big-endian host,
closing it, then starting it on a little-endian host that resulted in
this from realvnc:

Rect too big: 24832x33024 at 8448,16640 exceeds 640x480
 main:        Rect too big




On Sat, May 13, 2006 at 01:02:47AM +0200, Fabrice Bellard wrote:
> Troy Benjegerdes wrote:
> >The VNC protocol says the server is is supposed to send the data in the
> >format the client wants, however the current implementation sends vnc
> >data in the server native format.
> >
> >What is the best way to fix this? Using -bgr is not right since that
> >will mess up same-endian clients.
> 
> Try the attached patch.
> 
> Fabrice.

> Index: vnc.c
> ===================================================================
> RCS file: /sources/qemu/qemu/vnc.c,v
> retrieving revision 1.5
> diff -u -w -r1.5 vnc.c
> --- vnc.c     3 May 2006 21:18:59 -0000       1.5
> +++ vnc.c     12 May 2006 22:54:21 -0000
> @@ -42,6 +42,14 @@
>  
>  typedef int VncReadEvent(VncState *vs, char *data, size_t len);
>  
> +typedef void VncWritePixels(VncState *vs, void *data, int size);
> +
> +typedef void VncSendHextileTile(VncState *vs,
> +                                int x, int y, int w, int h,
> +                                uint32_t *last_bg, 
> +                                uint32_t *last_fg,
> +                                int *has_bg, int *has_fg);
> +
>  struct VncState
>  {
>      QEMUTimer *timer;
> @@ -53,12 +61,19 @@
>      int height;
>      uint64_t dirty_row[768];
>      char *old_data;
> -    int depth;
> +    int depth; /* internal VNC frame buffer byte per pixel */
>      int has_resize;
>      int has_hextile;
>      Buffer output;
>      Buffer input;
>      kbd_layout_t *kbd_layout;
> +    /* current output mode information */
> +    VncWritePixels *write_pixels;
> +    VncSendHextileTile *send_hextile_tile;
> +    int pix_bpp, pix_big_endian;
> +    int red_shift, red_max, red_shift1;
> +    int green_shift, green_max, green_shift1;
> +    int blue_shift, blue_max, blue_shift1;
>  
>      VncReadEvent *read_handler;
>      size_t read_handler_expect;
> @@ -130,6 +145,66 @@
>      }
>  }
>  
> +/* fastest code */
> +static void vnc_write_pixels_copy(VncState *vs, void *pixels, int size)
> +{
> +    vnc_write(vs, pixels, size);
> +}
> +
> +/* slowest but generic code. */
> +static void vnc_convert_pixel(VncState *vs, uint8_t *buf, uint32_t v)
> +{
> +    unsigned int r, g, b;
> +
> +    r = (v >> vs->red_shift1) & vs->red_max;
> +    g = (v >> vs->green_shift1) & vs->green_max;
> +    b = (v >> vs->blue_shift1) & vs->blue_max;
> +    v = (r << vs->red_shift) | 
> +        (g << vs->green_shift) | 
> +        (b << vs->blue_shift);
> +    switch(vs->pix_bpp) {
> +    case 1:
> +        buf[0] = v;
> +        break;
> +    case 2:
> +        if (vs->pix_big_endian) {
> +            buf[0] = v >> 8;
> +            buf[1] = v;
> +        } else {
> +            buf[1] = v >> 8;
> +            buf[0] = v;
> +        }
> +        break;
> +    default:
> +    case 4:
> +        if (vs->pix_big_endian) {
> +            buf[0] = v >> 24;
> +            buf[1] = v >> 16;
> +            buf[2] = v >> 8;
> +            buf[3] = v;
> +        } else {
> +            buf[3] = v >> 24;
> +            buf[2] = v >> 16;
> +            buf[1] = v >> 8;
> +            buf[0] = v;
> +        }
> +        break;
> +    }
> +}
> +
> +static void vnc_write_pixels_generic(VncState *vs, void *pixels1, int size)
> +{
> +    uint32_t *pixels = pixels1;
> +    uint8_t buf[4];
> +    int n, i;
> +
> +    n = size >> 2;
> +    for(i = 0; i < n; i++) {
> +        vnc_convert_pixel(vs, buf, pixels[i]);
> +        vnc_write(vs, buf, vs->pix_bpp);
> +    }
> +}
> +
>  static void send_framebuffer_update_raw(VncState *vs, int x, int y, int w, 
> int h)
>  {
>      int i;
> @@ -139,7 +214,7 @@
>  
>      row = vs->ds->data + y * vs->ds->linesize + x * vs->depth;
>      for (i = 0; i < h; i++) {
> -     vnc_write(vs, row, w * vs->depth);
> +     vs->write_pixels(vs, row, w * vs->depth);
>       row += vs->ds->linesize;
>      }
>  }
> @@ -162,35 +237,26 @@
>  #include "vnchextile.h"
>  #undef BPP
>  
> +#define GENERIC
> +#define BPP 32
> +#include "vnchextile.h"
> +#undef BPP
> +#undef GENERIC
> +
>  static void send_framebuffer_update_hextile(VncState *vs, int x, int y, int 
> w, int h)
>  {
>      int i, j;
>      int has_fg, has_bg;
>      uint32_t last_fg32, last_bg32;
> -    uint16_t last_fg16, last_bg16;
> -    uint8_t last_fg8, last_bg8;
>  
>      vnc_framebuffer_update(vs, x, y, w, h, 5);
>  
>      has_fg = has_bg = 0;
>      for (j = y; j < (y + h); j += 16) {
>       for (i = x; i < (x + w); i += 16) {
> -         switch (vs->depth) {
> -         case 1:
> -             send_hextile_tile_8(vs, i, j, MIN(16, x + w - i), MIN(16, y + h 
> - j),
> -                                 &last_bg8, &last_fg8, &has_bg, &has_fg);
> -             break;
> -         case 2:
> -             send_hextile_tile_16(vs, i, j, MIN(16, x + w - i), MIN(16, y + 
> h - j),
> -                                  &last_bg16, &last_fg16, &has_bg, &has_fg);
> -             break;
> -         case 4:
> -             send_hextile_tile_32(vs, i, j, MIN(16, x + w - i), MIN(16, y + 
> h - j),
> +            vs->send_hextile_tile(vs, i, j, 
> +                                  MIN(16, x + w - i), MIN(16, y + h - j),
>                                    &last_bg32, &last_fg32, &has_bg, &has_fg);
> -             break;
> -         default:
> -             break;
> -         }
>       }
>      }
>  }
> @@ -660,31 +726,80 @@
>      }
>  }
>  
> +static int compute_nbits(unsigned int val)
> +{
> +    int n;
> +    n = 0;
> +    while (val != 0) {
> +        n++;
> +        val >>= 1;
> +    }
> +    return n;
> +}
> +
>  static void set_pixel_format(VncState *vs,
>                            int bits_per_pixel, int depth,
>                            int big_endian_flag, int true_color_flag,
>                            int red_max, int green_max, int blue_max,
>                            int red_shift, int green_shift, int blue_shift)
>  {
> -    switch (bits_per_pixel) {
> -    case 32:
> -    case 24:
> +    int host_big_endian_flag;
> +
> +#ifdef WORDS_BIGENDIAN
> +    host_big_endian_flag = 1;
> +#else
> +    host_big_endian_flag = 0;
> +#endif
> +    if (!true_color_flag) {
> +    fail:
> +     vnc_client_error(vs);
> +        return;
> +    }
> +    if (bits_per_pixel == 32 && 
> +        host_big_endian_flag == big_endian_flag &&
> +        red_max == 0xff && green_max == 0xff && blue_max == 0xff &&
> +        red_shift == 16 && green_shift == 8 && blue_shift == 0) {
>       vs->depth = 4;
> -     break;
> -    case 16:
> +        vs->write_pixels = vnc_write_pixels_copy;
> +        vs->send_hextile_tile = send_hextile_tile_32;
> +    } else 
> +    if (bits_per_pixel == 16 && 
> +        host_big_endian_flag == big_endian_flag &&
> +        red_max == 31 && green_max == 63 && blue_max == 31 &&
> +        red_shift == 11 && green_shift == 5 && blue_shift == 0) {
>       vs->depth = 2;
> -     break;
> -    case 8:
> +        vs->write_pixels = vnc_write_pixels_copy;
> +        vs->send_hextile_tile = send_hextile_tile_16;
> +    } else 
> +    if (bits_per_pixel == 8 && 
> +        red_max == 7 && green_max == 7 && blue_max == 3 &&
> +        red_shift == 5 && green_shift == 2 && blue_shift == 0) {
>       vs->depth = 1;
> -     break;
> -    default:
> -     vnc_client_error(vs);
> -     break;
> +        vs->write_pixels = vnc_write_pixels_copy;
> +        vs->send_hextile_tile = send_hextile_tile_8;
> +    } else 
> +    {
> +        /* generic and slower case */
> +        if (bits_per_pixel != 8 &&
> +            bits_per_pixel != 16 &&
> +            bits_per_pixel != 32)
> +            goto fail;
> +        vs->depth = 4;
> +        vs->red_shift = red_shift;
> +        vs->red_max = red_max;
> +        vs->red_shift1 = 24 - compute_nbits(red_max);
> +        vs->green_shift = green_shift;
> +        vs->green_max = green_max;
> +        vs->green_shift1 = 16 - compute_nbits(green_max);
> +        vs->blue_shift = blue_shift;
> +        vs->blue_max = blue_max;
> +        vs->blue_shift1 = 8 - compute_nbits(blue_max);
> +        vs->pix_bpp = bits_per_pixel / 8;
> +        vs->pix_big_endian = big_endian_flag;
> +        vs->write_pixels = vnc_write_pixels_generic;
> +        vs->send_hextile_tile = send_hextile_tile_generic;
>      }
>  
> -    if (!true_color_flag)
> -     vnc_client_error(vs);
> -
>      vnc_dpy_resize(vs->ds, vs->ds->width, vs->ds->height);
>      memset(vs->dirty_row, 0xFF, sizeof(vs->dirty_row));
>      memset(vs->old_data, 42, vs->ds->linesize * vs->ds->height);
> @@ -774,7 +889,11 @@
>  
>      vnc_write_u8(vs, vs->depth * 8); /* bits-per-pixel */
>      vnc_write_u8(vs, vs->depth * 8); /* depth */
> +#ifdef WORDS_BIGENDIAN
> +    vnc_write_u8(vs, 1);             /* big-endian-flag */
> +#else
>      vnc_write_u8(vs, 0);             /* big-endian-flag */
> +#endif
>      vnc_write_u8(vs, 1);             /* true-color-flag */
>      if (vs->depth == 4) {
>       vnc_write_u16(vs, 0xFF);     /* red-max */
> @@ -783,6 +902,7 @@
>       vnc_write_u8(vs, 16);        /* red-shift */
>       vnc_write_u8(vs, 8);         /* green-shift */
>       vnc_write_u8(vs, 0);         /* blue-shift */
> +        vs->send_hextile_tile = send_hextile_tile_32;
>      } else if (vs->depth == 2) {
>       vnc_write_u16(vs, 31);       /* red-max */
>       vnc_write_u16(vs, 63);       /* green-max */
> @@ -790,14 +910,18 @@
>       vnc_write_u8(vs, 11);        /* red-shift */
>       vnc_write_u8(vs, 5);         /* green-shift */
>       vnc_write_u8(vs, 0);         /* blue-shift */
> +        vs->send_hextile_tile = send_hextile_tile_16;
>      } else if (vs->depth == 1) {
> -     vnc_write_u16(vs, 3);        /* red-max */
> +        /* XXX: change QEMU pixel 8 bit pixel format to match the VNC one ? 
> */
> +     vnc_write_u16(vs, 7);        /* red-max */
>       vnc_write_u16(vs, 7);        /* green-max */
>       vnc_write_u16(vs, 3);        /* blue-max */
>       vnc_write_u8(vs, 5);         /* red-shift */
>       vnc_write_u8(vs, 2);         /* green-shift */
>       vnc_write_u8(vs, 0);         /* blue-shift */
> +        vs->send_hextile_tile = send_hextile_tile_8;
>      }
> +    vs->write_pixels = vnc_write_pixels_copy;
>       
>      vnc_write(vs, pad, 3);           /* padding */
>  
> Index: vnchextile.h
> ===================================================================
> RCS file: /sources/qemu/qemu/vnchextile.h,v
> retrieving revision 1.1
> diff -u -w -r1.1 vnchextile.h
> --- vnchextile.h      30 Apr 2006 21:28:36 -0000      1.1
> +++ vnchextile.h      12 May 2006 22:54:21 -0000
> @@ -1,15 +1,23 @@
>  #define CONCAT_I(a, b) a ## b
>  #define CONCAT(a, b) CONCAT_I(a, b)
>  #define pixel_t CONCAT(uint, CONCAT(BPP, _t))
> +#ifdef GENERIC
> +#define NAME generic
> +#else
> +#define NAME BPP
> +#endif
>  
> -static void CONCAT(send_hextile_tile_, BPP)(VncState *vs,
> +static void CONCAT(send_hextile_tile_, NAME)(VncState *vs,
>                                           int x, int y, int w, int h,
> -                                         pixel_t *last_bg, pixel_t *last_fg,
> +                                             uint32_t *last_bg32, 
> +                                             uint32_t *last_fg32,
>                                           int *has_bg, int *has_fg)
>  {
>      char *row = (vs->ds->data + y * vs->ds->linesize + x * vs->depth);
>      pixel_t *irow = (pixel_t *)row;
>      int j, i;
> +    pixel_t *last_bg = (pixel_t *)last_bg32;
> +    pixel_t *last_fg = (pixel_t *)last_fg32;
>      pixel_t bg = 0;
>      pixel_t fg = 0;
>      int n_colors = 0;
> @@ -122,10 +130,15 @@
>                   has_color = 1;
>               } else if (irow[i] != color) {
>                   has_color = 0;
> -
> +#ifdef GENERIC
> +                    vnc_convert_pixel(vs, data + n_data, color);
> +                    n_data += vs->pix_bpp;
> +#else
>                   memcpy(data + n_data, &color, sizeof(color));
> -                 hextile_enc_cord(data + n_data + sizeof(pixel_t), min_x, j, 
> i - min_x, 1);
> -                 n_data += 2 + sizeof(pixel_t);
> +                    n_data += sizeof(pixel_t);
> +#endif
> +                 hextile_enc_cord(data + n_data, min_x, j, i - min_x, 1);
> +                 n_data += 2;
>                   n_subtiles++;
>  
>                   min_x = -1;
> @@ -137,9 +150,15 @@
>               }
>           }
>           if (has_color) {
> +#ifdef GENERIC
> +                vnc_convert_pixel(vs, data + n_data, color);
> +                n_data += vs->pix_bpp;
> +#else
>               memcpy(data + n_data, &color, sizeof(color));
> -             hextile_enc_cord(data + n_data + sizeof(pixel_t), min_x, j, i - 
> min_x, 1);
> -             n_data += 2 + sizeof(pixel_t);
> +                n_data += sizeof(pixel_t);
> +#endif
> +             hextile_enc_cord(data + n_data, min_x, j, i - min_x, 1);
> +             n_data += 2;
>               n_subtiles++;
>           }
>           irow += vs->ds->linesize / sizeof(pixel_t);
> @@ -169,21 +188,22 @@
>      vnc_write_u8(vs, flags);
>      if (n_colors < 4) {
>       if (flags & 0x02)
> -         vnc_write(vs, last_bg, sizeof(pixel_t));
> +         vs->write_pixels(vs, last_bg, sizeof(pixel_t));
>       if (flags & 0x04)
> -         vnc_write(vs, last_fg, sizeof(pixel_t));
> +         vs->write_pixels(vs, last_fg, sizeof(pixel_t));
>       if (n_subtiles) {
>           vnc_write_u8(vs, n_subtiles);
>           vnc_write(vs, data, n_data);
>       }
>      } else {
>       for (j = 0; j < h; j++) {
> -         vnc_write(vs, row, w * vs->depth);
> +         vs->write_pixels(vs, row, w * vs->depth);
>           row += vs->ds->linesize;
>       }
>      }
>  }
>  
> +#undef NAME
>  #undef pixel_t
>  #undef CONCAT_I
>  #undef CONCAT

> _______________________________________________
> Qemu-devel mailing list
> address@hidden
> http://lists.nongnu.org/mailman/listinfo/qemu-devel


-- 
--------------------------------------------------------------------------
Troy Benjegerdes                'da hozer'                address@hidden  

Somone asked me why I work on this free (http://www.fsf.org/philosophy/)
software stuff and not get a real job. Charles Shultz had the best answer:

"Why do musicians compose symphonies and poets write poems? They do it
because life wouldn't have any meaning for them if they didn't. That's why
I draw cartoons. It's my life." -- Charles Shultz