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

[Fabrice Bellard]

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