From: Troy Benjegerdes <hozer@hozed.org>
To: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] VNC cross-endian failures
Date: Fri, 12 May 2006 19:08:31 -0500 [thread overview]
Message-ID: <20060513000814.GQ15855@narn.hozed.org> (raw)
In-Reply-To: <44651417.4010005@bellard.org>
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
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel
--
--------------------------------------------------------------------------
Troy Benjegerdes 'da hozer' hozer@hozed.org
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
next prev parent reply other threads:[~2006-05-13 0:08 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-05-12 20:00 [Qemu-devel] VNC cross-endian failures Troy Benjegerdes
2006-05-12 22:21 ` Fabrice Bellard
2006-05-12 23:02 ` Fabrice Bellard
2006-05-13 0:08 ` Troy Benjegerdes [this message]
2006-05-13 0:20 ` Troy Benjegerdes
2006-05-13 2:32 ` Anthony Liguori
-- strict thread matches above, loose matches on Subject: below --
2006-05-13 0:33 Ben Taylor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20060513000814.GQ15855@narn.hozed.org \
--to=hozer@hozed.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).