[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] VNC cross-endian failures
|
From: |
Ben Taylor |
|
Subject: |
Re: [Qemu-devel] VNC cross-endian failures |
|
Date: |
Fri, 12 May 2006 20:33:51 -0400 |
---- Troy Benjegerdes <address@hidden> wrote:
> 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
>From a solaris sparc host, guesting DamnSmallLinux, the intial colors
looked ok, but then transformed into something less pleasing, and then
it started crashing realvnc viewer 4.1.1 (solaris x86 client) with rect too
big and other errors. (In fact, I could restart the realvnc viewer and get
the screen redrawn and then it crashed again - basically unusable at
that point)
I then tried connecting back to the session with tightvnc viewer V1.2.8, and
the colors were right and the tightvnc viewer didn't start crashing.
Pretty odd. Seems like we're close.
Ben
>
>
>
>
> 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
>
>
> _______________________________________________
> Qemu-devel mailing list
> address@hidden
> http://lists.nongnu.org/mailman/listinfo/qemu-devel