* Kshitij Suri (kshitij.s...@nutanix.com) wrote:
Currently screendump only supports PPM format, which is un-compressed and not
standard. Added an "format" parameter to qemu monitor screendump capabilites
to support PNG image capture using libpng. The param was added in QAPI schema
of screendump present in ui.json along with png_save() function which converts
pixman_image to PNG. HMP command equivalent was also modified to support the
feature.
Example usage:
{ "execute": "screendump", "arguments": { "filename": "/tmp/image",
"format":"png" } }
Resolves:
https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_qemu-2Dproject_qemu_-2D_issues_718&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w&m=YAoUy9TYXWb8lktv5hZwRWjZEBcoHKxfdrHMd8KkTnQAESANHEcW96C1ukSngzJ2&s=xJP6q9IE0req5UkOFLablyJWWREdmwS1NX4Yse0pYxE&e=
Signed-off-by: Kshitij Suri <kshitij.s...@nutanix.com>
---
hmp-commands.hx | 11 ++--
monitor/hmp-cmds.c | 4 +-
qapi/ui.json | 7 ++-
ui/console.c | 153 ++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 165 insertions(+), 10 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 70a9136ac2..2163337f35 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -244,17 +244,18 @@ ERST
{
.name = "screendump",
- .args_type = "filename:F,device:s?,head:i?",
- .params = "filename [device [head]]",
- .help = "save screen from head 'head' of display device 'device'
"
- "into PPM image 'filename'",
+ .args_type = "filename:F,device:s?,head:i?,format:f?",
+ .params = "filename [device [head]] [format]",
+ .help = "save screen from head 'head' of display device 'device'"
+ "in specified format 'format' as image 'filename'."
+ "Default format for screendump is PPM.",
.cmd = hmp_screendump,
.coroutine = true,
},
SRST
``screendump`` *filename*
- Save screen into PPM image *filename*.
+ Save screen as image *filename*.
ERST
{
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 2669156b28..3fb1394561 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1665,9 +1665,11 @@ hmp_screendump(Monitor *mon, const QDict *qdict)
const char *filename = qdict_get_str(qdict, "filename");
const char *id = qdict_get_try_str(qdict, "device");
int64_t head = qdict_get_try_int(qdict, "head", 0);
+ const char *format = qdict_get_str(qdict, "format");
Error *err = NULL;
- qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
+ qmp_screendump(filename, id != NULL, id, id != NULL, head,
+ format != NULL, format, &err);
hmp_handle_error(mon, err);
}
I think that's OK from the HMP front, see some questions below.
diff --git a/qapi/ui.json b/qapi/ui.json
index 9354f4c467..9fdb56b60b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -76,7 +76,7 @@
##
# @screendump:
#
-# Write a PPM of the VGA screen to a file.
+# Write a screenshot of the VGA screen to a file.
#
# @filename: the path of a new PPM file to store the image
#
@@ -87,6 +87,9 @@
# parameter is missing, head #0 will be used. Also note that the head
# can only be specified in conjunction with the device ID. (Since 2.12)
#
+# @format: image format for screendump is specified. Currently only PNG and
+# PPM are supported.
+#
# Returns: Nothing on success
#
# Since: 0.14
@@ -99,7 +102,7 @@
#
##
{ 'command': 'screendump',
- 'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+ 'data': {'filename': 'str', '*device': 'str', '*head': 'int', '*format':
'str'},
'coroutine': true }
##
diff --git a/ui/console.c b/ui/console.c
index 40eebb6d2c..7813b195ac 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -37,6 +37,9 @@
#include "exec/memory.h"
#include "io/channel-file.h"
#include "qom/object.h"
+#ifdef CONFIG_VNC_PNG
+#include "png.h"
+#endif
#define DEFAULT_BACKSCROLL 512
#define CONSOLE_CURSOR_PERIOD 500
@@ -289,6 +292,137 @@ void graphic_hw_invalidate(QemuConsole *con)
}
}
+#ifdef CONFIG_VNC_PNG
+/**
+ * a8r8g8b8_to_rgba: Convert a8r8g8b8 to rgba format
+ *
+ * @dst: Destination pointer.
+ * @src: Source pointer.
+ * @n_pixels: Size of image.
+ */
+static void a8r8g8b8_to_rgba(uint32_t *dst, uint32_t *src, int n_pixels)
+{
+ uint8_t *dst8 = (uint8_t *)dst;
+ int i;
+
+ for (i = 0; i < n_pixels; ++i) {
+ uint32_t p = src[i];
+ uint8_t a, r, g, b;
+
+ a = (p & 0xff000000) >> 24;
+ r = (p & 0x00ff0000) >> 16;
+ g = (p & 0x0000ff00) >> 8;
+ b = (p & 0x000000ff) >> 0;
+
+ if (a != 0) {
+ #define DIVIDE(c, a) \
+ do { \
+ int t = ((c) * 255) / a; \
+ (c) = t < 0 ? 0 : t > 255 ? 255 : t; \
I can't see how 't' can be less than 0 here?
+ } while (0)
+
+ DIVIDE(r, a);
+ DIVIDE(g, a);
+ DIVIDE(b, a);
+ #undef DIVIDE
+ }
+
+ *dst8++ = r;
+ *dst8++ = g;
+ *dst8++ = b;
+ *dst8++ = a;
+ }
+}
Now, I'm not too much of a graphics person, but I'm a bit confused; this
seems to be applying the alpha scaling to the r/g/b/ data - but then
still including the alpha value in the output; doesn't that mean that
the thing displaying the result will also apply the alpha scaling again?
+/**
+ * png_save: Take a screenshot as PNG
+ *
+ * Saves screendump as a PNG file
+ *
+ * Returns true for success or false for error.
+ * Inspired from png test utils from
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_aseprite_pixman&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w&m=YAoUy9TYXWb8lktv5hZwRWjZEBcoHKxfdrHMd8KkTnQAESANHEcW96C1ukSngzJ2&s=WhMqvptTX23UfMHwjTaBDTYU4xqtPO0dvK7B8Tc1nCw&e=
+ *
+ * @fd: File descriptor for PNG file.
+ * @image: Image data in pixman format.
+ * @errp: Pointer to an error.
+ */
+static bool png_save(int fd, pixman_image_t *image, Error **errp)
+{
+ int width = pixman_image_get_width(image);
+ int height = pixman_image_get_height(image);
+ int stride = width * 4;
+ g_autofree uint32_t *src_data = g_malloc(height * stride);
+ g_autofree uint32_t *dest_data = g_malloc(height * stride);
+ g_autoptr(pixman_image_t) src_copy;
+ g_autoptr(pixman_image_t) dest_copy;
+ g_autofree png_struct *write_struct;
+ g_autofree png_info *info_struct;
+ g_autofree png_bytep *row_pointers = g_malloc(height * sizeof(png_bytep));
+ FILE *f = fdopen(fd, "wb");
+ int y;
+ if (!f) {
+ error_setg(errp, "Failed to create file from file descriptor");
+ return false;
+ }
+
+ src_copy = pixman_image_create_bits(PIXMAN_a8r8g8b8, width, height,
+ src_data, stride);
+
+ pixman_image_composite32(PIXMAN_OP_SRC, image, NULL, src_copy, 0, 0, 0, 0,
+ 0, 0, width, height);
Nothing in the rest of qemu uses pixman_image_composite*32* - all the
rest uses the plain version; what's the difference?
+ memcpy(dest_data, src_data, sizeof(*src_data));
Why if you're just about to nuke dest_data with the converted version?
+ a8r8g8b8_to_rgba(dest_data, src_data, height * width);
+
+ for (y = 0; y < height; ++y) {
+ row_pointers[y] = (png_bytep)(src_data + y * width);
+ }
+
+ write_struct = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL,
+ NULL, NULL);
+ if (!write_struct) {
+ error_setg(errp, "PNG creation failed. Unable to write struct");
+ return false;
+ }
+
+ info_struct = png_create_info_struct(write_struct);
+
+ if (!info_struct) {
+ error_setg(errp, "PNG creation failed. Unable to write info");
+ return false;
+ }
+
+ png_init_io(write_struct, f);
+
+ png_set_IHDR(write_struct, info_struct, width, height, 8,
+ PNG_COLOR_TYPE_RGB_ALPHA, PNG_INTERLACE_NONE,
+ PNG_COMPRESSION_TYPE_BASE, PNG_FILTER_TYPE_BASE);
+
+ png_write_info(write_struct, info_struct);
+
+ png_write_image(write_struct, row_pointers);
Had you considered using png_write_row and avoiding having to
hold/allocate the destination and row_pointers at all?
(In actual fact, can you steal the code in ui/vnc-enc-tight.c
send_png_rect ? )
Dave
+ png_write_end(write_struct, NULL);
+
+ if (fclose(f) != 0) {
+ error_setg(errp, "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 VNC PNG support for png screendump");
+ return false;
+}
+
+#endif /* CONFIG_VNC_PNG */
+
static bool ppm_save(int fd, pixman_image_t *image, Error **errp)
{
int width = pixman_image_get_width(image);
@@ -327,7 +461,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,
+ const char *format, Error **errp)
{
g_autoptr(pixman_image_t) image = NULL;
QemuConsole *con;
@@ -383,8 +518,22 @@ 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)) {
+
+ if (has_format && strcmp(format, "png") == 0) {
+ /* PNG format specified for screendump */
+ if (!png_save(fd, image, errp)) {
+ qemu_unlink(filename);
+ }
+ } else if (!has_format || (has_format && strcmp(format, "ppm") == 0)) {
+ /* PPM format specified/default for screendump */
+ if (!ppm_save(fd, image, errp)) {
+ qemu_unlink(filename);
+ }
+ } else {
+ /* Invalid specified for screendump */
+ error_setg(errp, "Invalid format provided for screendump.");
qemu_unlink(filename);
+ return;
}
}
--
2.22.3
