[Qemu-devel] [PATCH] PNG screendump alternative

[Qemu-devel] [PATCH] PNG screendump alternative

[Rafael]

[-- Attachment #1: Type: text/plain, Size: 736 bytes --]

Hi,

I  had already started to work on supporting PNG dump output on
screendump when got to know there was another effort to do the same
(see Add PNG screendump thread from Daniel P. Berrange). Anyway, I'd
like to share my implementation which is a little bit different. It
does not use any resource/lib besides libpng and qemu itself and is
based on qemu-kvm 0.15.1 ppm_save code.

I'm using that for a personal project developed at Mandriva which uses
QMP directly to communicate with qemu VMs rather than using the
libvirt although its usage is considered for the future. There is
something more to show but it is just QMP related and soon I'll have
it done to share.

Comments and suggestions are appreciated.

Cheers
Rafael Cabral

[-- Attachment #2: enable-screendump-png.patch --]
[-- Type: application/octet-stream, Size: 6370 bytes --]

--- configure	2011-10-19 13:54:48.000000000 +0000
+++ configure	2012-03-15 13:33:26.000000000 +0000
@@ -141,6 +141,7 @@
 attr=""
 vhost_net=""
 xfs=""
+dump_png=""
 
 gprof="no"
 debug_tcg="no"
@@ -790,6 +791,10 @@
   ;;
   --disable-guest-agent) guest_agent="no"
   ;;
+  --disable-dump-png) dump_png="no"
+  ;;
+  --enable-dump-png) dump_png="yes"
+  ;;
   *) echo "ERROR: unknown option $opt"; show_help="yes"
   ;;
   esac
@@ -1075,6 +1080,8 @@
 echo "  --enable-usb-redir       enable usb network redirection support"
 echo "  --disable-guest-agent    disable building of the QEMU Guest Agent"
 echo "  --enable-guest-agent     enable building of the QEMU Guest Agent"
+echo "  --disable-dump-png       disable PNG for screendump output"
+echo "  --enable-dump-png        enable PNG for screendump output"
 echo ""
 echo "NOTE: The object files are built at the place where configure is launched"
 exit 1
@@ -2610,6 +2617,40 @@
 fi
 
 ##########################################
+# DUMP PNG detection
+if test "$dump_png" = "yes" ; then
+cat > $TMPC <<EOF
+//#include <stdio.h>
+#include <png.h>
+#include <stddef.h>
+int main(void) {
+    png_structp png_ptr;
+    png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
+    return 0;
+}
+EOF
+  if $pkg_config libpng --modversion >/dev/null 2>&1; then
+    dump_png_cflags=`$pkg_config libpng --cflags 2> /dev/null`
+    dump_png_libs=`$pkg_config libpng --libs 2> /dev/null`
+  else
+    dump_png_cflags=""
+    dump_png_libs="-lpng"
+  fi
+  if compile_prog "$dump_png_cflags" "$dump_png_libs" ; then
+    dump_png=yes
+    libs_softmmu="$dump_png_libs $libs_softmmu"
+  else
+    if test "dump_png" = "yes" ; then
+      feature_not_found "dump-png"
+    fi
+    dump_png=no
+  fi
+fi
+#########################################
+
+
+
+##########################################
 # End of CC checks
 # After here, no more $cc or $ld runs
 
@@ -2776,6 +2817,7 @@
 echo "usb net redir     $usb_redir"
 echo "OpenGL support    $opengl"
 echo "build guest agent $guest_agent"
+echo "Dump png $dump_png"
 
 if test $sdl_too_old = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -3083,6 +3125,11 @@
   echo "CONFIG_OPENGL=y" >> $config_host_mak
 fi
 
+if test "$dump_png" = "yes" ; then
+  echo "CONFIG_DUMP_PNG=y" >> $config_host_mak
+  echo "DUMP_PNG_CFLAGS=$dump_png_cflags" >> $config_host_mak
+fi
+
 # XXX: suppress that
 if [ "$bsd" = "yes" ] ; then
   echo "CONFIG_BSD=y" >> $config_host_mak
--- hw/vga.c	2011-10-19 13:54:48.000000000 +0000
+++ hw/vga.c	2012-03-17 21:53:12.000000000 +0000
@@ -29,6 +29,10 @@
 #include "pixel_ops.h"
 #include "qemu-timer.h"
 
+#ifdef CONFIG_DUMP_PNG
+#include <png.h>
+#endif
+
 //#define DEBUG_VGA
 //#define DEBUG_VGA_MEM
 //#define DEBUG_VGA_REG
@@ -2329,7 +2333,14 @@
                                 int x, int y, int w, int h)
 {
     if (screen_dump_filename) {
-        ppm_save(screen_dump_filename, ds->surface);
+#ifdef CONFIG_DUMP_PNG
+		if (is_png_extension(screen_dump_filename))
+			png_save(screen_dump_filename, ds->surface);
+		else
+			ppm_save(screen_dump_filename, ds->surface);
+#else
+		ppm_save(screen_dump_filename, ds->surface);
+#endif
         screen_dump_filename = NULL;
     }
 }
@@ -2342,6 +2353,95 @@
 {
 }
 
+#ifdef CONFIG_DUMP_PNG
+int is_png_extension(const char * filename)
+{
+    if (!(filename == NULL)) {
+        char * c;
+        for (c = filename;*c != NULL; ++c);
+        c -= 4;
+        if (*c++ == '.' && *c++ == 'p' && *c++ == 'n' && *c == 'g')
+            return 1;
+    }
+    return 0;
+}
+
+int png_save(const char *filename, struct DisplaySurface *ds)
+{
+    FILE *f;
+    uint8_t *d, *d1;
+    uint32_t v;
+    int y, x;
+    uint8_t r, g, b;
+    // int ret;
+
+    f = fopen(filename, "wb");
+    if (!f)
+        return -1;
+
+	/* initialize stuff */
+	png_structp png_ptr;
+    png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
+	if (!png_ptr)
+		return -1;
+
+	png_infop info_ptr;
+    info_ptr = png_create_info_struct(png_ptr);
+    if (!info_ptr)
+		return -1;
+
+	/* write header */
+    if (setjmp(png_jmpbuf(png_ptr)))
+		return -1;
+
+	// bit depth 16
+    png_set_IHDR(png_ptr, info_ptr, ds->width, ds->height,
+                 8, PNG_COLOR_TYPE_RGB, PNG_INTERLACE_NONE,
+                 PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
+    
+	/* Process ds */
+    png_byte ** row_pointers = png_malloc(png_ptr, ds->height * sizeof(png_byte *));
+    d1 = ds->data;
+    for(y = 0; y < ds->height; y++) {
+		row_pointers[y] = (png_byte *) png_malloc(png_ptr, sizeof(uint8_t) * ds->width * 3);
+        png_byte * row = row_pointers[y];
+        d = d1;
+        for(x = 0; x < ds->width; x++) {
+            if (ds->pf.bits_per_pixel == 32) {
+                v = *(uint32_t *)d;
+			}
+            else {
+                v = (uint32_t) (*(uint16_t *)d);
+			}
+            r = ((v >> ds->pf.rshift) & ds->pf.rmax) * 256 /
+                (ds->pf.rmax + 1);
+            g = ((v >> ds->pf.gshift) & ds->pf.gmax) * 256 /
+                (ds->pf.gmax + 1);
+            b = ((v >> ds->pf.bshift) & ds->pf.bmax) * 256 /
+                (ds->pf.bmax + 1);
+			*row++ = r;
+			*row++ = g;
+			*row++ = b;
+            d += ds->pf.bytes_per_pixel;
+        }
+        d1 += ds->linesize;
+    }
+
+	png_init_io(png_ptr, f);
+	png_set_rows(png_ptr, info_ptr, row_pointers);
+    png_write_png(png_ptr, info_ptr, PNG_TRANSFORM_IDENTITY, NULL);
+
+    /* cleanup heap allocation */
+    for (y=0; y<ds->height; y++)
+        png_free(png_ptr, row_pointers[y]);
+    png_free(png_ptr, row_pointers);
+    fclose(f);
+	return 0;
+}
+#endif
+
 int ppm_save(const char *filename, struct DisplaySurface *ds)
 {
     FILE *f;
--- hw/vga_int.h	2011-10-19 13:54:48.000000000 +0000
+++ hw/vga_int.h	2012-03-17 22:19:38.000000000 +0000
@@ -205,6 +205,10 @@
 void vga_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val);
 void vga_invalidate_scanlines(VGACommonState *s, int y1, int y2);
 int ppm_save(const char *filename, struct DisplaySurface *ds);
+#ifdef CONFIG_DUMP_PNG
+int is_png_extension(const char * filename);
+int png_save(const char *filename, struct DisplaySurface *ds);
+#endif
 
 void vga_draw_cursor_line_8(uint8_t *d1, const uint8_t *src1,
                             int poffset, int w,

Re: [Qemu-devel] [PATCH] PNG screendump alternative

[Luiz Capitulino]

On Sat, 17 Mar 2012 19:45:56 -0300
Rafael <verganic@gmail.com> wrote:

> Hi,
> 
> I  had already started to work on supporting PNG dump output on
> screendump when got to know there was another effort to do the same
> (see Add PNG screendump thread from Daniel P. Berrange). Anyway, I'd
> like to share my implementation which is a little bit different. It
> does not use any resource/lib besides libpng and qemu itself and is
> based on qemu-kvm 0.15.1 ppm_save code.

There are a few problems with this patch. First, it should be against qemu.git
and 0.15.1 is a bit of an old release (we're moving towards 1.1).

Also, I'm not sure that we should completely rely on the file extension,
maybe we should have an argument for it...

Anyway, I recommend you join the efforts for the new screendump command
along with Marc-Andre and Alon.

> 
> I'm using that for a personal project developed at Mandriva which uses
> QMP directly to communicate with qemu VMs rather than using the
> libvirt although its usage is considered for the future. There is
> something more to show but it is just QMP related and soon I'll have
> it done to share.
> 
> Comments and suggestions are appreciated.
> 
> Cheers
> Rafael Cabral

Re: [Qemu-devel] [PATCH] PNG screendump alternative

[Anthony Liguori]

On 03/17/2012 05:45 PM, Rafael wrote:
> Hi,
>
> I  had already started to work on supporting PNG dump output on
> screendump when got to know there was another effort to do the same
> (see Add PNG screendump thread from Daniel P. Berrange). Anyway, I'd
> like to share my implementation which is a little bit different. It
> does not use any resource/lib besides libpng and qemu itself and is
> based on qemu-kvm 0.15.1 ppm_save code.
>
> I'm using that for a personal project developed at Mandriva which uses
> QMP directly to communicate with qemu VMs rather than using the
> libvirt although its usage is considered for the future. There is
> something more to show but it is just QMP related and soon I'll have
> it done to share.
>
> Comments and suggestions are appreciated.

I think the added value of this is minimal compared to the alternative 
implementation (just run convert on the PPM).

Can you talk a bit more about your use-case any why post-processing the PPM 
isn't a reasonable alternative?

Regards,

Anthony Liguori

>
> Cheers
> Rafael Cabral

Re: [Qemu-devel] [PATCH] PNG screendump alternative

[Rafael da Veiga Cabral]

On 03/19/2012 05:50 PM, Luiz Capitulino wrote:
> On Sat, 17 Mar 2012 19:45:56 -0300
> Rafael<verganic@gmail.com>  wrote:
>
>> Hi,
>>
>> I  had already started to work on supporting PNG dump output on
>> screendump when got to know there was another effort to do the same
>> (see Add PNG screendump thread from Daniel P. Berrange). Anyway, I'd
>> like to share my implementation which is a little bit different. It
>> does not use any resource/lib besides libpng and qemu itself and is
>> based on qemu-kvm 0.15.1 ppm_save code.
> There are a few problems with this patch. First, it should be against qemu.git
> and 0.15.1 is a bit of an old release (we're moving towards 1.1).

Ok, it's old but pretty stable component for my unstable project, but as 
you've mentioned would be a
good try to use 1.1 version and towards.

>
> Also, I'm not sure that we should completely rely on the file extension,
> maybe we should have an argument for it...

eg.: screendump -<png|tiff|jpeg> filename=path/file ?
and the ppm format as fallback.

>
> Anyway, I recommend you join the efforts for the new screendump command
> along with Marc-Andre and Alon.

Great, by increasing the solution where my proposal fits.

>> I'm using that for a personal project developed at Mandriva which uses
>> QMP directly to communicate with qemu VMs rather than using the
>> libvirt although its usage is considered for the future. There is
>> something more to show but it is just QMP related and soon I'll have
>> it done to share.
>>
>> Comments and suggestions are appreciated.
>>
>> Cheers
>> Rafael Cabral
>>

Comments appreciated, thanks Capitulino.
Rafael Cabral

Re: [Qemu-devel] [PATCH] PNG screendump alternative

[Rafael da Veiga Cabral]

On 03/19/2012 06:10 PM, Anthony Liguori wrote:
> On 03/17/2012 05:45 PM, Rafael wrote:
>> Hi,
>>
>> I  had already started to work on supporting PNG dump output on
>> screendump when got to know there was another effort to do the same
>> (see Add PNG screendump thread from Daniel P. Berrange). Anyway, I'd
>> like to share my implementation which is a little bit different. It
>> does not use any resource/lib besides libpng and qemu itself and is
>> based on qemu-kvm 0.15.1 ppm_save code.
>>
>> I'm using that for a personal project developed at Mandriva which uses
>> QMP directly to communicate with qemu VMs rather than using the
>> libvirt although its usage is considered for the future. There is
>> something more to show but it is just QMP related and soon I'll have
>> it done to share.
>>
>> Comments and suggestions are appreciated.
>
> I think the added value of this is minimal compared to the alternative 
> implementation (just run convert on the PPM).
>
> Can you talk a bit more about your use-case any why post-processing 
> the PPM isn't a reasonable alternative?
>

It's because my use-case requires a screenshoot on each second or less. 
It's a functional testing tool based on what is happening on the vm 
screen. The testing stuff  is based on basic image processing of the 
screens. Therefore as screen are grabbed all time the less CPU and Disk 
I/O are required the better,  for example, by changing from PPM to PNG 
at least 2.2 Megas of disk writing are saved on each second. Think about 
a session of 15 minutes or more  (15 * 60 * 2.2).  I've also thought 
that by saving directly from the screen buffer to png should save some 
CPU cycles rather than saving to PPM first and converting it to PNG on 
the qemu runtime.


> Regards,
>
> Anthony Liguori
>
>>
>> Cheers
>> Rafael Cabral
>

Thanks Anthony
Rafael Cabral