* [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
@ 2022-03-22 10:49 Kshitij Suri
2022-03-22 10:49 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
2022-03-28 9:49 ` [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG Kshitij Suri
0 siblings, 2 replies; 23+ messages in thread
From: Kshitij Suri @ 2022-03-22 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: soham.ghosh, thuth, berrange, prerna.saxena, dgilbert, armbru,
Kshitij Suri, philippe.mathieu.daude, kraxel, prachatos.mitra,
eblake
Libpng is only detected if VNC is enabled currently. This patch adds a
generalised png option in the meson build which is aimed to replace use of
CONFIG_VNC_PNG with CONFIG_PNG.
Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
meson.build | 9 ++++-----
meson_options.txt | 4 ++--
ui/vnc-enc-tight.c | 18 +++++++++---------
ui/vnc.c | 4 ++--
ui/vnc.h | 2 +-
5 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/meson.build b/meson.build
index 282e7c4650..ccb6840a49 100644
--- a/meson.build
+++ b/meson.build
@@ -1115,14 +1115,13 @@ if gtkx11.found()
x11 = dependency('x11', method: 'pkg-config', required: gtkx11.found(),
kwargs: static_kwargs)
endif
+png = dependency('libpng', required: get_option('png'),
+ method: 'pkg-config', kwargs: static_kwargs)
vnc = not_found
-png = not_found
jpeg = not_found
sasl = not_found
if get_option('vnc').allowed() and have_system
vnc = declare_dependency() # dummy dependency
- png = dependency('libpng', required: get_option('vnc_png'),
- method: 'pkg-config', kwargs: static_kwargs)
jpeg = dependency('libjpeg', required: get_option('vnc_jpeg'),
method: 'pkg-config', kwargs: static_kwargs)
sasl = cc.find_library('sasl2', has_headers: ['sasl/sasl.h'],
@@ -1554,9 +1553,9 @@ config_host_data.set('CONFIG_TPM', have_tpm)
config_host_data.set('CONFIG_USB_LIBUSB', libusb.found())
config_host_data.set('CONFIG_VDE', vde.found())
config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', have_vhost_user_blk_server)
+config_host_data.set('CONFIG_PNG', png.found())
config_host_data.set('CONFIG_VNC', vnc.found())
config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
-config_host_data.set('CONFIG_VNC_PNG', png.found())
config_host_data.set('CONFIG_VNC_SASL', sasl.found())
config_host_data.set('CONFIG_VIRTFS', have_virtfs)
config_host_data.set('CONFIG_VTE', vte.found())
@@ -3638,11 +3637,11 @@ summary_info += {'curses support': curses}
summary_info += {'virgl support': virgl}
summary_info += {'curl support': curl}
summary_info += {'Multipath support': mpathpersist}
+summary_info += {'PNG support': png}
summary_info += {'VNC support': vnc}
if vnc.found()
summary_info += {'VNC SASL support': sasl}
summary_info += {'VNC JPEG support': jpeg}
- summary_info += {'VNC PNG support': png}
endif
if targetos not in ['darwin', 'haiku', 'windows']
summary_info += {'OSS support': oss}
diff --git a/meson_options.txt b/meson_options.txt
index 52b11cead4..d85734f8e6 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -177,12 +177,12 @@ option('vde', type : 'feature', value : 'auto',
description: 'vde network backend support')
option('virglrenderer', type : 'feature', value : 'auto',
description: 'virgl rendering support')
+option('png', type : 'feature', value : 'auto',
+ description: 'PNG support with libpng')
option('vnc', type : 'feature', value : 'auto',
description: 'VNC server')
option('vnc_jpeg', type : 'feature', value : 'auto',
description: 'JPEG lossy compression for VNC server')
-option('vnc_png', type : 'feature', value : 'auto',
- description: 'PNG compression for VNC server')
option('vnc_sasl', type : 'feature', value : 'auto',
description: 'SASL authentication for VNC server')
option('vte', type : 'feature', value : 'auto',
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 7b86a4713d..e879cca7f5 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -32,7 +32,7 @@
INT32 definitions between jmorecfg.h (included by jpeglib.h) and
Win32 basetsd.h (included by windows.h). */
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
/* The following define is needed by pngconf.h. Otherwise it won't compile,
because setjmp.h was already included by qemu-common.h. */
#define PNG_SKIP_SETJMP_CHECK
@@ -95,7 +95,7 @@ static const struct {
};
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
static const struct {
int png_zlib_level, png_filters;
} tight_png_conf[] = {
@@ -919,7 +919,7 @@ static int send_full_color_rect(VncState *vs, int x, int y, int w, int h)
int stream = 0;
ssize_t bytes;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
return send_png_rect(vs, x, y, w, h, NULL);
}
@@ -966,7 +966,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
int stream = 1;
int level = tight_conf[vs->tight->compression].mono_zlib_level;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
int ret;
int bpp = vs->client_pf.bytes_per_pixel * 8;
@@ -1020,7 +1020,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
struct palette_cb_priv {
VncState *vs;
uint8_t *header;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
png_colorp png_palette;
#endif
};
@@ -1082,7 +1082,7 @@ static int send_palette_rect(VncState *vs, int x, int y,
int colors;
ssize_t bytes;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
return send_png_rect(vs, x, y, w, h, palette);
}
@@ -1233,7 +1233,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality)
/*
* PNG compression stuff.
*/
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
static void write_png_palette(int idx, uint32_t pix, void *opaque)
{
struct palette_cb_priv *priv = opaque;
@@ -1379,7 +1379,7 @@ static int send_png_rect(VncState *vs, int x, int y, int w, int h,
buffer_reset(&vs->tight->png);
return 1;
}
-#endif /* CONFIG_VNC_PNG */
+#endif /* CONFIG_PNG */
static void vnc_tight_start(VncState *vs)
{
@@ -1706,7 +1706,7 @@ void vnc_tight_clear(VncState *vs)
#ifdef CONFIG_VNC_JPEG
buffer_free(&vs->tight->jpeg);
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
buffer_free(&vs->tight->png);
#endif
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 310a873c21..8376291b47 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2165,7 +2165,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
vs->features |= VNC_FEATURE_TIGHT_MASK;
vs->vnc_encoding = enc;
break;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
case VNC_ENCODING_TIGHT_PNG:
vs->features |= VNC_FEATURE_TIGHT_PNG_MASK;
vs->vnc_encoding = enc;
@@ -3256,7 +3256,7 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
#ifdef CONFIG_VNC_JPEG
buffer_init(&vs->tight->jpeg, "vnc-tight-jpeg/%p", sioc);
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
buffer_init(&vs->tight->png, "vnc-tight-png/%p", sioc);
#endif
buffer_init(&vs->zlib.zlib, "vnc-zlib/%p", sioc);
diff --git a/ui/vnc.h b/ui/vnc.h
index a7149831f9..a60fb13115 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -201,7 +201,7 @@ typedef struct VncTight {
#ifdef CONFIG_VNC_JPEG
Buffer jpeg;
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
Buffer png;
#endif
int levels[4];
--
2.22.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
2022-03-22 10:49 [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG Kshitij Suri
@ 2022-03-22 10:49 ` Kshitij Suri
2022-03-28 9:50 ` Kshitij Suri
2022-03-28 9:52 ` Daniel P. Berrangé
2022-03-28 9:49 ` [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG Kshitij Suri
1 sibling, 2 replies; 23+ messages in thread
From: Kshitij Suri @ 2022-03-22 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: soham.ghosh, thuth, berrange, prerna.saxena, dgilbert, armbru,
Kshitij Suri, philippe.mathieu.daude, kraxel, prachatos.mitra,
eblake
Currently screendump only supports PPM format, which is un-compressed and not
standard. Added a "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 <kshitij.suri@nutanix.com>
---
diff to v1:
- Removed repeated alpha conversion operation.
- Modified logic to mirror png conversion in vnc-enc-tight.c file.
- Added a new CONFIG_PNG parameter for libpng support.
- Changed input format to enum instead of string.
- Improved error handling.
hmp-commands.hx | 11 ++---
monitor/hmp-cmds.c | 12 +++++-
qapi/ui.json | 24 +++++++++--
ui/console.c | 101 +++++++++++++++++++++++++++++++++++++++++++--
4 files changed, 136 insertions(+), 12 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8476277aa9..19b7cab595 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:s?,device:s?,head:i?",
+ .params = "filename [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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 634968498b..2442bfa989 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/qapi/ui.json b/qapi/ui.json
index 664da9e462..e8060d6b3c 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.0
+#
+##
+{ '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 is specified. (default: ppm) (Since 7.0)
+#
# 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 }
##
diff --git a/ui/console.c b/ui/console.c
index da434ce1b2..f42f64d556 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);
+ }
}
}
--
2.22.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
2022-03-22 10:49 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
@ 2022-03-28 9:50 ` Kshitij Suri
2022-03-28 9:52 ` Daniel P. Berrangé
1 sibling, 0 replies; 23+ messages in thread
From: Kshitij Suri @ 2022-03-28 9:50 UTC (permalink / raw)
To: qemu-devel
Cc: soham.ghosh, thuth, berrange, prerna.saxena, dgilbert, armbru,
philippe.mathieu.daude, kraxel, prachatos.mitra, eblake
Hi, Hope this mail finds you well. I have updated the code as required
and would be grateful if you could review and suggest changes that are
needed to be implemented. In case no change is required, please do let
me know the next steps for the same.
Regards,
Kshitij Suri
On 22/03/22 4:19 pm, Kshitij Suri wrote:
> Currently screendump only supports PPM format, which is un-compressed and not
> standard. Added a "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 <kshitij.suri@nutanix.com>
> ---
> diff to v1:
> - Removed repeated alpha conversion operation.
> - Modified logic to mirror png conversion in vnc-enc-tight.c file.
> - Added a new CONFIG_PNG parameter for libpng support.
> - Changed input format to enum instead of string.
> - Improved error handling.
> hmp-commands.hx | 11 ++---
> monitor/hmp-cmds.c | 12 +++++-
> qapi/ui.json | 24 +++++++++--
> ui/console.c | 101 +++++++++++++++++++++++++++++++++++++++++++--
> 4 files changed, 136 insertions(+), 12 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 8476277aa9..19b7cab595 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:s?,device:s?,head:i?",
> + .params = "filename [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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 634968498b..2442bfa989 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/qapi/ui.json b/qapi/ui.json
> index 664da9e462..e8060d6b3c 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.0
> +#
> +##
> +{ '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 is specified. (default: ppm) (Since 7.0)
> +#
> # 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 }
>
> ##
> diff --git a/ui/console.c b/ui/console.c
> index da434ce1b2..f42f64d556 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);
> + }
> }
> }
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
2022-03-22 10:49 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
2022-03-28 9:50 ` Kshitij Suri
@ 2022-03-28 9:52 ` Daniel P. Berrangé
2022-03-28 9:56 ` Kshitij Suri
1 sibling, 1 reply; 23+ messages in thread
From: Daniel P. Berrangé @ 2022-03-28 9:52 UTC (permalink / raw)
To: Kshitij Suri
Cc: soham.ghosh, thuth, prerna.saxena, armbru, qemu-devel,
philippe.mathieu.daude, kraxel, prachatos.mitra, eblake,
dgilbert
On Tue, Mar 22, 2022 at 10:49:53AM +0000, Kshitij Suri wrote:
> Currently screendump only supports PPM format, which is un-compressed and not
> standard. Added a "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 <kshitij.suri@nutanix.com>
> ---
> diff to v1:
> - Removed repeated alpha conversion operation.
> - Modified logic to mirror png conversion in vnc-enc-tight.c file.
> - Added a new CONFIG_PNG parameter for libpng support.
> - Changed input format to enum instead of string.
> - Improved error handling.
> hmp-commands.hx | 11 ++---
> monitor/hmp-cmds.c | 12 +++++-
> qapi/ui.json | 24 +++++++++--
> ui/console.c | 101 +++++++++++++++++++++++++++++++++++++++++++--
> 4 files changed, 136 insertions(+), 12 deletions(-)
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 664da9e462..e8060d6b3c 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.0
This will probably end up being 7.1 at this point.
> +#
> +##
> +{ '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 is specified. (default: ppm) (Since 7.0)
Likewise probably 7.1
None the less,
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
2022-03-28 9:52 ` Daniel P. Berrangé
@ 2022-03-28 9:56 ` Kshitij Suri
0 siblings, 0 replies; 23+ messages in thread
From: Kshitij Suri @ 2022-03-28 9:56 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: soham.ghosh, thuth, prerna.saxena, armbru, qemu-devel,
philippe.mathieu.daude, kraxel, prachatos.mitra, eblake,
dgilbert
On 28/03/22 3:22 pm, Daniel P. Berrangé wrote:
> On Tue, Mar 22, 2022 at 10:49:53AM +0000, Kshitij Suri wrote:
>> Currently screendump only supports PPM format, which is un-compressed and not
>> standard. Added a "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=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w&m=SfkDKyBHSRlW9cvs1qhKamw8p2-qgFH_XQcMVSwM5IPIjGQm0Emsj496xu-fpwaw&s=4EZwj8JQje-qxGhg2-Vv81iA-atUQTCNNBnlk1AW7do&e=
>>
>> Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
>> ---
>> diff to v1:
>> - Removed repeated alpha conversion operation.
>> - Modified logic to mirror png conversion in vnc-enc-tight.c file.
>> - Added a new CONFIG_PNG parameter for libpng support.
>> - Changed input format to enum instead of string.
>> - Improved error handling.
>> hmp-commands.hx | 11 ++---
>> monitor/hmp-cmds.c | 12 +++++-
>> qapi/ui.json | 24 +++++++++--
>> ui/console.c | 101 +++++++++++++++++++++++++++++++++++++++++++--
>> 4 files changed, 136 insertions(+), 12 deletions(-)
>
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index 664da9e462..e8060d6b3c 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.0
> This will probably end up being 7.1 at this point.
Oh okay. Will update the version
>> +#
>> +##
>> +{ '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 is specified. (default: ppm) (Since 7.0)
> Likewise probably 7.1
>
>
> None the less,
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>
>
> With regards,
> Daniel
Thank you for your review! Will update the version and send it in the
following patch
Regards,
Kshitij Suri
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
2022-03-22 10:49 [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG Kshitij Suri
2022-03-22 10:49 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
@ 2022-03-28 9:49 ` Kshitij Suri
1 sibling, 0 replies; 23+ messages in thread
From: Kshitij Suri @ 2022-03-28 9:49 UTC (permalink / raw)
To: qemu-devel
Cc: soham.ghosh, thuth, berrange, prerna.saxena, dgilbert, armbru,
philippe.mathieu.daude, kraxel, prachatos.mitra, eblake
Hi, Hope this mail finds you well. I have updated the code as required
and would be grateful if you could review and suggest changes that are
needed to be implemented. In case no change is required, please do let
me know the next steps for the same.
Regards,
Kshitij Suri
On 22/03/22 4:19 pm, Kshitij Suri wrote:
> Libpng is only detected if VNC is enabled currently. This patch adds a
> generalised png option in the meson build which is aimed to replace use of
> CONFIG_VNC_PNG with CONFIG_PNG.
>
> Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> meson.build | 9 ++++-----
> meson_options.txt | 4 ++--
> ui/vnc-enc-tight.c | 18 +++++++++---------
> ui/vnc.c | 4 ++--
> ui/vnc.h | 2 +-
> 5 files changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 282e7c4650..ccb6840a49 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1115,14 +1115,13 @@ if gtkx11.found()
> x11 = dependency('x11', method: 'pkg-config', required: gtkx11.found(),
> kwargs: static_kwargs)
> endif
> +png = dependency('libpng', required: get_option('png'),
> + method: 'pkg-config', kwargs: static_kwargs)
> vnc = not_found
> -png = not_found
> jpeg = not_found
> sasl = not_found
> if get_option('vnc').allowed() and have_system
> vnc = declare_dependency() # dummy dependency
> - png = dependency('libpng', required: get_option('vnc_png'),
> - method: 'pkg-config', kwargs: static_kwargs)
> jpeg = dependency('libjpeg', required: get_option('vnc_jpeg'),
> method: 'pkg-config', kwargs: static_kwargs)
> sasl = cc.find_library('sasl2', has_headers: ['sasl/sasl.h'],
> @@ -1554,9 +1553,9 @@ config_host_data.set('CONFIG_TPM', have_tpm)
> config_host_data.set('CONFIG_USB_LIBUSB', libusb.found())
> config_host_data.set('CONFIG_VDE', vde.found())
> config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', have_vhost_user_blk_server)
> +config_host_data.set('CONFIG_PNG', png.found())
> config_host_data.set('CONFIG_VNC', vnc.found())
> config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
> -config_host_data.set('CONFIG_VNC_PNG', png.found())
> config_host_data.set('CONFIG_VNC_SASL', sasl.found())
> config_host_data.set('CONFIG_VIRTFS', have_virtfs)
> config_host_data.set('CONFIG_VTE', vte.found())
> @@ -3638,11 +3637,11 @@ summary_info += {'curses support': curses}
> summary_info += {'virgl support': virgl}
> summary_info += {'curl support': curl}
> summary_info += {'Multipath support': mpathpersist}
> +summary_info += {'PNG support': png}
> summary_info += {'VNC support': vnc}
> if vnc.found()
> summary_info += {'VNC SASL support': sasl}
> summary_info += {'VNC JPEG support': jpeg}
> - summary_info += {'VNC PNG support': png}
> endif
> if targetos not in ['darwin', 'haiku', 'windows']
> summary_info += {'OSS support': oss}
> diff --git a/meson_options.txt b/meson_options.txt
> index 52b11cead4..d85734f8e6 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -177,12 +177,12 @@ option('vde', type : 'feature', value : 'auto',
> description: 'vde network backend support')
> option('virglrenderer', type : 'feature', value : 'auto',
> description: 'virgl rendering support')
> +option('png', type : 'feature', value : 'auto',
> + description: 'PNG support with libpng')
> option('vnc', type : 'feature', value : 'auto',
> description: 'VNC server')
> option('vnc_jpeg', type : 'feature', value : 'auto',
> description: 'JPEG lossy compression for VNC server')
> -option('vnc_png', type : 'feature', value : 'auto',
> - description: 'PNG compression for VNC server')
> option('vnc_sasl', type : 'feature', value : 'auto',
> description: 'SASL authentication for VNC server')
> option('vte', type : 'feature', value : 'auto',
> diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
> index 7b86a4713d..e879cca7f5 100644
> --- a/ui/vnc-enc-tight.c
> +++ b/ui/vnc-enc-tight.c
> @@ -32,7 +32,7 @@
> INT32 definitions between jmorecfg.h (included by jpeglib.h) and
> Win32 basetsd.h (included by windows.h). */
>
> -#ifdef CONFIG_VNC_PNG
> +#ifdef CONFIG_PNG
> /* The following define is needed by pngconf.h. Otherwise it won't compile,
> because setjmp.h was already included by qemu-common.h. */
> #define PNG_SKIP_SETJMP_CHECK
> @@ -95,7 +95,7 @@ static const struct {
> };
> #endif
>
> -#ifdef CONFIG_VNC_PNG
> +#ifdef CONFIG_PNG
> static const struct {
> int png_zlib_level, png_filters;
> } tight_png_conf[] = {
> @@ -919,7 +919,7 @@ static int send_full_color_rect(VncState *vs, int x, int y, int w, int h)
> int stream = 0;
> ssize_t bytes;
>
> -#ifdef CONFIG_VNC_PNG
> +#ifdef CONFIG_PNG
> if (tight_can_send_png_rect(vs, w, h)) {
> return send_png_rect(vs, x, y, w, h, NULL);
> }
> @@ -966,7 +966,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
> int stream = 1;
> int level = tight_conf[vs->tight->compression].mono_zlib_level;
>
> -#ifdef CONFIG_VNC_PNG
> +#ifdef CONFIG_PNG
> if (tight_can_send_png_rect(vs, w, h)) {
> int ret;
> int bpp = vs->client_pf.bytes_per_pixel * 8;
> @@ -1020,7 +1020,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
> struct palette_cb_priv {
> VncState *vs;
> uint8_t *header;
> -#ifdef CONFIG_VNC_PNG
> +#ifdef CONFIG_PNG
> png_colorp png_palette;
> #endif
> };
> @@ -1082,7 +1082,7 @@ static int send_palette_rect(VncState *vs, int x, int y,
> int colors;
> ssize_t bytes;
>
> -#ifdef CONFIG_VNC_PNG
> +#ifdef CONFIG_PNG
> if (tight_can_send_png_rect(vs, w, h)) {
> return send_png_rect(vs, x, y, w, h, palette);
> }
> @@ -1233,7 +1233,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality)
> /*
> * PNG compression stuff.
> */
> -#ifdef CONFIG_VNC_PNG
> +#ifdef CONFIG_PNG
> static void write_png_palette(int idx, uint32_t pix, void *opaque)
> {
> struct palette_cb_priv *priv = opaque;
> @@ -1379,7 +1379,7 @@ static int send_png_rect(VncState *vs, int x, int y, int w, int h,
> buffer_reset(&vs->tight->png);
> return 1;
> }
> -#endif /* CONFIG_VNC_PNG */
> +#endif /* CONFIG_PNG */
>
> static void vnc_tight_start(VncState *vs)
> {
> @@ -1706,7 +1706,7 @@ void vnc_tight_clear(VncState *vs)
> #ifdef CONFIG_VNC_JPEG
> buffer_free(&vs->tight->jpeg);
> #endif
> -#ifdef CONFIG_VNC_PNG
> +#ifdef CONFIG_PNG
> buffer_free(&vs->tight->png);
> #endif
> }
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 310a873c21..8376291b47 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -2165,7 +2165,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
> vs->features |= VNC_FEATURE_TIGHT_MASK;
> vs->vnc_encoding = enc;
> break;
> -#ifdef CONFIG_VNC_PNG
> +#ifdef CONFIG_PNG
> case VNC_ENCODING_TIGHT_PNG:
> vs->features |= VNC_FEATURE_TIGHT_PNG_MASK;
> vs->vnc_encoding = enc;
> @@ -3256,7 +3256,7 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
> #ifdef CONFIG_VNC_JPEG
> buffer_init(&vs->tight->jpeg, "vnc-tight-jpeg/%p", sioc);
> #endif
> -#ifdef CONFIG_VNC_PNG
> +#ifdef CONFIG_PNG
> buffer_init(&vs->tight->png, "vnc-tight-png/%p", sioc);
> #endif
> buffer_init(&vs->zlib.zlib, "vnc-zlib/%p", sioc);
> diff --git a/ui/vnc.h b/ui/vnc.h
> index a7149831f9..a60fb13115 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -201,7 +201,7 @@ typedef struct VncTight {
> #ifdef CONFIG_VNC_JPEG
> Buffer jpeg;
> #endif
> -#ifdef CONFIG_VNC_PNG
> +#ifdef CONFIG_PNG
> Buffer png;
> #endif
> int levels[4];
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
@ 2022-03-28 16:54 Kshitij Suri
2022-03-28 16:54 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
0 siblings, 1 reply; 23+ messages in thread
From: Kshitij Suri @ 2022-03-28 16:54 UTC (permalink / raw)
To: qemu-devel
Cc: soham.ghosh, thuth, berrange, prerna.saxena, dgilbert, armbru,
Kshitij Suri, philippe.mathieu.daude, kraxel, prachatos.mitra,
eblake
Libpng is only detected if VNC is enabled currently. This patch adds a
generalised png option in the meson build which is aimed to replace use of
CONFIG_VNC_PNG with CONFIG_PNG.
Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
meson.build | 9 ++++-----
meson_options.txt | 4 ++--
ui/vnc-enc-tight.c | 18 +++++++++---------
ui/vnc.c | 4 ++--
ui/vnc.h | 2 +-
5 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/meson.build b/meson.build
index 282e7c4650..ccb6840a49 100644
--- a/meson.build
+++ b/meson.build
@@ -1115,14 +1115,13 @@ if gtkx11.found()
x11 = dependency('x11', method: 'pkg-config', required: gtkx11.found(),
kwargs: static_kwargs)
endif
+png = dependency('libpng', required: get_option('png'),
+ method: 'pkg-config', kwargs: static_kwargs)
vnc = not_found
-png = not_found
jpeg = not_found
sasl = not_found
if get_option('vnc').allowed() and have_system
vnc = declare_dependency() # dummy dependency
- png = dependency('libpng', required: get_option('vnc_png'),
- method: 'pkg-config', kwargs: static_kwargs)
jpeg = dependency('libjpeg', required: get_option('vnc_jpeg'),
method: 'pkg-config', kwargs: static_kwargs)
sasl = cc.find_library('sasl2', has_headers: ['sasl/sasl.h'],
@@ -1554,9 +1553,9 @@ config_host_data.set('CONFIG_TPM', have_tpm)
config_host_data.set('CONFIG_USB_LIBUSB', libusb.found())
config_host_data.set('CONFIG_VDE', vde.found())
config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', have_vhost_user_blk_server)
+config_host_data.set('CONFIG_PNG', png.found())
config_host_data.set('CONFIG_VNC', vnc.found())
config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
-config_host_data.set('CONFIG_VNC_PNG', png.found())
config_host_data.set('CONFIG_VNC_SASL', sasl.found())
config_host_data.set('CONFIG_VIRTFS', have_virtfs)
config_host_data.set('CONFIG_VTE', vte.found())
@@ -3638,11 +3637,11 @@ summary_info += {'curses support': curses}
summary_info += {'virgl support': virgl}
summary_info += {'curl support': curl}
summary_info += {'Multipath support': mpathpersist}
+summary_info += {'PNG support': png}
summary_info += {'VNC support': vnc}
if vnc.found()
summary_info += {'VNC SASL support': sasl}
summary_info += {'VNC JPEG support': jpeg}
- summary_info += {'VNC PNG support': png}
endif
if targetos not in ['darwin', 'haiku', 'windows']
summary_info += {'OSS support': oss}
diff --git a/meson_options.txt b/meson_options.txt
index 52b11cead4..d85734f8e6 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -177,12 +177,12 @@ option('vde', type : 'feature', value : 'auto',
description: 'vde network backend support')
option('virglrenderer', type : 'feature', value : 'auto',
description: 'virgl rendering support')
+option('png', type : 'feature', value : 'auto',
+ description: 'PNG support with libpng')
option('vnc', type : 'feature', value : 'auto',
description: 'VNC server')
option('vnc_jpeg', type : 'feature', value : 'auto',
description: 'JPEG lossy compression for VNC server')
-option('vnc_png', type : 'feature', value : 'auto',
- description: 'PNG compression for VNC server')
option('vnc_sasl', type : 'feature', value : 'auto',
description: 'SASL authentication for VNC server')
option('vte', type : 'feature', value : 'auto',
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 7b86a4713d..e879cca7f5 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -32,7 +32,7 @@
INT32 definitions between jmorecfg.h (included by jpeglib.h) and
Win32 basetsd.h (included by windows.h). */
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
/* The following define is needed by pngconf.h. Otherwise it won't compile,
because setjmp.h was already included by qemu-common.h. */
#define PNG_SKIP_SETJMP_CHECK
@@ -95,7 +95,7 @@ static const struct {
};
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
static const struct {
int png_zlib_level, png_filters;
} tight_png_conf[] = {
@@ -919,7 +919,7 @@ static int send_full_color_rect(VncState *vs, int x, int y, int w, int h)
int stream = 0;
ssize_t bytes;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
return send_png_rect(vs, x, y, w, h, NULL);
}
@@ -966,7 +966,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
int stream = 1;
int level = tight_conf[vs->tight->compression].mono_zlib_level;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
int ret;
int bpp = vs->client_pf.bytes_per_pixel * 8;
@@ -1020,7 +1020,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
struct palette_cb_priv {
VncState *vs;
uint8_t *header;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
png_colorp png_palette;
#endif
};
@@ -1082,7 +1082,7 @@ static int send_palette_rect(VncState *vs, int x, int y,
int colors;
ssize_t bytes;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
return send_png_rect(vs, x, y, w, h, palette);
}
@@ -1233,7 +1233,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality)
/*
* PNG compression stuff.
*/
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
static void write_png_palette(int idx, uint32_t pix, void *opaque)
{
struct palette_cb_priv *priv = opaque;
@@ -1379,7 +1379,7 @@ static int send_png_rect(VncState *vs, int x, int y, int w, int h,
buffer_reset(&vs->tight->png);
return 1;
}
-#endif /* CONFIG_VNC_PNG */
+#endif /* CONFIG_PNG */
static void vnc_tight_start(VncState *vs)
{
@@ -1706,7 +1706,7 @@ void vnc_tight_clear(VncState *vs)
#ifdef CONFIG_VNC_JPEG
buffer_free(&vs->tight->jpeg);
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
buffer_free(&vs->tight->png);
#endif
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 310a873c21..8376291b47 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2165,7 +2165,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
vs->features |= VNC_FEATURE_TIGHT_MASK;
vs->vnc_encoding = enc;
break;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
case VNC_ENCODING_TIGHT_PNG:
vs->features |= VNC_FEATURE_TIGHT_PNG_MASK;
vs->vnc_encoding = enc;
@@ -3256,7 +3256,7 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
#ifdef CONFIG_VNC_JPEG
buffer_init(&vs->tight->jpeg, "vnc-tight-jpeg/%p", sioc);
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
buffer_init(&vs->tight->png, "vnc-tight-png/%p", sioc);
#endif
buffer_init(&vs->zlib.zlib, "vnc-zlib/%p", sioc);
diff --git a/ui/vnc.h b/ui/vnc.h
index a7149831f9..a60fb13115 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -201,7 +201,7 @@ typedef struct VncTight {
#ifdef CONFIG_VNC_JPEG
Buffer jpeg;
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
Buffer png;
#endif
int levels[4];
--
2.22.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
2022-03-28 16:54 Kshitij Suri
@ 2022-03-28 16:54 ` Kshitij Suri
2022-03-29 6:42 ` Markus Armbruster
0 siblings, 1 reply; 23+ messages in thread
From: Kshitij Suri @ 2022-03-28 16:54 UTC (permalink / raw)
To: qemu-devel
Cc: soham.ghosh, thuth, berrange, prerna.saxena, dgilbert, armbru,
Kshitij Suri, philippe.mathieu.daude, kraxel, prachatos.mitra,
eblake
Currently screendump only supports PPM format, which is un-compressed and not
standard. Added a "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 <kshitij.suri@nutanix.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
hmp-commands.hx | 11 ++---
monitor/hmp-cmds.c | 12 +++++-
qapi/ui.json | 24 +++++++++--
ui/console.c | 101 +++++++++++++++++++++++++++++++++++++++++++--
4 files changed, 136 insertions(+), 12 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8476277aa9..19b7cab595 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:s?,device:s?,head:i?",
+ .params = "filename [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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 634968498b..2442bfa989 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/qapi/ui.json b/qapi/ui.json
index 664da9e462..24371fce05 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 is specified. (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 }
##
diff --git a/ui/console.c b/ui/console.c
index da434ce1b2..f42f64d556 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);
+ }
}
}
--
2.22.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
2022-03-28 16:54 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
@ 2022-03-29 6:42 ` Markus Armbruster
2022-03-29 7:06 ` Kshitij Suri
0 siblings, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2022-03-29 6:42 UTC (permalink / raw)
To: Kshitij Suri
Cc: soham.ghosh, thuth, berrange, prerna.saxena, qemu-devel,
dgilbert, philippe.mathieu.daude, kraxel, prachatos.mitra,
eblake
If I count correctly, this is the fifth posting tagged "v2". Don't do
that, please, as it's quite confusing.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
2022-03-29 6:42 ` Markus Armbruster
@ 2022-03-29 7:06 ` Kshitij Suri
0 siblings, 0 replies; 23+ messages in thread
From: Kshitij Suri @ 2022-03-29 7:06 UTC (permalink / raw)
To: Markus Armbruster
Cc: soham.ghosh, thuth, berrange, prerna.saxena, qemu-devel,
dgilbert, philippe.mathieu.daude, kraxel, prachatos.mitra,
eblake
On 29/03/22 12:12 pm, Markus Armbruster wrote:
> If I count correctly, this is the fifth posting tagged "v2". Don't do
> that, please, as it's quite confusing.
>
Thank you for your review and I apologise for that since I am fairly new
to upstreaming. As per what I read version updates should be done only
when there are significant design changes to the patch which didn't
happen in the v2 version. Will update it to v3 and send the patch.
Regards,
Kshitij Suri
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
@ 2022-03-22 8:18 Kshitij Suri
2022-03-22 8:18 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
0 siblings, 1 reply; 23+ messages in thread
From: Kshitij Suri @ 2022-03-22 8:18 UTC (permalink / raw)
To: qemu-devel
Cc: soham.ghosh, thuth, berrange, prerna.saxena, dgilbert, armbru,
Kshitij Suri, philippe.mathieu.daude, kraxel, prachatos.mitra,
eblake
Libpng is only detected if VNC is enabled currently. This patch adds a
generalised png option in the meson build which is aimed to replace use of
CONFIG_VNC_PNG with CONFIG_PNG.
Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
---
meson.build | 9 ++++-----
meson_options.txt | 4 ++--
ui/vnc-enc-tight.c | 18 +++++++++---------
ui/vnc.c | 4 ++--
ui/vnc.h | 2 +-
5 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/meson.build b/meson.build
index 282e7c4650..ccb6840a49 100644
--- a/meson.build
+++ b/meson.build
@@ -1115,14 +1115,13 @@ if gtkx11.found()
x11 = dependency('x11', method: 'pkg-config', required: gtkx11.found(),
kwargs: static_kwargs)
endif
+png = dependency('libpng', required: get_option('png'),
+ method: 'pkg-config', kwargs: static_kwargs)
vnc = not_found
-png = not_found
jpeg = not_found
sasl = not_found
if get_option('vnc').allowed() and have_system
vnc = declare_dependency() # dummy dependency
- png = dependency('libpng', required: get_option('vnc_png'),
- method: 'pkg-config', kwargs: static_kwargs)
jpeg = dependency('libjpeg', required: get_option('vnc_jpeg'),
method: 'pkg-config', kwargs: static_kwargs)
sasl = cc.find_library('sasl2', has_headers: ['sasl/sasl.h'],
@@ -1554,9 +1553,9 @@ config_host_data.set('CONFIG_TPM', have_tpm)
config_host_data.set('CONFIG_USB_LIBUSB', libusb.found())
config_host_data.set('CONFIG_VDE', vde.found())
config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', have_vhost_user_blk_server)
+config_host_data.set('CONFIG_PNG', png.found())
config_host_data.set('CONFIG_VNC', vnc.found())
config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
-config_host_data.set('CONFIG_VNC_PNG', png.found())
config_host_data.set('CONFIG_VNC_SASL', sasl.found())
config_host_data.set('CONFIG_VIRTFS', have_virtfs)
config_host_data.set('CONFIG_VTE', vte.found())
@@ -3638,11 +3637,11 @@ summary_info += {'curses support': curses}
summary_info += {'virgl support': virgl}
summary_info += {'curl support': curl}
summary_info += {'Multipath support': mpathpersist}
+summary_info += {'PNG support': png}
summary_info += {'VNC support': vnc}
if vnc.found()
summary_info += {'VNC SASL support': sasl}
summary_info += {'VNC JPEG support': jpeg}
- summary_info += {'VNC PNG support': png}
endif
if targetos not in ['darwin', 'haiku', 'windows']
summary_info += {'OSS support': oss}
diff --git a/meson_options.txt b/meson_options.txt
index 52b11cead4..d85734f8e6 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -177,12 +177,12 @@ option('vde', type : 'feature', value : 'auto',
description: 'vde network backend support')
option('virglrenderer', type : 'feature', value : 'auto',
description: 'virgl rendering support')
+option('png', type : 'feature', value : 'auto',
+ description: 'PNG support with libpng')
option('vnc', type : 'feature', value : 'auto',
description: 'VNC server')
option('vnc_jpeg', type : 'feature', value : 'auto',
description: 'JPEG lossy compression for VNC server')
-option('vnc_png', type : 'feature', value : 'auto',
- description: 'PNG compression for VNC server')
option('vnc_sasl', type : 'feature', value : 'auto',
description: 'SASL authentication for VNC server')
option('vte', type : 'feature', value : 'auto',
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 7b86a4713d..e879cca7f5 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -32,7 +32,7 @@
INT32 definitions between jmorecfg.h (included by jpeglib.h) and
Win32 basetsd.h (included by windows.h). */
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
/* The following define is needed by pngconf.h. Otherwise it won't compile,
because setjmp.h was already included by qemu-common.h. */
#define PNG_SKIP_SETJMP_CHECK
@@ -95,7 +95,7 @@ static const struct {
};
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
static const struct {
int png_zlib_level, png_filters;
} tight_png_conf[] = {
@@ -919,7 +919,7 @@ static int send_full_color_rect(VncState *vs, int x, int y, int w, int h)
int stream = 0;
ssize_t bytes;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
return send_png_rect(vs, x, y, w, h, NULL);
}
@@ -966,7 +966,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
int stream = 1;
int level = tight_conf[vs->tight->compression].mono_zlib_level;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
int ret;
int bpp = vs->client_pf.bytes_per_pixel * 8;
@@ -1020,7 +1020,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
struct palette_cb_priv {
VncState *vs;
uint8_t *header;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
png_colorp png_palette;
#endif
};
@@ -1082,7 +1082,7 @@ static int send_palette_rect(VncState *vs, int x, int y,
int colors;
ssize_t bytes;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
return send_png_rect(vs, x, y, w, h, palette);
}
@@ -1233,7 +1233,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality)
/*
* PNG compression stuff.
*/
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
static void write_png_palette(int idx, uint32_t pix, void *opaque)
{
struct palette_cb_priv *priv = opaque;
@@ -1379,7 +1379,7 @@ static int send_png_rect(VncState *vs, int x, int y, int w, int h,
buffer_reset(&vs->tight->png);
return 1;
}
-#endif /* CONFIG_VNC_PNG */
+#endif /* CONFIG_PNG */
static void vnc_tight_start(VncState *vs)
{
@@ -1706,7 +1706,7 @@ void vnc_tight_clear(VncState *vs)
#ifdef CONFIG_VNC_JPEG
buffer_free(&vs->tight->jpeg);
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
buffer_free(&vs->tight->png);
#endif
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 310a873c21..8376291b47 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2165,7 +2165,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
vs->features |= VNC_FEATURE_TIGHT_MASK;
vs->vnc_encoding = enc;
break;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
case VNC_ENCODING_TIGHT_PNG:
vs->features |= VNC_FEATURE_TIGHT_PNG_MASK;
vs->vnc_encoding = enc;
@@ -3256,7 +3256,7 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
#ifdef CONFIG_VNC_JPEG
buffer_init(&vs->tight->jpeg, "vnc-tight-jpeg/%p", sioc);
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
buffer_init(&vs->tight->png, "vnc-tight-png/%p", sioc);
#endif
buffer_init(&vs->zlib.zlib, "vnc-zlib/%p", sioc);
diff --git a/ui/vnc.h b/ui/vnc.h
index a7149831f9..a60fb13115 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -201,7 +201,7 @@ typedef struct VncTight {
#ifdef CONFIG_VNC_JPEG
Buffer jpeg;
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
Buffer png;
#endif
int levels[4];
--
2.22.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
2022-03-22 8:18 [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG Kshitij Suri
@ 2022-03-22 8:18 ` Kshitij Suri
2022-03-22 9:47 ` Daniel P. Berrangé
0 siblings, 1 reply; 23+ messages in thread
From: Kshitij Suri @ 2022-03-22 8:18 UTC (permalink / raw)
To: qemu-devel
Cc: soham.ghosh, thuth, berrange, prerna.saxena, dgilbert, armbru,
Kshitij Suri, philippe.mathieu.daude, kraxel, prachatos.mitra,
eblake
Currently screendump only supports PPM format, which is un-compressed and not
standard. Added a "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 <kshitij.suri@nutanix.com>
---
diff to v1:
- Removed repeated alpha conversion operation.
- Modified logic to mirror png conversion in vnc-enc-tight.c file.
- Added a new CONFIG_PNG parameter for libpng support.
- Changed input format to enum instead of string.
- Improved error handling.
hmp-commands.hx | 11 ++---
monitor/hmp-cmds.c | 20 ++++++++-
qapi/ui.json | 24 +++++++++--
ui/console.c | 101 +++++++++++++++++++++++++++++++++++++++++++--
4 files changed, 144 insertions(+), 12 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8476277aa9..19b7cab595 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:s?,device:s?,head:i?",
+ .params = "filename [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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 634968498b..bf3ba76bd3 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1720,9 +1720,27 @@ 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);
+ int val = qapi_enum_parse(&ImageFormat_lookup, input_format,
+ IMAGE_FORMAT_PPM, &err);
+ if (err) {
+ goto end;
+ }
+
+ switch (val) {
+ case IMAGE_FORMAT_PNG:
+ format = IMAGE_FORMAT_PNG;
+ break;
+ default:
+ format = IMAGE_FORMAT_PPM;
+ }
+
+ qmp_screendump(filename, id != NULL, id, id != NULL, head,
+ input_format != NULL, format, &err);
+end:
hmp_handle_error(mon, err);
}
diff --git a/qapi/ui.json b/qapi/ui.json
index 664da9e462..e8060d6b3c 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.0
+#
+##
+{ '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 is specified. (default: ppm) (Since 7.0)
+#
# 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 }
##
diff --git a/ui/console.c b/ui/console.c
index da434ce1b2..8814d50bcd 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);
+ }
}
}
--
2.22.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
2022-03-22 8:18 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
@ 2022-03-22 9:47 ` Daniel P. Berrangé
2022-03-22 9:56 ` Kshitij Suri
2022-03-22 10:15 ` Markus Armbruster
0 siblings, 2 replies; 23+ messages in thread
From: Daniel P. Berrangé @ 2022-03-22 9:47 UTC (permalink / raw)
To: Kshitij Suri
Cc: soham.ghosh, thuth, prerna.saxena, armbru, qemu-devel,
philippe.mathieu.daude, kraxel, prachatos.mitra, eblake,
dgilbert
On Tue, Mar 22, 2022 at 08:18:45AM +0000, Kshitij Suri wrote:
> Currently screendump only supports PPM format, which is un-compressed and not
> standard. Added a "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 <kshitij.suri@nutanix.com>
> ---
> diff to v1:
> - Removed repeated alpha conversion operation.
> - Modified logic to mirror png conversion in vnc-enc-tight.c file.
> - Added a new CONFIG_PNG parameter for libpng support.
> - Changed input format to enum instead of string.
> - Improved error handling.
> hmp-commands.hx | 11 ++---
> monitor/hmp-cmds.c | 20 ++++++++-
> qapi/ui.json | 24 +++++++++--
> ui/console.c | 101 +++++++++++++++++++++++++++++++++++++++++++--
> 4 files changed, 144 insertions(+), 12 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 8476277aa9..19b7cab595 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:s?,device:s?,head:i?",
> + .params = "filename [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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 634968498b..bf3ba76bd3 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1720,9 +1720,27 @@ 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);
> + int val = qapi_enum_parse(&ImageFormat_lookup, input_format,
> + IMAGE_FORMAT_PPM, &err);
> + if (err) {
> + goto end;
> + }
> +
> + switch (val) {
> + case IMAGE_FORMAT_PNG:
> + format = IMAGE_FORMAT_PNG;
> + break;
> + default:
> + format = IMAGE_FORMAT_PPM;
> + }
This switch looks pointless - the code is passing the default into
qapi_enum_parse already, this doesn't need to handle defaulting
again. This just needs
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);
> }
>
> + for (y = 0; y < height; ++y) {
> + qemu_pixman_linebuf_fill(linebuf, image, width, 0, y);
> + png_write_row(png_ptr, buf);
> + }
Tiny style bug, indent off-by-1
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
2022-03-22 9:47 ` Daniel P. Berrangé
@ 2022-03-22 9:56 ` Kshitij Suri
2022-03-22 10:15 ` Markus Armbruster
1 sibling, 0 replies; 23+ messages in thread
From: Kshitij Suri @ 2022-03-22 9:56 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: soham.ghosh, thuth, prerna.saxena, armbru, qemu-devel,
philippe.mathieu.daude, kraxel, prachatos.mitra, eblake,
dgilbert
On 22/03/22 3:17 pm, Daniel P. Berrangé wrote:
> On Tue, Mar 22, 2022 at 08:18:45AM +0000, Kshitij Suri wrote:
>> Currently screendump only supports PPM format, which is un-compressed and not
>> standard. Added a "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=n4gY8td5J_mIXLjj-t4OvqdRNDLAKuLn003VbLjWtqVaIGciyzeqQ8Fij45WWQ9m&s=gw9lEcMePStOP8Kcb4RSP_znNQSVAEUtBC63b1g1x5Q&e=
>>
>> Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
>> ---
>> diff to v1:
>> - Removed repeated alpha conversion operation.
>> - Modified logic to mirror png conversion in vnc-enc-tight.c file.
>> - Added a new CONFIG_PNG parameter for libpng support.
>> - Changed input format to enum instead of string.
>> - Improved error handling.
>> hmp-commands.hx | 11 ++---
>> monitor/hmp-cmds.c | 20 ++++++++-
>> qapi/ui.json | 24 +++++++++--
>> ui/console.c | 101 +++++++++++++++++++++++++++++++++++++++++++--
>> 4 files changed, 144 insertions(+), 12 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 8476277aa9..19b7cab595 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:s?,device:s?,head:i?",
>> + .params = "filename [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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
>> index 634968498b..bf3ba76bd3 100644
>> --- a/monitor/hmp-cmds.c
>> +++ b/monitor/hmp-cmds.c
>> @@ -1720,9 +1720,27 @@ 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);
>> + int val = qapi_enum_parse(&ImageFormat_lookup, input_format,
>> + IMAGE_FORMAT_PPM, &err);
>> + if (err) {
>> + goto end;
>> + }
>> +
>> + switch (val) {
>> + case IMAGE_FORMAT_PNG:
>> + format = IMAGE_FORMAT_PNG;
>> + break;
>> + default:
>> + format = IMAGE_FORMAT_PPM;
>> + }
> This switch looks pointless - the code is passing the default into
> qapi_enum_parse already, this doesn't need to handle defaulting
> again. This just needs
>
> format = qapi_enum_parse(&ImageFormat_lookup, input_format,
> IMAGE_FORMAT_PPM, &err);
> if (err) {
> goto end;
> }
Apologies, missed that. Will update in the following patch.
>> +
>> + qmp_screendump(filename, id != NULL, id, id != NULL, head,
>> + input_format != NULL, format, &err);
>> +end:
>> hmp_handle_error(mon, err);
>> }
>>
>> + for (y = 0; y < height; ++y) {
>> + qemu_pixman_linebuf_fill(linebuf, image, width, 0, y);
>> + png_write_row(png_ptr, buf);
>> + }
> Tiny style bug, indent off-by-1
>
>
> With regards,
> Daniel
Thank you for your review!
Regards,
Kshitij Suri
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
2022-03-22 9:47 ` Daniel P. Berrangé
2022-03-22 9:56 ` Kshitij Suri
@ 2022-03-22 10:15 ` Markus Armbruster
2022-03-22 10:19 ` Kshitij Suri
1 sibling, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2022-03-22 10:15 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: soham.ghosh, thuth, prerna.saxena, qemu-devel, dgilbert,
Kshitij Suri, philippe.mathieu.daude, kraxel, prachatos.mitra,
eblake
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Tue, Mar 22, 2022 at 08:18:45AM +0000, Kshitij Suri wrote:
>> Currently screendump only supports PPM format, which is un-compressed and not
>> standard. Added a "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 <kshitij.suri@nutanix.com>
>> ---
>> diff to v1:
>> - Removed repeated alpha conversion operation.
>> - Modified logic to mirror png conversion in vnc-enc-tight.c file.
>> - Added a new CONFIG_PNG parameter for libpng support.
>> - Changed input format to enum instead of string.
>> - Improved error handling.
>> hmp-commands.hx | 11 ++---
>> monitor/hmp-cmds.c | 20 ++++++++-
>> qapi/ui.json | 24 +++++++++--
>> ui/console.c | 101 +++++++++++++++++++++++++++++++++++++++++++--
>> 4 files changed, 144 insertions(+), 12 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 8476277aa9..19b7cab595 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:s?,device:s?,head:i?",
>> + .params = "filename [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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
>> index 634968498b..bf3ba76bd3 100644
>> --- a/monitor/hmp-cmds.c
>> +++ b/monitor/hmp-cmds.c
>> @@ -1720,9 +1720,27 @@ 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);
>> + int val = qapi_enum_parse(&ImageFormat_lookup, input_format,
>> + IMAGE_FORMAT_PPM, &err);
>> + if (err) {
>> + goto end;
>> + }
>> +
>> + switch (val) {
>> + case IMAGE_FORMAT_PNG:
>> + format = IMAGE_FORMAT_PNG;
>> + break;
>> + default:
>> + format = IMAGE_FORMAT_PPM;
>> + }
>
> This switch looks pointless - the code is passing the default into
> qapi_enum_parse already, this doesn't need to handle defaulting
> again. This just needs
>
> format = qapi_enum_parse(&ImageFormat_lookup, input_format,
> IMAGE_FORMAT_PPM, &err);
> if (err) {
> goto end;
> }
Correct. See my review of v1 for a detailed explanation.
>
>> +
>> + qmp_screendump(filename, id != NULL, id, id != NULL, head,
>> + input_format != NULL, format, &err);
>> +end:
>> hmp_handle_error(mon, err);
>> }
>>
>
>> + for (y = 0; y < height; ++y) {
>> + qemu_pixman_linebuf_fill(linebuf, image, width, 0, y);
>> + png_write_row(png_ptr, buf);
>> + }
>
> Tiny style bug, indent off-by-1
>
>
> With regards,
> Daniel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
2022-03-22 10:15 ` Markus Armbruster
@ 2022-03-22 10:19 ` Kshitij Suri
0 siblings, 0 replies; 23+ messages in thread
From: Kshitij Suri @ 2022-03-22 10:19 UTC (permalink / raw)
To: Markus Armbruster, Daniel P. Berrangé
Cc: soham.ghosh, thuth, prerna.saxena, qemu-devel, dgilbert,
philippe.mathieu.daude, kraxel, prachatos.mitra, eblake
On 22/03/22 3:45 pm, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
>> On Tue, Mar 22, 2022 at 08:18:45AM +0000, Kshitij Suri wrote:
>>> Currently screendump only supports PPM format, which is un-compressed and not
>>> standard. Added a "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=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w&m=xFoMMCA3XaOs_syRsqPtgrgJy9F4pkHZI4GT5W-tgE7tns-XSTSdrcEjEu3Ft8gY&s=71h7PAt6kWooW2flunhGC7SDoQJeI46QCYxdIwnrA1Y&e=
>>>
>>> Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
>>> ---
>>> diff to v1:
>>> - Removed repeated alpha conversion operation.
>>> - Modified logic to mirror png conversion in vnc-enc-tight.c file.
>>> - Added a new CONFIG_PNG parameter for libpng support.
>>> - Changed input format to enum instead of string.
>>> - Improved error handling.
>>> hmp-commands.hx | 11 ++---
>>> monitor/hmp-cmds.c | 20 ++++++++-
>>> qapi/ui.json | 24 +++++++++--
>>> ui/console.c | 101 +++++++++++++++++++++++++++++++++++++++++++--
>>> 4 files changed, 144 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>> index 8476277aa9..19b7cab595 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:s?,device:s?,head:i?",
>>> + .params = "filename [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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
>>> index 634968498b..bf3ba76bd3 100644
>>> --- a/monitor/hmp-cmds.c
>>> +++ b/monitor/hmp-cmds.c
>>> @@ -1720,9 +1720,27 @@ 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);
>>> + int val = qapi_enum_parse(&ImageFormat_lookup, input_format,
>>> + IMAGE_FORMAT_PPM, &err);
>>> + if (err) {
>>> + goto end;
>>> + }
>>> +
>>> + switch (val) {
>>> + case IMAGE_FORMAT_PNG:
>>> + format = IMAGE_FORMAT_PNG;
>>> + break;
>>> + default:
>>> + format = IMAGE_FORMAT_PPM;
>>> + }
>> This switch looks pointless - the code is passing the default into
>> qapi_enum_parse already, this doesn't need to handle defaulting
>> again. This just needs
>>
>> format = qapi_enum_parse(&ImageFormat_lookup, input_format,
>> IMAGE_FORMAT_PPM, &err);
>> if (err) {
>> goto end;
>> }
> Correct. See my review of v1 for a detailed explanation.
Thank you! Will modify it in the upcoming patch
>
>>> +
>>> + qmp_screendump(filename, id != NULL, id, id != NULL, head,
>>> + input_format != NULL, format, &err);
>>> +end:
>>> hmp_handle_error(mon, err);
>>> }
>>>
>>> + for (y = 0; y < height; ++y) {
>>> + qemu_pixman_linebuf_fill(linebuf, image, width, 0, y);
>>> + png_write_row(png_ptr, buf);
>>> + }
>> Tiny style bug, indent off-by-1
>>
>>
>> With regards,
>> Daniel
Thank you for review!
Regards,
Kshitij Suri
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
@ 2022-03-01 6:44 Kshitij Suri
2022-03-01 6:44 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
0 siblings, 1 reply; 23+ messages in thread
From: Kshitij Suri @ 2022-03-01 6:44 UTC (permalink / raw)
To: qemu-devel
Cc: soham.ghosh, thuth, prerna.saxena, dgilbert, armbru,
Kshitij Suri, kraxel, prachatos.mitra, eblake
Libpng is only detected if VNC is enabled currently. This patch adds a
generalised png option in the meson build which is aimed to replace use of
CONFIG_VNC_PNG with CONFIG_PNG.
Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
---
meson.build | 9 ++++-----
meson_options.txt | 4 ++--
ui/vnc-enc-tight.c | 18 +++++++++---------
ui/vnc.c | 4 ++--
ui/vnc.h | 2 +-
5 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/meson.build b/meson.build
index 8df40bfac4..3b67f83a2c 100644
--- a/meson.build
+++ b/meson.build
@@ -1112,14 +1112,13 @@ if gtkx11.found()
x11 = dependency('x11', method: 'pkg-config', required: gtkx11.found(),
kwargs: static_kwargs)
endif
+png = dependency('libpng', required: get_option('png'),
+ method: 'pkg-config', kwargs: static_kwargs)
vnc = not_found
-png = not_found
jpeg = not_found
sasl = not_found
if get_option('vnc').allowed() and have_system
vnc = declare_dependency() # dummy dependency
- png = dependency('libpng', required: get_option('vnc_png'),
- method: 'pkg-config', kwargs: static_kwargs)
jpeg = dependency('libjpeg', required: get_option('vnc_jpeg'),
method: 'pkg-config', kwargs: static_kwargs)
sasl = cc.find_library('sasl2', has_headers: ['sasl/sasl.h'],
@@ -1537,9 +1536,9 @@ config_host_data.set('CONFIG_TPM', have_tpm)
config_host_data.set('CONFIG_USB_LIBUSB', libusb.found())
config_host_data.set('CONFIG_VDE', vde.found())
config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', have_vhost_user_blk_server)
+config_host_data.set('CONFIG_PNG', png.found())
config_host_data.set('CONFIG_VNC', vnc.found())
config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
-config_host_data.set('CONFIG_VNC_PNG', png.found())
config_host_data.set('CONFIG_VNC_SASL', sasl.found())
config_host_data.set('CONFIG_VIRTFS', have_virtfs)
config_host_data.set('CONFIG_VTE', vte.found())
@@ -3579,11 +3578,11 @@ summary_info += {'curses support': curses}
summary_info += {'virgl support': virgl}
summary_info += {'curl support': curl}
summary_info += {'Multipath support': mpathpersist}
+summary_info += {'PNG support': png}
summary_info += {'VNC support': vnc}
if vnc.found()
summary_info += {'VNC SASL support': sasl}
summary_info += {'VNC JPEG support': jpeg}
- summary_info += {'VNC PNG support': png}
endif
if targetos not in ['darwin', 'haiku', 'windows']
summary_info += {'OSS support': oss}
diff --git a/meson_options.txt b/meson_options.txt
index 52b11cead4..d85734f8e6 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -177,12 +177,12 @@ option('vde', type : 'feature', value : 'auto',
description: 'vde network backend support')
option('virglrenderer', type : 'feature', value : 'auto',
description: 'virgl rendering support')
+option('png', type : 'feature', value : 'auto',
+ description: 'PNG support with libpng')
option('vnc', type : 'feature', value : 'auto',
description: 'VNC server')
option('vnc_jpeg', type : 'feature', value : 'auto',
description: 'JPEG lossy compression for VNC server')
-option('vnc_png', type : 'feature', value : 'auto',
- description: 'PNG compression for VNC server')
option('vnc_sasl', type : 'feature', value : 'auto',
description: 'SASL authentication for VNC server')
option('vte', type : 'feature', value : 'auto',
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index cebd35841a..a23ad712eb 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -32,7 +32,7 @@
INT32 definitions between jmorecfg.h (included by jpeglib.h) and
Win32 basetsd.h (included by windows.h). */
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
/* The following define is needed by pngconf.h. Otherwise it won't compile,
because setjmp.h was already included by qemu-common.h. */
#define PNG_SKIP_SETJMP_CHECK
@@ -95,7 +95,7 @@ static const struct {
};
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
static const struct {
int png_zlib_level, png_filters;
} tight_png_conf[] = {
@@ -919,7 +919,7 @@ static int send_full_color_rect(VncState *vs, int x, int y, int w, int h)
int stream = 0;
ssize_t bytes;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
return send_png_rect(vs, x, y, w, h, NULL);
}
@@ -966,7 +966,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
int stream = 1;
int level = tight_conf[vs->tight->compression].mono_zlib_level;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
int ret;
int bpp = vs->client_pf.bytes_per_pixel * 8;
@@ -1020,7 +1020,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
struct palette_cb_priv {
VncState *vs;
uint8_t *header;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
png_colorp png_palette;
#endif
};
@@ -1082,7 +1082,7 @@ static int send_palette_rect(VncState *vs, int x, int y,
int colors;
ssize_t bytes;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
return send_png_rect(vs, x, y, w, h, palette);
}
@@ -1233,7 +1233,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality)
/*
* PNG compression stuff.
*/
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
static void write_png_palette(int idx, uint32_t pix, void *opaque)
{
struct palette_cb_priv *priv = opaque;
@@ -1379,7 +1379,7 @@ static int send_png_rect(VncState *vs, int x, int y, int w, int h,
buffer_reset(&vs->tight->png);
return 1;
}
-#endif /* CONFIG_VNC_PNG */
+#endif /* CONFIG_PNG */
static void vnc_tight_start(VncState *vs)
{
@@ -1706,7 +1706,7 @@ void vnc_tight_clear(VncState *vs)
#ifdef CONFIG_VNC_JPEG
buffer_free(&vs->tight->jpeg);
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
buffer_free(&vs->tight->png);
#endif
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 3ccd33dedc..a588ddff1c 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2165,7 +2165,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
vs->features |= VNC_FEATURE_TIGHT_MASK;
vs->vnc_encoding = enc;
break;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
case VNC_ENCODING_TIGHT_PNG:
vs->features |= VNC_FEATURE_TIGHT_PNG_MASK;
vs->vnc_encoding = enc;
@@ -3248,7 +3248,7 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
#ifdef CONFIG_VNC_JPEG
buffer_init(&vs->tight->jpeg, "vnc-tight-jpeg/%p", sioc);
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
buffer_init(&vs->tight->png, "vnc-tight-png/%p", sioc);
#endif
buffer_init(&vs->zlib.zlib, "vnc-zlib/%p", sioc);
diff --git a/ui/vnc.h b/ui/vnc.h
index a7149831f9..a60fb13115 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -201,7 +201,7 @@ typedef struct VncTight {
#ifdef CONFIG_VNC_JPEG
Buffer jpeg;
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
Buffer png;
#endif
int levels[4];
--
2.22.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG.
2022-03-01 6:44 [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG Kshitij Suri
@ 2022-03-01 6:44 ` Kshitij Suri
2022-03-07 16:41 ` Kshitij Suri
2022-03-11 12:20 ` Markus Armbruster
0 siblings, 2 replies; 23+ messages in thread
From: Kshitij Suri @ 2022-03-01 6:44 UTC (permalink / raw)
To: qemu-devel
Cc: soham.ghosh, thuth, prerna.saxena, dgilbert, armbru,
Kshitij Suri, kraxel, prachatos.mitra, eblake
Currently screendump only supports PPM format, which is un-compressed and not
standard. Added a "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 <kshitij.suri@nutanix.com>
---
diff to v1:
- Removed repeated alpha conversion operation.
- Modified logic to mirror png conversion in vnc-enc-tight.c file.
- Added a new CONFIG_PNG parameter for libpng support.
- Changed input format to enum instead of string.
- Improved error handling.
hmp-commands.hx | 11 ++---
monitor/hmp-cmds.c | 19 ++++++++-
qapi/ui.json | 25 +++++++++--
ui/console.c | 102 +++++++++++++++++++++++++++++++++++++++++++--
4 files changed, 145 insertions(+), 12 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 70a9136ac2..5eda4eeb24 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'."
+ "Currently only 'png' and 'ppm' formats are supported.",
.cmd = hmp_screendump,
.coroutine = true,
},
SRST
``screendump`` *filename*
- Save screen into PPM image *filename*.
+ Save screen as an image *filename*, with default format of PPM.
ERST
{
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 8c384dc1b2..9a640146eb 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1677,9 +1677,26 @@ 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_str(qdict, "format");
Error *err = NULL;
+ ImageFormat format;
- qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
+ int val = qapi_enum_parse(&ImageFormat_lookup, input_format, -1, &err);
+ if (val < 0) {
+ goto end;
+ }
+
+ switch (val) {
+ case IMAGE_FORMAT_PNG:
+ format = IMAGE_FORMAT_PNG;
+ break;
+ default:
+ format = IMAGE_FORMAT_PPM;
+ }
+
+ qmp_screendump(filename, id != NULL, id, id != NULL, head,
+ input_format != NULL, format, &err);
+end:
hmp_handle_error(mon, err);
}
diff --git a/qapi/ui.json b/qapi/ui.json
index 9354f4c467..6aa0dd7c1b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -73,12 +73,27 @@
##
{ 'command': 'expire_password', 'data': {'protocol': 'str', 'time': 'str'} }
+##
+# @ImageFormat:
+#
+# Available list of supported types.
+#
+# @png: PNG format
+#
+# @ppm: PPM format
+#
+# Since: 7.0
+#
+##
+{ 'enum': 'ImageFormat',
+ 'data': ['ppm', 'png'] }
+
##
# @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
+# @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)
@@ -87,6 +102,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. ppm is the set as the
+# default format. (Since 7.0)
+#
# Returns: Nothing on success
#
# Since: 0.14
@@ -99,7 +117,8 @@
#
##
{ 'command': 'screendump',
- 'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+ 'data': {'filename': 'str', '*device': 'str', '*head': 'int',
+ '*format': 'ImageFormat'},
'coroutine': true }
##
diff --git a/ui/console.c b/ui/console.c
index 40eebb6d2c..aab561e1ac 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
@@ -289,6 +292,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);
@@ -327,7 +413,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;
@@ -383,8 +470,17 @@ 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);
+ }
}
}
--
2.22.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG.
2022-03-01 6:44 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
@ 2022-03-07 16:41 ` Kshitij Suri
2022-03-11 12:20 ` Markus Armbruster
1 sibling, 0 replies; 23+ messages in thread
From: Kshitij Suri @ 2022-03-07 16:41 UTC (permalink / raw)
To: qemu-devel
Cc: soham.ghosh, thuth,
berrange@redhat.com >> Daniel P. Berrangé,
prerna.saxena, dgilbert, armbru, philippe.mathieu.daude, kraxel,
prachatos.mitra, eblake
Hi,
Hope this mail finds you well. I have updated the code as required and
request you to review and suggest changes that are needed to be
implemented. In case no change is required, please do let me know the
next steps for the same.
Regards,
Kshitij Suri
On 01/03/22 12:14 pm, Kshitij Suri wrote:
> Currently screendump only supports PPM format, which is un-compressed and not
> standard. Added a "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 <kshitij.suri@nutanix.com>
> ---
> diff to v1:
> - Removed repeated alpha conversion operation.
> - Modified logic to mirror png conversion in vnc-enc-tight.c file.
> - Added a new CONFIG_PNG parameter for libpng support.
> - Changed input format to enum instead of string.
> - Improved error handling.
> hmp-commands.hx | 11 ++---
> monitor/hmp-cmds.c | 19 ++++++++-
> qapi/ui.json | 25 +++++++++--
> ui/console.c | 102 +++++++++++++++++++++++++++++++++++++++++++--
> 4 files changed, 145 insertions(+), 12 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 70a9136ac2..5eda4eeb24 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'."
> + "Currently only 'png' and 'ppm' formats are supported.",
> .cmd = hmp_screendump,
> .coroutine = true,
> },
>
> SRST
> ``screendump`` *filename*
> - Save screen into PPM image *filename*.
> + Save screen as an image *filename*, with default format of PPM.
> ERST
>
> {
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 8c384dc1b2..9a640146eb 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1677,9 +1677,26 @@ 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_str(qdict, "format");
> Error *err = NULL;
> + ImageFormat format;
>
> - qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
> + int val = qapi_enum_parse(&ImageFormat_lookup, input_format, -1, &err);
> + if (val < 0) {
> + goto end;
> + }
> +
> + switch (val) {
> + case IMAGE_FORMAT_PNG:
> + format = IMAGE_FORMAT_PNG;
> + break;
> + default:
> + format = IMAGE_FORMAT_PPM;
> + }
> +
> + qmp_screendump(filename, id != NULL, id, id != NULL, head,
> + input_format != NULL, format, &err);
> +end:
> hmp_handle_error(mon, err);
> }
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 9354f4c467..6aa0dd7c1b 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -73,12 +73,27 @@
> ##
> { 'command': 'expire_password', 'data': {'protocol': 'str', 'time': 'str'} }
>
> +##
> +# @ImageFormat:
> +#
> +# Available list of supported types.
> +#
> +# @png: PNG format
> +#
> +# @ppm: PPM format
> +#
> +# Since: 7.0
> +#
> +##
> +{ 'enum': 'ImageFormat',
> + 'data': ['ppm', 'png'] }
> +
> ##
> # @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
> +# @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)
> @@ -87,6 +102,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. ppm is the set as the
> +# default format. (Since 7.0)
> +#
> # Returns: Nothing on success
> #
> # Since: 0.14
> @@ -99,7 +117,8 @@
> #
> ##
> { 'command': 'screendump',
> - 'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
> + 'data': {'filename': 'str', '*device': 'str', '*head': 'int',
> + '*format': 'ImageFormat'},
> 'coroutine': true }
>
> ##
> diff --git a/ui/console.c b/ui/console.c
> index 40eebb6d2c..aab561e1ac 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
> @@ -289,6 +292,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);
> @@ -327,7 +413,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;
> @@ -383,8 +470,17 @@ 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);
> + }
> }
> }
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG.
2022-03-01 6:44 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
2022-03-07 16:41 ` Kshitij Suri
@ 2022-03-11 12:20 ` Markus Armbruster
2022-03-15 4:36 ` Kshitij Suri
1 sibling, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2022-03-11 12:20 UTC (permalink / raw)
To: Kshitij Suri
Cc: soham.ghosh, thuth, prerna.saxena, qemu-devel, dgilbert, kraxel,
prachatos.mitra, eblake
Kshitij Suri <kshitij.suri@nutanix.com> writes:
> Currently screendump only supports PPM format, which is un-compressed and not
> standard. Added a "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 <kshitij.suri@nutanix.com>
> ---
[...]
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 8c384dc1b2..9a640146eb 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1677,9 +1677,26 @@ 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_str(qdict, "format");
> Error *err = NULL;
> + ImageFormat format;
>
> - qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
> + int val = qapi_enum_parse(&ImageFormat_lookup, input_format, -1, &err);
> + if (val < 0) {
> + goto end;
> + }
> +
> + switch (val) {
> + case IMAGE_FORMAT_PNG:
> + format = IMAGE_FORMAT_PNG;
> + break;
> + default:
> + format = IMAGE_FORMAT_PPM;
> + }
> +
> + qmp_screendump(filename, id != NULL, id, id != NULL, head,
> + input_format != NULL, format, &err);
> +end:
> hmp_handle_error(mon, err);
> }
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 9354f4c467..6aa0dd7c1b 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -73,12 +73,27 @@
> ##
> { 'command': 'expire_password', 'data': {'protocol': 'str', 'time': 'str'} }
>
> +##
> +# @ImageFormat:
> +#
> +# Available list of supported types.
This is just a hair better than "Lorem ipsum" :)
Suggest: Supported image format types.
> +#
> +# @png: PNG format
> +#
> +# @ppm: PPM format
> +#
> +# Since: 7.0
> +#
> +##
> +{ 'enum': 'ImageFormat',
> + 'data': ['ppm', 'png'] }
> +
> ##
> # @screendump:
> #
> -# Write a PPM of the VGA screen to a file.
> +# Write a screenshot of the VGA screen to a file.
Is "VGA screen" accurate? Or does this work for other displays, too?
> #
> -# @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)
> @@ -87,6 +102,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. ppm is the set as the
> +# default format. (Since 7.0)
I figure you mean "is set as the default". Suggest to replace the
sentence by "(default: ppm)".
> +#
> # Returns: Nothing on success
> #
> # Since: 0.14
> @@ -99,7 +117,8 @@
> #
> ##
> { 'command': 'screendump',
> - 'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
> + 'data': {'filename': 'str', '*device': 'str', '*head': 'int',
> + '*format': 'ImageFormat'},
> 'coroutine': true }
>
> ##
[...]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG.
2022-03-11 12:20 ` Markus Armbruster
@ 2022-03-15 4:36 ` Kshitij Suri
2022-03-15 10:06 ` Markus Armbruster
0 siblings, 1 reply; 23+ messages in thread
From: Kshitij Suri @ 2022-03-15 4:36 UTC (permalink / raw)
To: Markus Armbruster
Cc: soham.ghosh, thuth, prerna.saxena, qemu-devel, dgilbert, kraxel,
prachatos.mitra, eblake
On 11/03/22 5:50 pm, Markus Armbruster wrote:
> Kshitij Suri <kshitij.suri@nutanix.com> writes:
>
>> Currently screendump only supports PPM format, which is un-compressed and not
>> standard. Added a "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=SxmcA4FlCCy9O9eUaDUiSY37bauF6iJbDRVL--VUyTG5Vze_GFjmJuxgwAVYRjad&s=OIKnm9xXYjeat7TyIJ_-z9EvG2XYXMULNbHe0Bjzyjo&e=
>>
>> Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
>> ---
> [...]
>
>> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
>> index 8c384dc1b2..9a640146eb 100644
>> --- a/monitor/hmp-cmds.c
>> +++ b/monitor/hmp-cmds.c
>> @@ -1677,9 +1677,26 @@ 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_str(qdict, "format");
>> Error *err = NULL;
>> + ImageFormat format;
>>
>> - qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
>> + int val = qapi_enum_parse(&ImageFormat_lookup, input_format, -1, &err);
>> + if (val < 0) {
>> + goto end;
>> + }
>> +
>> + switch (val) {
>> + case IMAGE_FORMAT_PNG:
>> + format = IMAGE_FORMAT_PNG;
>> + break;
>> + default:
>> + format = IMAGE_FORMAT_PPM;
>> + }
>> +
>> + qmp_screendump(filename, id != NULL, id, id != NULL, head,
>> + input_format != NULL, format, &err);
>> +end:
>> hmp_handle_error(mon, err);
>> }
>>
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index 9354f4c467..6aa0dd7c1b 100644
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -73,12 +73,27 @@
>> ##
>> { 'command': 'expire_password', 'data': {'protocol': 'str', 'time': 'str'} }
>>
>> +##
>> +# @ImageFormat:
>> +#
>> +# Available list of supported types.
> This is just a hair better than "Lorem ipsum" :)
>
> Suggest: Supported image format types.
Will add in the updated patch
>
>> +#
>> +# @png: PNG format
>> +#
>> +# @ppm: PPM format
>> +#
>> +# Since: 7.0
>> +#
>> +##
>> +{ 'enum': 'ImageFormat',
>> + 'data': ['ppm', 'png'] }
>> +
>> ##
>> # @screendump:
>> #
>> -# Write a PPM of the VGA screen to a file.
>> +# Write a screenshot of the VGA screen to a file.
> Is "VGA screen" accurate? Or does this work for other displays, too?
The patch didn't modify any display changes and VGA screen was
previously supported display type.
>
>> #
>> -# @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)
>> @@ -87,6 +102,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. ppm is the set as the
>> +# default format. (Since 7.0)
> I figure you mean "is set as the default". Suggest to replace the
> sentence by "(default: ppm)".
Will add in the updated patch.
>
>> +#
>> # Returns: Nothing on success
>> #
>> # Since: 0.14
>> @@ -99,7 +117,8 @@
>> #
>> ##
>> { 'command': 'screendump',
>> - 'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
>> + 'data': {'filename': 'str', '*device': 'str', '*head': 'int',
>> + '*format': 'ImageFormat'},
>> 'coroutine': true }
>>
>> ##
> [...]
Thank you for your review!
Regards,
Kshitij Suri
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG.
2022-03-15 4:36 ` Kshitij Suri
@ 2022-03-15 10:06 ` Markus Armbruster
2022-03-15 10:19 ` Daniel P. Berrangé
0 siblings, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2022-03-15 10:06 UTC (permalink / raw)
To: Kshitij Suri
Cc: soham.ghosh, thuth, prerna.saxena, qemu-devel, dgilbert, kraxel,
prachatos.mitra, eblake
Kshitij Suri <kshitij.suri@nutanix.com> writes:
> On 11/03/22 5:50 pm, Markus Armbruster wrote:
>> Kshitij Suri <kshitij.suri@nutanix.com> writes:
>>
>>> Currently screendump only supports PPM format, which is un-compressed and not
>>> standard. Added a "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=SxmcA4FlCCy9O9eUaDUiSY37bauF6iJbDRVL--VUyTG5Vze_GFjmJuxgwAVYRjad&s=OIKnm9xXYjeat7TyIJ_-z9EvG2XYXMULNbHe0Bjzyjo&e=
>>>
>>> Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
[...]
>>> diff --git a/qapi/ui.json b/qapi/ui.json
>>> index 9354f4c467..6aa0dd7c1b 100644
>>> --- a/qapi/ui.json
>>> +++ b/qapi/ui.json
[...]
>>> ##
>>> # @screendump:
>>> #
>>> -# Write a PPM of the VGA screen to a file.
>>> +# Write a screenshot of the VGA screen to a file.
>>
>> Is "VGA screen" accurate? Or does this work for other displays, too?
>
> The patch didn't modify any display changes and VGA screen was
> previously supported display type.
Let me rephrase my question: was "VGA screen" accurate before your
patch?
[...]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG.
2022-03-15 10:06 ` Markus Armbruster
@ 2022-03-15 10:19 ` Daniel P. Berrangé
2022-03-15 13:23 ` Markus Armbruster
2022-03-16 18:11 ` Kshitij Suri
0 siblings, 2 replies; 23+ messages in thread
From: Daniel P. Berrangé @ 2022-03-15 10:19 UTC (permalink / raw)
To: Markus Armbruster
Cc: soham.ghosh, prerna.saxena, qemu-devel, dgilbert, Kshitij Suri,
kraxel, thuth, prachatos.mitra, eblake
On Tue, Mar 15, 2022 at 11:06:31AM +0100, Markus Armbruster wrote:
> Kshitij Suri <kshitij.suri@nutanix.com> writes:
>
> > On 11/03/22 5:50 pm, Markus Armbruster wrote:
> >> Kshitij Suri <kshitij.suri@nutanix.com> writes:
> >>
> >>> Currently screendump only supports PPM format, which is un-compressed and not
> >>> standard. Added a "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=SxmcA4FlCCy9O9eUaDUiSY37bauF6iJbDRVL--VUyTG5Vze_GFjmJuxgwAVYRjad&s=OIKnm9xXYjeat7TyIJ_-z9EvG2XYXMULNbHe0Bjzyjo&e=
> >>>
> >>> Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
>
> [...]
>
> >>> diff --git a/qapi/ui.json b/qapi/ui.json
> >>> index 9354f4c467..6aa0dd7c1b 100644
> >>> --- a/qapi/ui.json
> >>> +++ b/qapi/ui.json
>
> [...]
>
> >>> ##
> >>> # @screendump:
> >>> #
> >>> -# Write a PPM of the VGA screen to a file.
> >>> +# Write a screenshot of the VGA screen to a file.
> >>
> >> Is "VGA screen" accurate? Or does this work for other displays, too?
> >
> > The patch didn't modify any display changes and VGA screen was
> > previously supported display type.
>
> Let me rephrase my question: was "VGA screen" accurate before your
> patch?
No, it would be better phrased as
"Capture the specified screen contents and write it to a file"
In a multi-head scenario, it can be any of the output heads, and
whether the head is in a VGA mode or not is irrelevant to the
command functionality.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG.
2022-03-15 10:19 ` Daniel P. Berrangé
@ 2022-03-15 13:23 ` Markus Armbruster
2022-03-16 18:11 ` Kshitij Suri
1 sibling, 0 replies; 23+ messages in thread
From: Markus Armbruster @ 2022-03-15 13:23 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: soham.ghosh, prerna.saxena, qemu-devel, dgilbert, Kshitij Suri,
kraxel, thuth, prachatos.mitra, eblake
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Tue, Mar 15, 2022 at 11:06:31AM +0100, Markus Armbruster wrote:
>> Kshitij Suri <kshitij.suri@nutanix.com> writes:
>>
>> > On 11/03/22 5:50 pm, Markus Armbruster wrote:
>> >> Kshitij Suri <kshitij.suri@nutanix.com> writes:
>> >>
>> >>> Currently screendump only supports PPM format, which is un-compressed and not
>> >>> standard. Added a "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=SxmcA4FlCCy9O9eUaDUiSY37bauF6iJbDRVL--VUyTG5Vze_GFjmJuxgwAVYRjad&s=OIKnm9xXYjeat7TyIJ_-z9EvG2XYXMULNbHe0Bjzyjo&e=
>> >>>
>> >>> Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
>>
>> [...]
>>
>> >>> diff --git a/qapi/ui.json b/qapi/ui.json
>> >>> index 9354f4c467..6aa0dd7c1b 100644
>> >>> --- a/qapi/ui.json
>> >>> +++ b/qapi/ui.json
>>
>> [...]
>>
>> >>> ##
>> >>> # @screendump:
>> >>> #
>> >>> -# Write a PPM of the VGA screen to a file.
>> >>> +# Write a screenshot of the VGA screen to a file.
>> >>
>> >> Is "VGA screen" accurate? Or does this work for other displays, too?
>> >
>> > The patch didn't modify any display changes and VGA screen was
>> > previously supported display type.
>>
>> Let me rephrase my question: was "VGA screen" accurate before your
>> patch?
>
> No, it would be better phrased as
>
> "Capture the specified screen contents and write it to a file"
>
> In a multi-head scenario, it can be any of the output heads, and
> whether the head is in a VGA mode or not is irrelevant to the
> command functionality.
Makes sense to me.
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG.
2022-03-15 10:19 ` Daniel P. Berrangé
2022-03-15 13:23 ` Markus Armbruster
@ 2022-03-16 18:11 ` Kshitij Suri
1 sibling, 0 replies; 23+ messages in thread
From: Kshitij Suri @ 2022-03-16 18:11 UTC (permalink / raw)
To: Daniel P. Berrangé, Markus Armbruster
Cc: soham.ghosh, prerna.saxena, qemu-devel, dgilbert, kraxel, thuth,
prachatos.mitra, eblake
On 15/03/22 3:49 pm, Daniel P. Berrangé wrote:
> On Tue, Mar 15, 2022 at 11:06:31AM +0100, Markus Armbruster wrote:
>> Kshitij Suri <kshitij.suri@nutanix.com> writes:
>>
>>> On 11/03/22 5:50 pm, Markus Armbruster wrote:
>>>> Kshitij Suri <kshitij.suri@nutanix.com> writes:
>>>>
>>>>> Currently screendump only supports PPM format, which is un-compressed and not
>>>>> standard. Added a "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=SxmcA4FlCCy9O9eUaDUiSY37bauF6iJbDRVL--VUyTG5Vze_GFjmJuxgwAVYRjad&s=OIKnm9xXYjeat7TyIJ_-z9EvG2XYXMULNbHe0Bjzyjo&e=
>>>>>
>>>>> Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
>> [...]
>>
>>>>> diff --git a/qapi/ui.json b/qapi/ui.json
>>>>> index 9354f4c467..6aa0dd7c1b 100644
>>>>> --- a/qapi/ui.json
>>>>> +++ b/qapi/ui.json
>> [...]
>>
>>>>> ##
>>>>> # @screendump:
>>>>> #
>>>>> -# Write a PPM of the VGA screen to a file.
>>>>> +# Write a screenshot of the VGA screen to a file.
>>>> Is "VGA screen" accurate? Or does this work for other displays, too?
>>> The patch didn't modify any display changes and VGA screen was
>>> previously supported display type.
>> Let me rephrase my question: was "VGA screen" accurate before your
>> patch?
> No, it would be better phrased as
>
> "Capture the specified screen contents and write it to a file"
>
> In a multi-head scenario, it can be any of the output heads, and
> whether the head is in a VGA mode or not is irrelevant to the
> command functionality.
>
> Regards,
> Daniel
Thank you! Will modify in the upcoming patch.
Regards,
Kshitij Suri
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
@ 2022-02-28 5:22 Kshitij Suri
2022-02-28 5:22 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
0 siblings, 1 reply; 23+ messages in thread
From: Kshitij Suri @ 2022-02-28 5:22 UTC (permalink / raw)
To: qemu-devel
Cc: soham.ghosh, thuth, prerna.saxena, dgilbert, armbru,
Kshitij Suri, kraxel, prachatos.mitra, eblake
Libpng is only detected if VNC is enabled currently. This patch adds a
generalised png option in the meson build which is aimed to replace use of
CONFIG_VNC_PNG with CONFIG_PNG.
Signed-off-by: Kshitij Suri <kshitij.suri@nutanix.com>
---
meson.build | 10 +++++-----
meson_options.txt | 4 ++--
ui/vnc-enc-tight.c | 18 +++++++++---------
ui/vnc.c | 4 ++--
ui/vnc.h | 2 +-
5 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/meson.build b/meson.build
index 8df40bfac4..3638957fc5 100644
--- a/meson.build
+++ b/meson.build
@@ -1112,14 +1112,14 @@ if gtkx11.found()
x11 = dependency('x11', method: 'pkg-config', required: gtkx11.found(),
kwargs: static_kwargs)
endif
-vnc = not_found
png = not_found
+png = dependency('libpng', required: get_option('png'),
+ method: 'pkg-config', kwargs: static_kwargs)
+vnc = not_found
jpeg = not_found
sasl = not_found
if get_option('vnc').allowed() and have_system
vnc = declare_dependency() # dummy dependency
- png = dependency('libpng', required: get_option('vnc_png'),
- method: 'pkg-config', kwargs: static_kwargs)
jpeg = dependency('libjpeg', required: get_option('vnc_jpeg'),
method: 'pkg-config', kwargs: static_kwargs)
sasl = cc.find_library('sasl2', has_headers: ['sasl/sasl.h'],
@@ -1537,9 +1537,9 @@ config_host_data.set('CONFIG_TPM', have_tpm)
config_host_data.set('CONFIG_USB_LIBUSB', libusb.found())
config_host_data.set('CONFIG_VDE', vde.found())
config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', have_vhost_user_blk_server)
+config_host_data.set('CONFIG_PNG', png.found())
config_host_data.set('CONFIG_VNC', vnc.found())
config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
-config_host_data.set('CONFIG_VNC_PNG', png.found())
config_host_data.set('CONFIG_VNC_SASL', sasl.found())
config_host_data.set('CONFIG_VIRTFS', have_virtfs)
config_host_data.set('CONFIG_VTE', vte.found())
@@ -3579,11 +3579,11 @@ summary_info += {'curses support': curses}
summary_info += {'virgl support': virgl}
summary_info += {'curl support': curl}
summary_info += {'Multipath support': mpathpersist}
+summary_info += {'PNG support': png}
summary_info += {'VNC support': vnc}
if vnc.found()
summary_info += {'VNC SASL support': sasl}
summary_info += {'VNC JPEG support': jpeg}
- summary_info += {'VNC PNG support': png}
endif
if targetos not in ['darwin', 'haiku', 'windows']
summary_info += {'OSS support': oss}
diff --git a/meson_options.txt b/meson_options.txt
index 52b11cead4..d85734f8e6 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -177,12 +177,12 @@ option('vde', type : 'feature', value : 'auto',
description: 'vde network backend support')
option('virglrenderer', type : 'feature', value : 'auto',
description: 'virgl rendering support')
+option('png', type : 'feature', value : 'auto',
+ description: 'PNG support with libpng')
option('vnc', type : 'feature', value : 'auto',
description: 'VNC server')
option('vnc_jpeg', type : 'feature', value : 'auto',
description: 'JPEG lossy compression for VNC server')
-option('vnc_png', type : 'feature', value : 'auto',
- description: 'PNG compression for VNC server')
option('vnc_sasl', type : 'feature', value : 'auto',
description: 'SASL authentication for VNC server')
option('vte', type : 'feature', value : 'auto',
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index cebd35841a..a23ad712eb 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -32,7 +32,7 @@
INT32 definitions between jmorecfg.h (included by jpeglib.h) and
Win32 basetsd.h (included by windows.h). */
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
/* The following define is needed by pngconf.h. Otherwise it won't compile,
because setjmp.h was already included by qemu-common.h. */
#define PNG_SKIP_SETJMP_CHECK
@@ -95,7 +95,7 @@ static const struct {
};
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
static const struct {
int png_zlib_level, png_filters;
} tight_png_conf[] = {
@@ -919,7 +919,7 @@ static int send_full_color_rect(VncState *vs, int x, int y, int w, int h)
int stream = 0;
ssize_t bytes;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
return send_png_rect(vs, x, y, w, h, NULL);
}
@@ -966,7 +966,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
int stream = 1;
int level = tight_conf[vs->tight->compression].mono_zlib_level;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
int ret;
int bpp = vs->client_pf.bytes_per_pixel * 8;
@@ -1020,7 +1020,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
struct palette_cb_priv {
VncState *vs;
uint8_t *header;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
png_colorp png_palette;
#endif
};
@@ -1082,7 +1082,7 @@ static int send_palette_rect(VncState *vs, int x, int y,
int colors;
ssize_t bytes;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
if (tight_can_send_png_rect(vs, w, h)) {
return send_png_rect(vs, x, y, w, h, palette);
}
@@ -1233,7 +1233,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality)
/*
* PNG compression stuff.
*/
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
static void write_png_palette(int idx, uint32_t pix, void *opaque)
{
struct palette_cb_priv *priv = opaque;
@@ -1379,7 +1379,7 @@ static int send_png_rect(VncState *vs, int x, int y, int w, int h,
buffer_reset(&vs->tight->png);
return 1;
}
-#endif /* CONFIG_VNC_PNG */
+#endif /* CONFIG_PNG */
static void vnc_tight_start(VncState *vs)
{
@@ -1706,7 +1706,7 @@ void vnc_tight_clear(VncState *vs)
#ifdef CONFIG_VNC_JPEG
buffer_free(&vs->tight->jpeg);
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
buffer_free(&vs->tight->png);
#endif
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 3ccd33dedc..a588ddff1c 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2165,7 +2165,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
vs->features |= VNC_FEATURE_TIGHT_MASK;
vs->vnc_encoding = enc;
break;
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
case VNC_ENCODING_TIGHT_PNG:
vs->features |= VNC_FEATURE_TIGHT_PNG_MASK;
vs->vnc_encoding = enc;
@@ -3248,7 +3248,7 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
#ifdef CONFIG_VNC_JPEG
buffer_init(&vs->tight->jpeg, "vnc-tight-jpeg/%p", sioc);
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
buffer_init(&vs->tight->png, "vnc-tight-png/%p", sioc);
#endif
buffer_init(&vs->zlib.zlib, "vnc-zlib/%p", sioc);
diff --git a/ui/vnc.h b/ui/vnc.h
index a7149831f9..a60fb13115 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -201,7 +201,7 @@ typedef struct VncTight {
#ifdef CONFIG_VNC_JPEG
Buffer jpeg;
#endif
-#ifdef CONFIG_VNC_PNG
+#ifdef CONFIG_PNG
Buffer png;
#endif
int levels[4];
--
2.22.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG.
2022-02-28 5:22 [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG Kshitij Suri
@ 2022-02-28 5:22 ` Kshitij Suri
0 siblings, 0 replies; 23+ messages in thread
From: Kshitij Suri @ 2022-02-28 5:22 UTC (permalink / raw)
To: qemu-devel
Cc: soham.ghosh, thuth, prerna.saxena, dgilbert, armbru,
Kshitij Suri, kraxel, prachatos.mitra, eblake
Currently screendump only supports PPM format, which is un-compressed and not
standard. Added a "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 <kshitij.suri@nutanix.com>
---
diff to v1:
- Removed repeated alpha conversion operation.
- Modified logic to mirror png conversion in vnc-enc-tight.c file.
- Added a new CONFIG_PNG parameter for libpng support.
- Changed input format to enum instead of string.
- Improved error handling.
hmp-commands.hx | 11 ++---
monitor/hmp-cmds.c | 19 ++++++++-
qapi/ui.json | 25 +++++++++--
ui/console.c | 102 +++++++++++++++++++++++++++++++++++++++++++--
4 files changed, 145 insertions(+), 12 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 70a9136ac2..5eda4eeb24 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'."
+ "Currently only 'png' and 'ppm' formats are supported.",
.cmd = hmp_screendump,
.coroutine = true,
},
SRST
``screendump`` *filename*
- Save screen into PPM image *filename*.
+ Save screen as an image *filename*, with default format of PPM.
ERST
{
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 8c384dc1b2..9a640146eb 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1677,9 +1677,26 @@ 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_str(qdict, "format");
Error *err = NULL;
+ ImageFormat format;
- qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
+ int val = qapi_enum_parse(&ImageFormat_lookup, input_format, -1, &err);
+ if (val < 0) {
+ goto end;
+ }
+
+ switch (val) {
+ case IMAGE_FORMAT_PNG:
+ format = IMAGE_FORMAT_PNG;
+ break;
+ default:
+ format = IMAGE_FORMAT_PPM;
+ }
+
+ qmp_screendump(filename, id != NULL, id, id != NULL, head,
+ input_format != NULL, format, &err);
+end:
hmp_handle_error(mon, err);
}
diff --git a/qapi/ui.json b/qapi/ui.json
index 9354f4c467..6aa0dd7c1b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -73,12 +73,27 @@
##
{ 'command': 'expire_password', 'data': {'protocol': 'str', 'time': 'str'} }
+##
+# @ImageFormat:
+#
+# Available list of supported types.
+#
+# @png: PNG format
+#
+# @ppm: PPM format
+#
+# Since: 7.0
+#
+##
+{ 'enum': 'ImageFormat',
+ 'data': ['ppm', 'png'] }
+
##
# @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
+# @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)
@@ -87,6 +102,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. ppm is the set as the
+# default format. (Since 7.0)
+#
# Returns: Nothing on success
#
# Since: 0.14
@@ -99,7 +117,8 @@
#
##
{ 'command': 'screendump',
- 'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+ 'data': {'filename': 'str', '*device': 'str', '*head': 'int',
+ '*format': 'ImageFormat'},
'coroutine': true }
##
diff --git a/ui/console.c b/ui/console.c
index 40eebb6d2c..aab561e1ac 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
@@ -289,6 +292,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);
@@ -327,7 +413,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;
@@ -383,8 +470,17 @@ 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);
+ }
}
}
--
2.22.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
end of thread, other threads:[~2022-03-29 7:09 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-22 10:49 [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG Kshitij Suri
2022-03-22 10:49 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
2022-03-28 9:50 ` Kshitij Suri
2022-03-28 9:52 ` Daniel P. Berrangé
2022-03-28 9:56 ` Kshitij Suri
2022-03-28 9:49 ` [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG Kshitij Suri
-- strict thread matches above, loose matches on Subject: below --
2022-03-28 16:54 Kshitij Suri
2022-03-28 16:54 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
2022-03-29 6:42 ` Markus Armbruster
2022-03-29 7:06 ` Kshitij Suri
2022-03-22 8:18 [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG Kshitij Suri
2022-03-22 8:18 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
2022-03-22 9:47 ` Daniel P. Berrangé
2022-03-22 9:56 ` Kshitij Suri
2022-03-22 10:15 ` Markus Armbruster
2022-03-22 10:19 ` Kshitij Suri
2022-03-01 6:44 [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG Kshitij Suri
2022-03-01 6:44 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
2022-03-07 16:41 ` Kshitij Suri
2022-03-11 12:20 ` Markus Armbruster
2022-03-15 4:36 ` Kshitij Suri
2022-03-15 10:06 ` Markus Armbruster
2022-03-15 10:19 ` Daniel P. Berrangé
2022-03-15 13:23 ` Markus Armbruster
2022-03-16 18:11 ` Kshitij Suri
2022-02-28 5:22 [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG Kshitij Suri
2022-02-28 5:22 ` [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG Kshitij Suri
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.