From: Kshitij Suri <kshitij.suri@nutanix.com>

Currently screendump only supports PPM format, which is un-compressed. Added
a "format" parameter to QMP and HMP screendump command to support PNG image
capture using libpng.

QMP example usage:
{ "execute": "screendump", "arguments": { "filename": "/tmp/image",
"format":"png" } }

HMP example usage:
screendump /tmp/image -f png

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/718

Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20220408071336.99839-3-kshitij.suri@nutanix.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 monitor/hmp-cmds.c |  12 +++++-
 ui/console.c       | 101 +++++++++++++++++++++++++++++++++++++++++++--
 hmp-commands.hx    |  11 ++---
 qapi/ui.json       |  24 +++++++++--
 4 files changed, 136 insertions(+), 12 deletions(-)

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 634968498b58..2442bfa98984 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1720,9 +1720,19 @@ hmp_screendump(Monitor *mon, const QDict *qdict)
     const char *filename = qdict_get_str(qdict, "filename");
     const char *id = qdict_get_try_str(qdict, "device");
     int64_t head = qdict_get_try_int(qdict, "head", 0);
+    const char *input_format  = qdict_get_try_str(qdict, "format");
     Error *err = NULL;
+    ImageFormat format;
 
-    qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
+    format = qapi_enum_parse(&ImageFormat_lookup, input_format,
+                              IMAGE_FORMAT_PPM, &err);
+    if (err) {
+        goto end;
+    }
+
+    qmp_screendump(filename, id != NULL, id, id != NULL, head,
+                   input_format != NULL, format, &err);
+end:
     hmp_handle_error(mon, err);
 }
 
diff --git a/ui/console.c b/ui/console.c
index 1752f2ec8897..15d0f6affd4c 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -37,6 +37,9 @@
 #include "exec/memory.h"
 #include "io/channel-file.h"
 #include "qom/object.h"
+#ifdef CONFIG_PNG
+#include <png.h>
+#endif
 
 #define DEFAULT_BACKSCROLL 512
 #define CONSOLE_CURSOR_PERIOD 500
@@ -291,6 +294,89 @@ void graphic_hw_invalidate(QemuConsole *con)
     }
 }
 
+#ifdef CONFIG_PNG
+/**
+ * png_save: Take a screenshot as PNG
+ *
+ * Saves screendump as a PNG file
+ *
+ * Returns true for success or false for error.
+ *
+ * @fd: File descriptor for PNG file.
+ * @image: Image data in pixman format.
+ * @errp: Pointer to an error.
+ */
+static bool png_save(int fd, pixman_image_t *image, Error **errp)
+{
+    int width = pixman_image_get_width(image);
+    int height = pixman_image_get_height(image);
+    g_autofree png_struct *png_ptr = NULL;
+    g_autofree png_info *info_ptr = NULL;
+    g_autoptr(pixman_image_t) linebuf =
+                            qemu_pixman_linebuf_create(PIXMAN_a8r8g8b8, width);
+    uint8_t *buf = (uint8_t *)pixman_image_get_data(linebuf);
+    FILE *f = fdopen(fd, "wb");
+    int y;
+    if (!f) {
+        error_setg_errno(errp, errno,
+                         "Failed to create file from file descriptor");
+        return false;
+    }
+
+    png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL,
+                                      NULL, NULL);
+    if (!png_ptr) {
+        error_setg(errp, "PNG creation failed. Unable to write struct");
+        fclose(f);
+        return false;
+    }
+
+    info_ptr = png_create_info_struct(png_ptr);
+
+    if (!info_ptr) {
+        error_setg(errp, "PNG creation failed. Unable to write info");
+        fclose(f);
+        png_destroy_write_struct(&png_ptr, &info_ptr);
+        return false;
+    }
+
+    png_init_io(png_ptr, f);
+
+    png_set_IHDR(png_ptr, info_ptr, width, height, 8,
+                 PNG_COLOR_TYPE_RGB_ALPHA, PNG_INTERLACE_NONE,
+                 PNG_COMPRESSION_TYPE_BASE, PNG_FILTER_TYPE_BASE);
+
+    png_write_info(png_ptr, info_ptr);
+
+    for (y = 0; y < height; ++y) {
+        qemu_pixman_linebuf_fill(linebuf, image, width, 0, y);
+        png_write_row(png_ptr, buf);
+    }
+    qemu_pixman_image_unref(linebuf);
+
+    png_write_end(png_ptr, NULL);
+
+    png_destroy_write_struct(&png_ptr, &info_ptr);
+
+    if (fclose(f) != 0) {
+        error_setg_errno(errp, errno,
+                         "PNG creation failed. Unable to close file");
+        return false;
+    }
+
+    return true;
+}
+
+#else /* no png support */
+
+static bool png_save(int fd, pixman_image_t *image, Error **errp)
+{
+    error_setg(errp, "Enable PNG support with libpng for screendump");
+    return false;
+}
+
+#endif /* CONFIG_PNG */
+
 static bool ppm_save(int fd, pixman_image_t *image, Error **errp)
 {
     int width = pixman_image_get_width(image);
@@ -329,7 +415,8 @@ static void graphic_hw_update_bh(void *con)
 /* Safety: coroutine-only, concurrent-coroutine safe, main thread only */
 void coroutine_fn
 qmp_screendump(const char *filename, bool has_device, const char *device,
-               bool has_head, int64_t head, Error **errp)
+               bool has_head, int64_t head,
+               bool has_format, ImageFormat format, Error **errp)
 {
     g_autoptr(pixman_image_t) image = NULL;
     QemuConsole *con;
@@ -385,8 +472,16 @@ qmp_screendump(const char *filename, bool has_device, const char *device,
      * yields and releases the BQL. It could produce corrupted dump, but
      * it should be otherwise safe.
      */
-    if (!ppm_save(fd, image, errp)) {
-        qemu_unlink(filename);
+    if (has_format && format == IMAGE_FORMAT_PNG) {
+        /* PNG format specified for screendump */
+        if (!png_save(fd, image, errp)) {
+            qemu_unlink(filename);
+        }
+    } else {
+        /* PPM format specified/default for screendump */
+        if (!ppm_save(fd, image, errp)) {
+            qemu_unlink(filename);
+        }
     }
 }
 
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8476277aa9c9..808020d0051b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -244,11 +244,12 @@ ERST
 
     {
         .name       = "screendump",
-        .args_type  = "filename:F,device:s?,head:i?",
-        .params     = "filename [device [head]]",
-        .help       = "save screen from head 'head' of display device 'device' "
-                      "into PPM image 'filename'",
-        .cmd        = hmp_screendump,
+        .args_type  = "filename:F,format:-fs,device:s?,head:i?",
+        .params     = "filename [-f format] [device [head]]",
+        .help       = "save screen from head 'head' of display device 'device'"
+                      "in specified format 'format' as image 'filename'."
+                      "Currently only 'png' and 'ppm' formats are supported.",
+         .cmd        = hmp_screendump,
         .coroutine  = true,
     },
 
diff --git a/qapi/ui.json b/qapi/ui.json
index 13a8bb82aa05..596f37fc37aa 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -157,12 +157,27 @@
 ##
 { 'command': 'expire_password', 'boxed': true, 'data': 'ExpirePasswordOptions' }
 
+##
+# @ImageFormat:
+#
+# Supported image format types.
+#
+# @png: PNG format
+#
+# @ppm: PPM format
+#
+# Since: 7.1
+#
+##
+{ 'enum': 'ImageFormat',
+  'data': ['ppm', 'png'] }
+
 ##
 # @screendump:
 #
-# Write a PPM of the VGA screen to a file.
+# Capture the contents of a screen and write it to a file.
 #
-# @filename: the path of a new PPM file to store the image
+# @filename: the path of a new file to store the image
 #
 # @device: ID of the display device that should be dumped. If this parameter
 #          is missing, the primary display will be used. (Since 2.12)
@@ -171,6 +186,8 @@
 #        parameter is missing, head #0 will be used. Also note that the head
 #        can only be specified in conjunction with the device ID. (Since 2.12)
 #
+# @format: image format for screendump. (default: ppm) (Since 7.1)
+#
 # Returns: Nothing on success
 #
 # Since: 0.14
@@ -183,7 +200,8 @@
 #
 ##
 { 'command': 'screendump',
-  'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+  'data': {'filename': 'str', '*device': 'str', '*head': 'int',
+           '*format': 'ImageFormat'},
   'coroutine': true }
 
 ##
-- 
2.35.1

On Wed, 27 Apr 2022 at 18:33, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> From: Kshitij Suri <kshitij.suri@nutanix.com>
>
> Currently screendump only supports PPM format, which is un-compressed. Added
> a "format" parameter to QMP and HMP screendump command to support PNG image
> capture using libpng.
>
> QMP example usage:
> { "execute": "screendump", "arguments": { "filename": "/tmp/image",
> "format":"png" } }
>
> HMP example usage:
> screendump /tmp/image -f png
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/718
>
> Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Acked-by: Markus Armbruster <armbru@redhat.com>
> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Message-Id: <20220408071336.99839-3-kshitij.suri@nutanix.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

This change seems to have broken building the system emulator
with --disable-vnc unless I also add --disable-png. Specifically:

> diff --git a/ui/console.c b/ui/console.c
> index 1752f2ec8897..15d0f6affd4c 100644
> --- a/ui/console.c
> +++ b/ui/console.c

> +    png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL,
> +                                      NULL, NULL);

ui/console.c now makes calls to functions from libpng, but
the commit didn't do anything in ui/meson.build to tell
the build system that this object file must now be linked
with libpng. Because vnc *does* correctly state its dependency
on libpng, builds that happen to include vnc will link OK
by accident, but builds without vnc will fail:

$ mkdir build/tst
$ (cd build/tst && ../../configure --target-list=i386-softmmu
--disable-tools --disable-vnc --enable-debug)
$ make -C build/tst -j8
[...]
/usr/bin/ld: libcommon.fa.p/ui_console.c.o: undefined reference to
symbol 'png_init_io@@PNG16_0'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libpng16.so.16: error adding
symbols: DSO missing from command line
collect2: error: ld returned 1 exit status

thanks
-- PMM

Hi,

I have sent the fix out at 
https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg01980.html.
Would be grateful for your review.

Regards,

Kshitij Suri

On 06/05/22 7:07 pm, Peter Maydell wrote:
> On Wed, 27 Apr 2022 at 18:33, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> From: Kshitij Suri <kshitij.suri@nutanix.com>
>>
>> Currently screendump only supports PPM format, which is un-compressed. Added
>> a "format" parameter to QMP and HMP screendump command to support PNG image
>> capture using libpng.
>>
>> QMP example usage:
>> { "execute": "screendump", "arguments": { "filename": "/tmp/image",
>> "format":"png" } }
>>
>> HMP example usage:
>> screendump /tmp/image -f png
>>
>> Resolves: https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_qemu-2Dproject_qemu_-2D_issues_718&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w&m=0pq2lv0SrogWXtqObS7lzBUp-Bbw4WYpJvheEOrvaMJrptl2ORkHDcDVKGuyG95V&s=T8YakRJNie_5nF8LIdJvsiSogf9iS7Ytlc205JIcaYQ&e=
>>
>> Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
>>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> Acked-by: Markus Armbruster <armbru@redhat.com>
>> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Message-Id: <20220408071336.99839-3-kshitij.suri@nutanix.com>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> This change seems to have broken building the system emulator
> with --disable-vnc unless I also add --disable-png. Specifically:
>
>> diff --git a/ui/console.c b/ui/console.c
>> index 1752f2ec8897..15d0f6affd4c 100644
>> --- a/ui/console.c
>> +++ b/ui/console.c
>> +    png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL,
>> +                                      NULL, NULL);
> ui/console.c now makes calls to functions from libpng, but
> the commit didn't do anything in ui/meson.build to tell
> the build system that this object file must now be linked
> with libpng. Because vnc *does* correctly state its dependency
> on libpng, builds that happen to include vnc will link OK
> by accident, but builds without vnc will fail:
>
> $ mkdir build/tst
> $ (cd build/tst && ../../configure --target-list=i386-softmmu
> --disable-tools --disable-vnc --enable-debug)
> $ make -C build/tst -j8
> [...]
> /usr/bin/ld: libcommon.fa.p/ui_console.c.o: undefined reference to
> symbol 'png_init_io@@PNG16_0'
> /usr/bin/ld: /usr/lib/x86_64-linux-gnu/libpng16.so.16: error adding
> symbols: DSO missing from command line
> collect2: error: ld returned 1 exit status
>
> thanks
> -- PMM