From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MWsq3-0004Dn-7u for qemu-devel@nongnu.org; Fri, 31 Jul 2009 10:12:03 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MWspy-0004CO-DM for qemu-devel@nongnu.org; Fri, 31 Jul 2009 10:12:02 -0400 Received: from [199.232.76.173] (port=41150 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MWspy-0004CL-88 for qemu-devel@nongnu.org; Fri, 31 Jul 2009 10:11:58 -0400 Received: from mail-yw0-f190.google.com ([209.85.211.190]:37274) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MWspx-000849-TW for qemu-devel@nongnu.org; Fri, 31 Jul 2009 10:11:58 -0400 Received: by ywh28 with SMTP id 28so2119348ywh.27 for ; Fri, 31 Jul 2009 07:11:55 -0700 (PDT) Message-ID: <4A72FBA7.6000309@codemonkey.ws> Date: Fri, 31 Jul 2009 09:11:51 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1249024897-11100-1-git-send-email-agraf@suse.de> In-Reply-To: <1249024897-11100-1-git-send-email-agraf@suse.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH] Add JPEG encoding to VNC server List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: kraxel@redhat.com, qemu-devel@nongnu.org, stefano.stabellini@eu.citrix.com Alexander Graf wrote: > We know most of the tight protocol already, so implementing JPEG is rather > easy, especially considering that I had this implementation lying around > still anyways. > > The big concern Anthony raised about JPEG compression is that as soon as you > use JPEG, CopyRect looks ugly for most use cases and most users probably don't > want it anyways. > > So the road I went for this patch was to only enable JPEG encoding when it's > the only available choice. Allow any other protocols? You don't get JPEG then. > > While this might sound like it renders the whole implementation useless, it > does make sense to implement it nevertheless. I have some ideas to implement > progressive encodings for video. > > So when we'd detect that one region is updated a lot in a short about of time > with content that zlib can't really handle well, we'd just send a really low > quality JPEG first and then send the update after a timer if the region wasn't > updated within that timeframe. > > But this is all ideas so far. For now the JPEG implementation is stand alone, > but would enable either me or someone else who'd like to do it worlds of > encoding fun :-). > > Also, if you're daring, you can always see if JPEG performs good for your > specific VNC workload and at least have the chance to use it. > Most clients selectively enable the jpeg part of tight by specifying a jpeg quality encoding. They usually don't do this by default. What I would like to see, is that we respect the quality encoding and if tight is requested without a jpeg quality, we send another type of tight update (even the compressed raw tiles would be fine). It's important for this to be useful with normal clients and for general assumptions about how servers deal with tight to be maintained. > Signed-off-by: Alexander Graf > --- > Makefile.target | 4 + > configure | 24 ++++++++ > vnc.c | 178 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- > vnc.h | 9 +++ > 4 files changed, 214 insertions(+), 1 deletions(-) > > diff --git a/Makefile.target b/Makefile.target > index 49ba08d..f1dd54d 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -303,6 +303,10 @@ CPPFLAGS += $(VNC_SASL_CFLAGS) > LIBS += $(VNC_SASL_LIBS) > endif > > +ifdef CONFIG_VNC_JPEG > just CONFIG_JPEG please. > +vnc_jpeg="yes" just jpeg. > ########################################## > +# VNC JPEG detection > +if test "$vnc_jpeg" = "yes" ; then > +cat > $TMPC < +#include > +int main(void) { jpeg_compress_struct s; jpeg_create_compress(&s); return 0; } > +EOF > + vnc_jpeg_libs="-ljpeg" > + if $cc $ARCH_CFLAGS -o $TMPE ${OS_CFLAGS} $TMPC $vnc_jpeg_libs > /dev/null 2> /dev/null ; then > + : > + else > + vnc_tls="no" > That's a typeo. > +#ifdef CONFIG_VNC_JPEG > How difficult would it be to introduce a vnc-tight.c? vnc.h has all the necessary structure definitions. That way, we can just avoid compiling vnc-tight.c if we don't have libjpeg instead of having tons of ifdefs. > +/* This is called once per encoding */ > +static void jpeg_init_destination(j_compress_ptr cinfo) > +{ > + VncState *vs = (VncState*)cinfo->client_data; > + Buffer *buffer = &vs->jpeg_buffer; > + > + cinfo->dest->next_output_byte = (JOCTET *)buffer->buffer + buffer->offset; > + cinfo->dest->free_in_buffer = (size_t)(buffer->capacity - buffer->offset); > +} > + > +/* This is called when we ran out of buffer (shouldn't happen!) */ > +static boolean jpeg_empty_output_buffer(j_compress_ptr cinfo) > +{ > + VncState *vs = (VncState*)cinfo->client_data; > + Buffer *buffer = &vs->jpeg_buffer; > + > + buffer->offset = buffer->capacity; > + buffer_reserve(buffer, 2048); > + jpeg_init_destination(cinfo); > + return TRUE; > +} > + > +/* This is called when we are done processing data */ > +static void jpeg_term_destination(j_compress_ptr cinfo) > +{ > + VncState *vs = (VncState*)cinfo->client_data; > + Buffer *buffer = &vs->jpeg_buffer; > + > + buffer->offset = buffer->capacity - cinfo->dest->free_in_buffer; > +} > + > + > +static void vnc_send_compact_size(VncState *vs, int len) > +{ > + char buf[3]; > + int lpc = 0; > + int bytes = 0; > + > + /* Adapted from SendCompressedData() in Xvnc/programs/Xserver/hw/vnc/tight.c */ > + buf[bytes++] = len & 0x7F; > + if (len > 0x7F) { > + buf[bytes-1] |= 0x80; > + buf[bytes++] = len >> 7 & 0x7F; > + if (len > 0x3FFF) { > + buf[bytes-1] |= 0x80; > + buf[bytes++] = len >> 14 & 0xFF; > + } > + } > + > + for(lpc = 0; lpc < bytes; lpc++) { > + vnc_write_u8(vs, buf[lpc]); > + } > +} > + > +static void jpeg_row2pixel(VncState *vs, char *in, char *out, int len) > +{ > + char *pi = in; > + char *po = out; > + int depth = vs->server.ds->pf.bytes_per_pixel; > + int i; > + > + for (i = 0; i < len; i++) { > + uint32_t v; > + uint8_t r, g, b; > + switch (depth) { > + case 1: > + po[0] = pi[0]; > + po[1] = pi[0]; > + po[2] = pi[0]; > + continue; > + break; > + case 2: > + v = *((uint16_t*)pi); > + break; > + case 4: > + v = *((uint32_t*)pi); > + break; > + } > + r = ((((v & vs->server.ds->pf.rmask) >> vs->server.ds->pf.rshift) > + << vs->clientds.pf.rbits) >> vs->server.ds->pf.rbits); > + g = ((((v & vs->server.ds->pf.gmask) >> vs->server.ds->pf.gshift) > + << vs->clientds.pf.gbits) >> vs->server.ds->pf.gbits); > + b = ((((v & vs->server.ds->pf.bmask) >> vs->server.ds->pf.bshift) > + << vs->clientds.pf.bbits) >> vs->server.ds->pf.bbits); > + > + po[0] = r; > + po[1] = g; > + po[2] = b; > + > + pi += depth; > + po += 3; // RGB > No C99 comments please. > + } > +} > + > +static void send_framebuffer_update_tight(VncState *vs, int x, int y, int w, int h) > +{ > + struct jpeg_compress_struct cinfo; > + struct jpeg_error_mgr jerr; > + uint8_t *row = ds_get_data(vs->ds) + > + y * ds_get_linesize(vs->ds) + > + x * ds_get_bytes_per_pixel(vs->ds); > + int dy; > + JSAMPROW row_pointer[1]; > + > + if(vnc_has_feature(vs, VNC_FEATURE_HEXTILE) && (w * h) < 300) { > Watch the formatting. After Stefano's series, this patch needs an update because vs->server no longer exists. Regards, Anthony Liguori