[PATCH] Added parameter to take screenshot with screendump as PNG

2022-02-22 Thread Kshitij Suri
Currently screendump only supports PPM format, which is un-compressed and not
standard. Added an "format" parameter to qemu monitor screendump capabilites
to support PNG image capture using libpng. The param was added in QAPI schema
of screendump present in ui.json along with png_save() function which converts
pixman_image to PNG. HMP command equivalent was also modified to support the
feature.

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

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

Signed-off-by: Kshitij Suri 
---
 hmp-commands.hx|  11 ++--
 monitor/hmp-cmds.c |   4 +-
 qapi/ui.json   |   7 ++-
 ui/console.c   | 153 -
 4 files changed, 165 insertions(+), 10 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 70a9136ac2..2163337f35 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -244,17 +244,18 @@ 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'",
+.args_type  = "filename:F,device:s?,head:i?,format:f?",
+.params = "filename [device [head]] [format]",
+.help   = "save screen from head 'head' of display device 'device'"
+  "in specified format 'format' as image 'filename'."
+  "Default format for screendump is PPM.",
 .cmd= hmp_screendump,
 .coroutine  = true,
 },
 
 SRST
 ``screendump`` *filename*
-  Save screen into PPM image *filename*.
+  Save screen as image *filename*.
 ERST
 
 {
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 2669156b28..3fb1394561 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1665,9 +1665,11 @@ 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 *format  = qdict_get_str(qdict, "format");
 Error *err = NULL;
 
-qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
+qmp_screendump(filename, id != NULL, id, id != NULL, head,
+   format != NULL, format, &err);
 hmp_handle_error(mon, err);
 }
 
diff --git a/qapi/ui.json b/qapi/ui.json
index 9354f4c467..9fdb56b60b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -76,7 +76,7 @@
 ##
 # @screendump:
 #
-# Write a PPM of the VGA screen to a file.
+# Write a screenshot of the VGA screen to a file.
 #
 # @filename: the path of a new PPM file to store the image
 #
@@ -87,6 +87,9 @@
 #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 is specified. Currently only PNG and
+# PPM are supported.
+#
 # Returns: Nothing on success
 #
 # Since: 0.14
@@ -99,7 +102,7 @@
 #
 ##
 { 'command': 'screendump',
-  'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+  'data': {'filename': 'str', '*device': 'str', '*head': 'int', '*format': 
'str'},
   'coroutine': true }
 
 ##
diff --git a/ui/console.c b/ui/console.c
index 40eebb6d2c..7813b195ac 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_VNC_PNG
+#include "png.h"
+#endif
 
 #define DEFAULT_BACKSCROLL 512
 #define CONSOLE_CURSOR_PERIOD 500
@@ -289,6 +292,137 @@ void graphic_hw_invalidate(QemuConsole *con)
 }
 }
 
+#ifdef CONFIG_VNC_PNG
+/**
+ * a8r8g8b8_to_rgba: Convert a8r8g8b8 to rgba format
+ *
+ * @dst: Destination pointer.
+ * @src: Source pointer.
+ * @n_pixels: Size of image.
+ */
+static void a8r8g8b8_to_rgba(uint32_t *dst, uint32_t *src, int n_pixels)
+{
+uint8_t *dst8 = (uint8_t *)dst;
+int i;
+
+for (i = 0; i < n_pixels; ++i) {
+uint32_t p = src[i];
+uint8_t a, r, g, b;
+
+a = (p & 0xff00) >> 24;
+r = (p & 0x00ff) >> 16;
+g = (p & 0xff00) >> 8;
+b = (p & 0x00ff) >> 0;
+
+if (a != 0) {
+#define DIVIDE(c, a) \
+do { \
+int t = ((c) * 255) / a; \
+(c) = t < 0 ? 0 : t > 255 ? 255 : t; \
+} while (0)
+
+DIVIDE(r, a);
+DIVIDE(g, a);
+DIVIDE(b, a);
+#undef DIVIDE
+}
+
+*dst8++ = r;
+*dst8++ = g;
+*dst8++ = b;
+*dst8++ = a;
+}
+}
+
+/**
+ * png_save: Take a screenshot as PNG
+ *
+ * Saves screendump as a PNG file
+ *
+ * Returns true for success or false for error.
+ * Inspired from png test utils from https://github.com/aseprite/pixman
+ *
+ 

Re: [PATCH] Added parameter to take screenshot with screendump as PNG

2022-02-22 Thread Daniel P . Berrangé
On Tue, Feb 22, 2022 at 03:27:58PM +, Kshitij Suri wrote:
> Currently screendump only supports PPM format, which is un-compressed and not
> standard. Added an "format" parameter to qemu monitor screendump capabilites
> to support PNG image capture using libpng. The param was added in QAPI schema
> of screendump present in ui.json along with png_save() function which converts
> pixman_image to PNG. HMP command equivalent was also modified to support the
> feature.
> 
> Example usage:
> { "execute": "screendump", "arguments": { "filename": "/tmp/image", 
> "format":"png" } }
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/718
> 
> Signed-off-by: Kshitij Suri 
> ---
>  hmp-commands.hx|  11 ++--
>  monitor/hmp-cmds.c |   4 +-
>  qapi/ui.json   |   7 ++-
>  ui/console.c   | 153 -
>  4 files changed, 165 insertions(+), 10 deletions(-)

You're going to need to update configure here.

Currently libpng is only detected if VNC is enabled.

This proposed change means we need to detect libpng
any time the build of system emulators is enabled.

The CONFIG_VNC_PNG option will also need renaming
to CONFIG_PNG

> +.args_type  = "filename:F,device:s?,head:i?,format:f?",
> +.params = "filename [device [head]] [format]",
> +.help   = "save screen from head 'head' of display device 
> 'device'"
> +  "in specified format 'format' as image 'filename'."
> +  "Default format for screendump is PPM.",

Mention what other formats are permitted, making it clear that
the format is in fact 'png' and 'ppm', not 'PPM'

> diff --git a/qapi/ui.json b/qapi/ui.json
> index 9354f4c467..9fdb56b60b 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -76,7 +76,7 @@
>  ##
>  # @screendump:
>  #
> -# Write a PPM of the VGA screen to a file.
> +# Write a screenshot of the VGA screen to a file.
>  #
>  # @filename: the path of a new PPM file to store the image
>  #
> @@ -87,6 +87,9 @@
>  #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 is specified. Currently only PNG and
> +# PPM are supported.
> +#
>  # Returns: Nothing on success
>  #
>  # Since: 0.14
> @@ -99,7 +102,7 @@
>  #
>  ##
>  { 'command': 'screendump',
> -  'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
> +  'data': {'filename': 'str', '*device': 'str', '*head': 'int', '*format': 
> 'str'},

This 'format' should not be a string, it needs to be an enum type.

>'coroutine': true }
>  
>  ##
> diff --git a/ui/console.c b/ui/console.c
> index 40eebb6d2c..7813b195ac 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_VNC_PNG
> +#include "png.h"
> +#endif
>  
>  #define DEFAULT_BACKSCROLL 512
>  #define CONSOLE_CURSOR_PERIOD 500
> @@ -289,6 +292,137 @@ void graphic_hw_invalidate(QemuConsole *con)
>  }
>  }
>  
> +#ifdef CONFIG_VNC_PNG
> +/**
> + * a8r8g8b8_to_rgba: Convert a8r8g8b8 to rgba format
> + *
> + * @dst: Destination pointer.
> + * @src: Source pointer.
> + * @n_pixels: Size of image.
> + */
> +static void a8r8g8b8_to_rgba(uint32_t *dst, uint32_t *src, int n_pixels)
> +{
> +uint8_t *dst8 = (uint8_t *)dst;
> +int i;
> +
> +for (i = 0; i < n_pixels; ++i) {
> +uint32_t p = src[i];
> +uint8_t a, r, g, b;
> +
> +a = (p & 0xff00) >> 24;
> +r = (p & 0x00ff) >> 16;
> +g = (p & 0xff00) >> 8;
> +b = (p & 0x00ff) >> 0;
> +
> +if (a != 0) {
> +#define DIVIDE(c, a) \
> +do { \
> +int t = ((c) * 255) / a; \
> +(c) = t < 0 ? 0 : t > 255 ? 255 : t; \
> +} while (0)
> +
> +DIVIDE(r, a);
> +DIVIDE(g, a);
> +DIVIDE(b, a);
> +#undef DIVIDE
> +}
> +
> +*dst8++ = r;
> +*dst8++ = g;
> +*dst8++ = b;
> +*dst8++ = a;
> +}
> +}
> +
> +/**
> + * png_save: Take a screenshot as PNG
> + *
> + * Saves screendump as a PNG file
> + *
> + * Returns true for success or false for error.
> + * Inspired from png test utils from https://github.com/aseprite/pixman
> + *
> + * @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);
> +int stride = width * 4;
> +g_autofree uint32_t *src_data = g_malloc(height * stride);
> +g_autofree uint32_t *dest_data = g_malloc(height * stride);
> +g_autoptr(pixman_image_t) src_copy;
> +g_autoptr(pixman_image_t) dest_copy;
> +g_autofree 

Re: [PATCH] Added parameter to take screenshot with screendump as PNG

2022-02-23 Thread Dr. David Alan Gilbert
* Kshitij Suri (kshitij.s...@nutanix.com) wrote:
> Currently screendump only supports PPM format, which is un-compressed and not
> standard. Added an "format" parameter to qemu monitor screendump capabilites
> to support PNG image capture using libpng. The param was added in QAPI schema
> of screendump present in ui.json along with png_save() function which converts
> pixman_image to PNG. HMP command equivalent was also modified to support the
> feature.
> 
> Example usage:
> { "execute": "screendump", "arguments": { "filename": "/tmp/image", 
> "format":"png" } }
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/718
> 
> Signed-off-by: Kshitij Suri 
> ---
>  hmp-commands.hx|  11 ++--
>  monitor/hmp-cmds.c |   4 +-
>  qapi/ui.json   |   7 ++-
>  ui/console.c   | 153 -
>  4 files changed, 165 insertions(+), 10 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 70a9136ac2..2163337f35 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -244,17 +244,18 @@ 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'",
> +.args_type  = "filename:F,device:s?,head:i?,format:f?",
> +.params = "filename [device [head]] [format]",
> +.help   = "save screen from head 'head' of display device 
> 'device'"
> +  "in specified format 'format' as image 'filename'."
> +  "Default format for screendump is PPM.",
>  .cmd= hmp_screendump,
>  .coroutine  = true,
>  },
>  
>  SRST
>  ``screendump`` *filename*
> -  Save screen into PPM image *filename*.
> +  Save screen as image *filename*.
>  ERST
>  
>  {
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 2669156b28..3fb1394561 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1665,9 +1665,11 @@ 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 *format  = qdict_get_str(qdict, "format");
>  Error *err = NULL;
>  
> -qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
> +qmp_screendump(filename, id != NULL, id, id != NULL, head,
> +   format != NULL, format, &err);
>  hmp_handle_error(mon, err);
>  }

I think that's OK from the HMP front, see some questions below.

> diff --git a/qapi/ui.json b/qapi/ui.json
> index 9354f4c467..9fdb56b60b 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -76,7 +76,7 @@
>  ##
>  # @screendump:
>  #
> -# Write a PPM of the VGA screen to a file.
> +# Write a screenshot of the VGA screen to a file.
>  #
>  # @filename: the path of a new PPM file to store the image
>  #
> @@ -87,6 +87,9 @@
>  #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 is specified. Currently only PNG and
> +# PPM are supported.
> +#
>  # Returns: Nothing on success
>  #
>  # Since: 0.14
> @@ -99,7 +102,7 @@
>  #
>  ##
>  { 'command': 'screendump',
> -  'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
> +  'data': {'filename': 'str', '*device': 'str', '*head': 'int', '*format': 
> 'str'},
>'coroutine': true }
>  
>  ##
> diff --git a/ui/console.c b/ui/console.c
> index 40eebb6d2c..7813b195ac 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_VNC_PNG
> +#include "png.h"
> +#endif
>  
>  #define DEFAULT_BACKSCROLL 512
>  #define CONSOLE_CURSOR_PERIOD 500
> @@ -289,6 +292,137 @@ void graphic_hw_invalidate(QemuConsole *con)
>  }
>  }
>  
> +#ifdef CONFIG_VNC_PNG
> +/**
> + * a8r8g8b8_to_rgba: Convert a8r8g8b8 to rgba format
> + *
> + * @dst: Destination pointer.
> + * @src: Source pointer.
> + * @n_pixels: Size of image.
> + */
> +static void a8r8g8b8_to_rgba(uint32_t *dst, uint32_t *src, int n_pixels)
> +{
> +uint8_t *dst8 = (uint8_t *)dst;
> +int i;
> +
> +for (i = 0; i < n_pixels; ++i) {
> +uint32_t p = src[i];
> +uint8_t a, r, g, b;
> +
> +a = (p & 0xff00) >> 24;
> +r = (p & 0x00ff) >> 16;
> +g = (p & 0xff00) >> 8;
> +b = (p & 0x00ff) >> 0;
> +
> +if (a != 0) {
> +#define DIVIDE(c, a) \
> +do { \
> +int t = ((c) * 255) / a; \
> +(c) = t < 0 ? 0 : t > 255 ? 255 : t; \

I can't see how 't' can be less than 0 here?

> +  

Re: [PATCH] Added parameter to take screenshot with screendump as PNG

2022-02-23 Thread Daniel P . Berrangé
On Tue, Feb 22, 2022 at 03:27:58PM +, Kshitij Suri wrote:
> Currently screendump only supports PPM format, which is un-compressed and not
> standard. Added an "format" parameter to qemu monitor screendump capabilites
> to support PNG image capture using libpng. The param was added in QAPI schema
> of screendump present in ui.json along with png_save() function which converts
> pixman_image to PNG. HMP command equivalent was also modified to support the
> feature.
> 
> Example usage:
> { "execute": "screendump", "arguments": { "filename": "/tmp/image", 
> "format":"png" } }
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/718
> 
> Signed-off-by: Kshitij Suri 
> ---
>  hmp-commands.hx|  11 ++--
>  monitor/hmp-cmds.c |   4 +-
>  qapi/ui.json   |   7 ++-
>  ui/console.c   | 153 -
>  4 files changed, 165 insertions(+), 10 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 70a9136ac2..2163337f35 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -244,17 +244,18 @@ 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'",
> +.args_type  = "filename:F,device:s?,head:i?,format:f?",
> +.params = "filename [device [head]] [format]",
> +.help   = "save screen from head 'head' of display device 
> 'device'"
> +  "in specified format 'format' as image 'filename'."
> +  "Default format for screendump is PPM.",
>  .cmd= hmp_screendump,
>  .coroutine  = true,
>  },
>  
>  SRST
>  ``screendump`` *filename*
> -  Save screen into PPM image *filename*.
> +  Save screen as image *filename*.
>  ERST
>  
>  {
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 2669156b28..3fb1394561 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1665,9 +1665,11 @@ 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 *format  = qdict_get_str(qdict, "format");
>  Error *err = NULL;
>  
> -qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
> +qmp_screendump(filename, id != NULL, id, id != NULL, head,
> +   format != NULL, format, &err);
>  hmp_handle_error(mon, err);
>  }
>  
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 9354f4c467..9fdb56b60b 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -76,7 +76,7 @@
>  ##
>  # @screendump:
>  #
> -# Write a PPM of the VGA screen to a file.
> +# Write a screenshot of the VGA screen to a file.
>  #
>  # @filename: the path of a new PPM file to store the image
>  #
> @@ -87,6 +87,9 @@
>  #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 is specified. Currently only PNG and
> +# PPM are supported.
> +#
>  # Returns: Nothing on success
>  #
>  # Since: 0.14
> @@ -99,7 +102,7 @@
>  #
>  ##
>  { 'command': 'screendump',
> -  'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
> +  'data': {'filename': 'str', '*device': 'str', '*head': 'int', '*format': 
> 'str'},
>'coroutine': true }
>  
>  ##
> diff --git a/ui/console.c b/ui/console.c
> index 40eebb6d2c..7813b195ac 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_VNC_PNG
> +#include "png.h"
> +#endif
>  
>  #define DEFAULT_BACKSCROLL 512
>  #define CONSOLE_CURSOR_PERIOD 500
> @@ -289,6 +292,137 @@ void graphic_hw_invalidate(QemuConsole *con)
>  }
>  }
>  
> +#ifdef CONFIG_VNC_PNG
> +/**
> + * a8r8g8b8_to_rgba: Convert a8r8g8b8 to rgba format
> + *
> + * @dst: Destination pointer.
> + * @src: Source pointer.
> + * @n_pixels: Size of image.
> + */
> +static void a8r8g8b8_to_rgba(uint32_t *dst, uint32_t *src, int n_pixels)
> +{
> +uint8_t *dst8 = (uint8_t *)dst;
> +int i;
> +
> +for (i = 0; i < n_pixels; ++i) {
> +uint32_t p = src[i];
> +uint8_t a, r, g, b;
> +
> +a = (p & 0xff00) >> 24;
> +r = (p & 0x00ff) >> 16;
> +g = (p & 0xff00) >> 8;
> +b = (p & 0x00ff) >> 0;
> +
> +if (a != 0) {
> +#define DIVIDE(c, a) \
> +do { \
> +int t = ((c) * 255) / a; \
> +(c) = t < 0 ? 0 : t > 255 ? 255 : t; \
> +} while (0)
> +
> +DIVIDE(r, a);
> +DIVIDE(g, a);
> +D

Re: [PATCH] Added parameter to take screenshot with screendump as PNG

2022-02-24 Thread Kshitij Suri


On 22/02/22 10:04 pm, Daniel P. Berrangé wrote:

On Tue, Feb 22, 2022 at 03:27:58PM +, Kshitij Suri wrote:

Currently screendump only supports PPM format, which is un-compressed and not
standard. Added an "format" parameter to qemu monitor screendump capabilites
to support PNG image capture using libpng. The param was added in QAPI schema
of screendump present in ui.json along with png_save() function which converts
pixman_image to PNG. HMP command equivalent was also modified to support the
feature.

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

Resolves:https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_qemu-2Dproject_qemu_-2D_issues_718&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w&m=eOSxOmlj_wVOfj7HF2fJ1VXlNNJp4k8UAlT5STcoAguScPKeHYwb6hwEuY54ok5c&s=ou3KExLg2-Cx31UtMV6vXErHpesdEHJjHXnbKcE9Wdk&e=  


Signed-off-by: Kshitij Suri
---
  hmp-commands.hx|  11 ++--
  monitor/hmp-cmds.c |   4 +-
  qapi/ui.json   |   7 ++-
  ui/console.c   | 153 -
  4 files changed, 165 insertions(+), 10 deletions(-)

You're going to need to update configure here.

Currently libpng is only detected if VNC is enabled.

This proposed change means we need to detect libpng
any time the build of system emulators is enabled.

The CONFIG_VNC_PNG option will also need renaming
to CONFIG_PNG


Yes I have added a new CONFIG_PNG option in the updated patch. As for 
CONFIG_VNC_PNG, can we have a seperate patch set


where I can replace CONFIG_VNC_PNG usage with combination of CONFIG_VNC 
and CONFIG_PNG? The replacement strategy would be as follows


#ifdef CONFIG_VNC_PNG -> #if defined(CONFIG_VNC) && defined(CONFIG_PNG)

Along with this, can we also use the new png meson-option to denote PNG 
format for VNC as well while maintaining backward compatibility? The 
change would look like


vnc_png = dependency('libpng', required: get_option('vnc_png'), method: 
'pkg-config', kwargs: static_kwargs)


gets changed to

vnc_png = dependency('libpng', required: get_option('vnc_png') || 
get_option('png'),
method: 'pkg-config', kwargs: static_kwargs)




+.args_type  = "filename:F,device:s?,head:i?,format:f?",
+.params = "filename [device [head]] [format]",
+.help   = "save screen from head 'head' of display device 'device'"
+  "in specified format 'format' as image 'filename'."
+  "Default format for screendump is PPM.",

Mention what other formats are permitted, making it clear that
the format is in fact 'png' and 'ppm', not 'PPM'

Updated in the v2 patch.

diff --git a/qapi/ui.json b/qapi/ui.json
index 9354f4c467..9fdb56b60b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -76,7 +76,7 @@
  ##
  # @screendump:
  #
-# Write a PPM of the VGA screen to a file.
+# Write a screenshot of the VGA screen to a file.
  #
  # @filename: the path of a new PPM file to store the image
  #
@@ -87,6 +87,9 @@
  #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 is specified. Currently only PNG and
+# PPM are supported.
+#
  # Returns: Nothing on success
  #
  # Since: 0.14
@@ -99,7 +102,7 @@
  #
  ##
  { 'command': 'screendump',
-  'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+  'data': {'filename': 'str', '*device': 'str', '*head': 'int', '*format': 
'str'},

This 'format' should not be a string, it needs to be an enum type.

Modified to an enum with png and ppm types.



'coroutine': true }
  
  ##

diff --git a/ui/console.c b/ui/console.c
index 40eebb6d2c..7813b195ac 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_VNC_PNG
+#include "png.h"
+#endif
  
  #define DEFAULT_BACKSCROLL 512

  #define CONSOLE_CURSOR_PERIOD 500
@@ -289,6 +292,137 @@ void graphic_hw_invalidate(QemuConsole *con)
  }
  }
  
+#ifdef CONFIG_VNC_PNG

+/**
+ * a8r8g8b8_to_rgba: Convert a8r8g8b8 to rgba format
+ *
+ * @dst: Destination pointer.
+ * @src: Source pointer.
+ * @n_pixels: Size of image.
+ */
+static void a8r8g8b8_to_rgba(uint32_t *dst, uint32_t *src, int n_pixels)
+{
+uint8_t *dst8 = (uint8_t *)dst;
+int i;
+
+for (i = 0; i < n_pixels; ++i) {
+uint32_t p = src[i];
+uint8_t a, r, g, b;
+
+a = (p & 0xff00) >> 24;
+r = (p & 0x00ff) >> 16;
+g = (p & 0xff00) >> 8;
+b = (p & 0x00ff) >> 0;
+
+if (a != 0) {
+#define DIVIDE(c, a) \
+do { \
+int t = ((c) * 255) / a; \
+(c) = t < 0 ? 0 : t > 255 ? 255 : t; \
+} while (0)
+
+DIVIDE(r, a);
+DIVIDE(g, a);
+DIVID

Re: [PATCH] Added parameter to take screenshot with screendump as PNG

2022-02-24 Thread Kshitij Suri



On 23/02/22 4:30 pm, Dr. David Alan Gilbert wrote:

* Kshitij Suri (kshitij.s...@nutanix.com) wrote:

Currently screendump only supports PPM format, which is un-compressed and not
standard. Added an "format" parameter to qemu monitor screendump capabilites
to support PNG image capture using libpng. The param was added in QAPI schema
of screendump present in ui.json along with png_save() function which converts
pixman_image to PNG. HMP command equivalent was also modified to support the
feature.

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

Resolves: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_qemu-2Dproject_qemu_-2D_issues_718&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w&m=YAoUy9TYXWb8lktv5hZwRWjZEBcoHKxfdrHMd8KkTnQAESANHEcW96C1ukSngzJ2&s=xJP6q9IE0req5UkOFLablyJWWREdmwS1NX4Yse0pYxE&e=

Signed-off-by: Kshitij Suri 
---
  hmp-commands.hx|  11 ++--
  monitor/hmp-cmds.c |   4 +-
  qapi/ui.json   |   7 ++-
  ui/console.c   | 153 -
  4 files changed, 165 insertions(+), 10 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 70a9136ac2..2163337f35 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -244,17 +244,18 @@ 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'",
+.args_type  = "filename:F,device:s?,head:i?,format:f?",
+.params = "filename [device [head]] [format]",
+.help   = "save screen from head 'head' of display device 'device'"
+  "in specified format 'format' as image 'filename'."
+  "Default format for screendump is PPM.",
  .cmd= hmp_screendump,
  .coroutine  = true,
  },
  
  SRST

  ``screendump`` *filename*
-  Save screen into PPM image *filename*.
+  Save screen as image *filename*.
  ERST
  
  {

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 2669156b28..3fb1394561 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1665,9 +1665,11 @@ 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 *format  = qdict_get_str(qdict, "format");
  Error *err = NULL;
  
-qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);

+qmp_screendump(filename, id != NULL, id, id != NULL, head,
+   format != NULL, format, &err);
  hmp_handle_error(mon, err);
  }

I think that's OK from the HMP front, see some questions below.


diff --git a/qapi/ui.json b/qapi/ui.json
index 9354f4c467..9fdb56b60b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -76,7 +76,7 @@
  ##
  # @screendump:
  #
-# Write a PPM of the VGA screen to a file.
+# Write a screenshot of the VGA screen to a file.
  #
  # @filename: the path of a new PPM file to store the image
  #
@@ -87,6 +87,9 @@
  #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 is specified. Currently only PNG and
+# PPM are supported.
+#
  # Returns: Nothing on success
  #
  # Since: 0.14
@@ -99,7 +102,7 @@
  #
  ##
  { 'command': 'screendump',
-  'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+  'data': {'filename': 'str', '*device': 'str', '*head': 'int', '*format': 
'str'},
'coroutine': true }
  
  ##

diff --git a/ui/console.c b/ui/console.c
index 40eebb6d2c..7813b195ac 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_VNC_PNG
+#include "png.h"
+#endif
  
  #define DEFAULT_BACKSCROLL 512

  #define CONSOLE_CURSOR_PERIOD 500
@@ -289,6 +292,137 @@ void graphic_hw_invalidate(QemuConsole *con)
  }
  }
  
+#ifdef CONFIG_VNC_PNG

+/**
+ * a8r8g8b8_to_rgba: Convert a8r8g8b8 to rgba format
+ *
+ * @dst: Destination pointer.
+ * @src: Source pointer.
+ * @n_pixels: Size of image.
+ */
+static void a8r8g8b8_to_rgba(uint32_t *dst, uint32_t *src, int n_pixels)
+{
+uint8_t *dst8 = (uint8_t *)dst;
+int i;
+
+for (i = 0; i < n_pixels; ++i) {
+uint32_t p = src[i];
+uint8_t a, r, g, b;
+
+a = (p & 0xff00) >> 24;
+r = (p & 0x00ff) >> 16;
+g = (p & 0xff00) >> 8;
+b = (p & 0x00ff) >> 0;
+
+if (a != 0) {
+#define DIVIDE(c, a) \
+do { \
+int t = ((c) * 255) / a; \
+(c) = t < 0 ? 0 : t > 255 ?