[PATCH] Added parameter to take screenshot with screendump as PNG
Currently screendump only supports PPM format, which is un-compressed and not
standard. Added an "format" parameter to qemu monitor screendump capabilites
to support PNG image capture using libpng. The param was added in QAPI schema
of screendump present in ui.json along with png_save() function which converts
pixman_image to PNG. HMP command equivalent was also modified to support the
feature.
Example usage:
{ "execute": "screendump", "arguments": { "filename": "/tmp/image",
"format":"png" } }
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/718
Signed-off-by: Kshitij Suri
---
hmp-commands.hx| 11 ++--
monitor/hmp-cmds.c | 4 +-
qapi/ui.json | 7 ++-
ui/console.c | 153 -
4 files changed, 165 insertions(+), 10 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 70a9136ac2..2163337f35 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -244,17 +244,18 @@ ERST
{
.name = "screendump",
-.args_type = "filename:F,device:s?,head:i?",
-.params = "filename [device [head]]",
-.help = "save screen from head 'head' of display device 'device'
"
- "into PPM image 'filename'",
+.args_type = "filename:F,device:s?,head:i?,format:f?",
+.params = "filename [device [head]] [format]",
+.help = "save screen from head 'head' of display device 'device'"
+ "in specified format 'format' as image 'filename'."
+ "Default format for screendump is PPM.",
.cmd= hmp_screendump,
.coroutine = true,
},
SRST
``screendump`` *filename*
- Save screen into PPM image *filename*.
+ Save screen as image *filename*.
ERST
{
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 2669156b28..3fb1394561 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1665,9 +1665,11 @@ hmp_screendump(Monitor *mon, const QDict *qdict)
const char *filename = qdict_get_str(qdict, "filename");
const char *id = qdict_get_try_str(qdict, "device");
int64_t head = qdict_get_try_int(qdict, "head", 0);
+const char *format = qdict_get_str(qdict, "format");
Error *err = NULL;
-qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
+qmp_screendump(filename, id != NULL, id, id != NULL, head,
+ format != NULL, format, &err);
hmp_handle_error(mon, err);
}
diff --git a/qapi/ui.json b/qapi/ui.json
index 9354f4c467..9fdb56b60b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -76,7 +76,7 @@
##
# @screendump:
#
-# Write a PPM of the VGA screen to a file.
+# Write a screenshot of the VGA screen to a file.
#
# @filename: the path of a new PPM file to store the image
#
@@ -87,6 +87,9 @@
#parameter is missing, head #0 will be used. Also note that the head
#can only be specified in conjunction with the device ID. (Since 2.12)
#
+# @format: image format for screendump is specified. Currently only PNG and
+# PPM are supported.
+#
# Returns: Nothing on success
#
# Since: 0.14
@@ -99,7 +102,7 @@
#
##
{ 'command': 'screendump',
- 'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+ 'data': {'filename': 'str', '*device': 'str', '*head': 'int', '*format':
'str'},
'coroutine': true }
##
diff --git a/ui/console.c b/ui/console.c
index 40eebb6d2c..7813b195ac 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -37,6 +37,9 @@
#include "exec/memory.h"
#include "io/channel-file.h"
#include "qom/object.h"
+#ifdef CONFIG_VNC_PNG
+#include "png.h"
+#endif
#define DEFAULT_BACKSCROLL 512
#define CONSOLE_CURSOR_PERIOD 500
@@ -289,6 +292,137 @@ void graphic_hw_invalidate(QemuConsole *con)
}
}
+#ifdef CONFIG_VNC_PNG
+/**
+ * a8r8g8b8_to_rgba: Convert a8r8g8b8 to rgba format
+ *
+ * @dst: Destination pointer.
+ * @src: Source pointer.
+ * @n_pixels: Size of image.
+ */
+static void a8r8g8b8_to_rgba(uint32_t *dst, uint32_t *src, int n_pixels)
+{
+uint8_t *dst8 = (uint8_t *)dst;
+int i;
+
+for (i = 0; i < n_pixels; ++i) {
+uint32_t p = src[i];
+uint8_t a, r, g, b;
+
+a = (p & 0xff00) >> 24;
+r = (p & 0x00ff) >> 16;
+g = (p & 0xff00) >> 8;
+b = (p & 0x00ff) >> 0;
+
+if (a != 0) {
+#define DIVIDE(c, a) \
+
[PATCH v2] Added parameter to take screenshot with screendump as PNG
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
---
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.
hmp-commands.hx| 11 +++---
meson.build| 13 +--
meson_options.txt | 2 +
monitor/hmp-cmds.c | 8 +++-
qapi/ui.json | 24 ++--
ui/console.c | 97 --
6 files changed, 139 insertions(+), 16 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 70a9136ac2..e43c9954b5 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*.
ERST
{
diff --git a/meson.build b/meson.build
index 8df40bfac4..fd550c01ec 100644
--- a/meson.build
+++ b/meson.build
@@ -1112,13 +1112,16 @@ 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
+vnc_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'),
+ vnc_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)
@@ -1537,9 +1540,10 @@ 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_PNG', vnc_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 +3583,12 @@ 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}
+ summary_info += {'VNC PNG support': vnc_png}
endif
if targetos not in ['darwin', 'haiku', 'windows']
summary_info += {'
Re: [PATCH] Added parameter to take screenshot with screendump as PNG
On 22/02/22 10:04 pm, Daniel P. Berrangé wrote:
On Tue, Feb 22, 2022 at 03:27:58PM +, Kshitij Suri wrote:
Currently screendump only supports PPM format, which is un-compressed and not
standard. Added an "format" parameter to qemu monitor screendump capabilites
to support PNG image capture using libpng. The param was added in QAPI schema
of screendump present in ui.json along with png_save() function which converts
pixman_image to PNG. HMP command equivalent was also modified to support the
feature.
Example usage:
{ "execute": "screendump", "arguments": { "filename": "/tmp/image",
"format":"png" } }
Resolves:https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_qemu-2Dproject_qemu_-2D_issues_718&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w&m=eOSxOmlj_wVOfj7HF2fJ1VXlNNJp4k8UAlT5STcoAguScPKeHYwb6hwEuY54ok5c&s=ou3KExLg2-Cx31UtMV6vXErHpesdEHJjHXnbKcE9Wdk&e=
Signed-off-by: Kshitij Suri
---
hmp-commands.hx| 11 ++--
monitor/hmp-cmds.c | 4 +-
qapi/ui.json | 7 ++-
ui/console.c | 153 -
4 files changed, 165 insertions(+), 10 deletions(-)
You're going to need to update configure here.
Currently libpng is only detected if VNC is enabled.
This proposed change means we need to detect libpng
any time the build of system emulators is enabled.
The CONFIG_VNC_PNG option will also need renaming
to CONFIG_PNG
Yes I have added a new CONFIG_PNG option in the updated patch. As for
CONFIG_VNC_PNG, can we have a seperate patch set
where I can replace CONFIG_VNC_PNG usage with combination of CONFIG_VNC
and CONFIG_PNG? The replacement strategy would be as follows
#ifdef CONFIG_VNC_PNG -> #if defined(CONFIG_VNC) && defined(CONFIG_PNG)
Along with this, can we also use the new png meson-option to denote PNG
format for VNC as well while maintaining backward compatibility? The
change would look like
vnc_png = dependency('libpng', required: get_option('vnc_png'), method:
'pkg-config', kwargs: static_kwargs)
gets changed to
vnc_png = dependency('libpng', required: get_option('vnc_png') ||
get_option('png'),
method: 'pkg-config', kwargs: static_kwargs)
+.args_type = "filename:F,device:s?,head:i?,format:f?",
+.params = "filename [device [head]] [format]",
+.help = "save screen from head 'head' of display device 'device'"
+ "in specified format 'format' as image 'filename'."
+ "Default format for screendump is PPM.",
Mention what other formats are permitted, making it clear that
the format is in fact 'png' and 'ppm', not 'PPM'
Updated in the v2 patch.
diff --git a/qapi/ui.json b/qapi/ui.json
index 9354f4c467..9fdb56b60b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -76,7 +76,7 @@
##
# @screendump:
#
-# Write a PPM of the VGA screen to a file.
+# Write a screenshot of the VGA screen to a file.
#
# @filename: the path of a new PPM file to store the image
#
@@ -87,6 +87,9 @@
#parameter is missing, head #0 will be used. Also note that the head
#can only be specified in conjunction with the device ID. (Since 2.12)
#
+# @format: image format for screendump is specified. Currently only PNG and
+# PPM are supported.
+#
# Returns: Nothing on success
#
# Since: 0.14
@@ -99,7 +102,7 @@
#
##
{ 'command': 'screendump',
- 'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+ 'data': {'filename': 'str', '*device': 'str', '*head': 'int', '*format':
'str'},
This 'format' should not be a string, it needs to be an enum type.
Modified to an enum with png and ppm types.
'coroutine': true }
##
diff --git a/ui/console.c b/ui/console.c
index 40eebb6d2c..7813b195ac 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -37,6 +37,9 @@
#include "exec/memory.h"
#include "io/channel-file.h"
#include "qom/object.h"
+#ifdef CONFIG_VNC_PNG
+#include "png.h"
+#endif
#define DEFAULT_BACKSCROLL 512
#define CONSOLE_CURSOR_PERIOD 500
@@ -289,6 +292,137 @@ void graphic_hw_invalidate(QemuConsole *con)
}
}
+#ifdef CONFIG_VNC_PNG
+/**
+ * a8r8g8b8_to_rgba: Convert a8r8g8b8 to rgba format
+ *
+ * @dst: Destination pointer.
+ * @src: Source pointer.
+ * @n_pixels: Size of image.
+ */
+static void a8r8g8b8_to_rgba(uint32_t *dst, uint32_t *src, int n_pixels)
+{
+
Re: [PATCH] Added parameter to take screenshot with screendump as PNG
On 23/02/22 4:30 pm, Dr. David Alan Gilbert wrote:
* Kshitij Suri (kshitij.s...@nutanix.com) wrote:
Currently screendump only supports PPM format, which is un-compressed and not
standard. Added an "format" parameter to qemu monitor screendump capabilites
to support PNG image capture using libpng. The param was added in QAPI schema
of screendump present in ui.json along with png_save() function which converts
pixman_image to PNG. HMP command equivalent was also modified to support the
feature.
Example usage:
{ "execute": "screendump", "arguments": { "filename": "/tmp/image",
"format":"png" } }
Resolves:
https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_qemu-2Dproject_qemu_-2D_issues_718&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w&m=YAoUy9TYXWb8lktv5hZwRWjZEBcoHKxfdrHMd8KkTnQAESANHEcW96C1ukSngzJ2&s=xJP6q9IE0req5UkOFLablyJWWREdmwS1NX4Yse0pYxE&e=
Signed-off-by: Kshitij Suri
---
hmp-commands.hx| 11 ++--
monitor/hmp-cmds.c | 4 +-
qapi/ui.json | 7 ++-
ui/console.c | 153 -
4 files changed, 165 insertions(+), 10 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 70a9136ac2..2163337f35 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -244,17 +244,18 @@ ERST
{
.name = "screendump",
-.args_type = "filename:F,device:s?,head:i?",
-.params = "filename [device [head]]",
-.help = "save screen from head 'head' of display device 'device'
"
- "into PPM image 'filename'",
+.args_type = "filename:F,device:s?,head:i?,format:f?",
+.params = "filename [device [head]] [format]",
+.help = "save screen from head 'head' of display device 'device'"
+ "in specified format 'format' as image 'filename'."
+ "Default format for screendump is PPM.",
.cmd= hmp_screendump,
.coroutine = true,
},
SRST
``screendump`` *filename*
- Save screen into PPM image *filename*.
+ Save screen as image *filename*.
ERST
{
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 2669156b28..3fb1394561 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1665,9 +1665,11 @@ hmp_screendump(Monitor *mon, const QDict *qdict)
const char *filename = qdict_get_str(qdict, "filename");
const char *id = qdict_get_try_str(qdict, "device");
int64_t head = qdict_get_try_int(qdict, "head", 0);
+const char *format = qdict_get_str(qdict, "format");
Error *err = NULL;
-qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
+qmp_screendump(filename, id != NULL, id, id != NULL, head,
+ format != NULL, format, &err);
hmp_handle_error(mon, err);
}
I think that's OK from the HMP front, see some questions below.
diff --git a/qapi/ui.json b/qapi/ui.json
index 9354f4c467..9fdb56b60b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -76,7 +76,7 @@
##
# @screendump:
#
-# Write a PPM of the VGA screen to a file.
+# Write a screenshot of the VGA screen to a file.
#
# @filename: the path of a new PPM file to store the image
#
@@ -87,6 +87,9 @@
#parameter is missing, head #0 will be used. Also note that the head
#can only be specified in conjunction with the device ID. (Since 2.12)
#
+# @format: image format for screendump is specified. Currently only PNG and
+# PPM are supported.
+#
# Returns: Nothing on success
#
# Since: 0.14
@@ -99,7 +102,7 @@
#
##
{ 'command': 'screendump',
- 'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+ 'data': {'filename': 'str', '*device': 'str', '*head': 'int', '*format':
'str'},
'coroutine': true }
##
diff --git a/ui/console.c b/ui/console.c
index 40eebb6d2c..7813b195ac 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -37,6 +37,9 @@
#include "exec/memory.h"
#include "io/channel-file.h"
#include "qom/object.h"
+#ifdef CONFIG_VNC_PNG
+#include "png.h"
+#endif
#define DEFAULT_BACKSCROLL 512
#define CONSOLE_CURSOR_PERIOD 500
@@ -289,6 +292,137 @@ void graphic_hw_invalidate(QemuConsole *con)
}
}
+#ifdef CONFIG_VNC_PNG
+/**
+ * a8r8g8b8_to_rgba: Convert a8r8g8b8 to rgba format
+ *
+ * @dst: Destination pointer.
+ * @src: Source pointer.
+ * @n_pixels: Siz
Re: [PATCH v2] Added parameter to take screenshot with screendump as PNG
On 24/02/22 9:32 pm, Eric Blake wrote:
On Thu, Feb 24, 2022 at 11:59:08AM +, 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=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w&m=qNJ3bItOdAVHt5dAMXQLkxPL-RWabpnjdw53Hqk9lMbMhTF5Z2PmCjhWiBIiyiII&s=HjICgN7yF07YFTdi0rumN8vhm3-EwLgjHTuHhZXVc5w&e=
Signed-off-by: Kshitij Suri
---
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.
+++ 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: 6.3
The next release is 7.0, not 6.3.
+#
+##
+{ '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. Currently only PNG and
+# PPM are supported.
The second sentence is no longer necessary, as the documentation for
the enum type covers that information now. Missing a '(since 7.0)' tag.
+#
# Returns: Nothing on success
#
# Since: 0.14
@@ -99,7 +117,7 @@
#
##
{ 'command': 'screendump',
- 'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+ 'data': {'filename': 'str', '*device': 'str', '*head': 'int', '*format':
'ImageFormat'},
Please wrap the long line.
Thank you for your prompt review! Will update the suggested changes in
the following patch.
Regards,
Kshitij
Re: [PATCH v2] Added parameter to take screenshot with screendump as PNG
On 24/02/22 9:48 pm, Daniel P. Berrangé wrote:
On Thu, Feb 24, 2022 at 11:59:08AM +, 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=Hu1QTP-aSF7FeXYQcdODcxg2wW1sBEpxaD4jWHYF5kxD2Z6ihhXkxRFOkovubo-f&s=YwpDKYWnYlYBM7aE1jNrISGXxP9nKm5f9Kfotxm5rZ4&e=
Signed-off-by: Kshitij Suri
---
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.
hmp-commands.hx| 11 +++---
meson.build| 13 +--
meson_options.txt | 2 +
monitor/hmp-cmds.c | 8 +++-
qapi/ui.json | 24 ++--
ui/console.c | 97 --
6 files changed, 139 insertions(+), 16 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 70a9136ac2..e43c9954b5 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*.
ERST
{
diff --git a/meson.build b/meson.build
index 8df40bfac4..fd550c01ec 100644
--- a/meson.build
+++ b/meson.build
@@ -1112,13 +1112,16 @@ 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
+vnc_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'),
+ vnc_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)
@@ -1537,9 +1540,10 @@ 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_PNG', vnc_png.found())
I think we should be removing CONFIG_VNC_PNG entirely - the VNC
code should just use CONFIG_PNG.
I'd suggest splitting ths into two patches. The first patch
updates meson.build to look for libpng unconditionally and
rename to CONFIG_PNG. The second patch introduces the PNG
support for screendump.
Yes can remove entirely with a combination of CONFIG_VNC and CONFIG_PNG
as follows
#ifdef CONFIG_VNC_PNG -> #if defined(CONFIG_VNC) && defined(CONFIG_PNG)
But
Re: [PATCH v2] Added parameter to take screenshot with screendump as PNG
On 25/02/22 2:40 pm, Daniel P. Berrangé wrote:
On Fri, Feb 25, 2022 at 11:26:20AM +0530, Kshitij Suri wrote:
On 24/02/22 9:48 pm, Daniel P. Berrangé wrote:
On Thu, Feb 24, 2022 at 11:59:08AM +, 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=Hu1QTP-aSF7FeXYQcdODcxg2wW1sBEpxaD4jWHYF5kxD2Z6ihhXkxRFOkovubo-f&s=YwpDKYWnYlYBM7aE1jNrISGXxP9nKm5f9Kfotxm5rZ4&e=
Signed-off-by: Kshitij Suri
---
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.
hmp-commands.hx| 11 +++---
meson.build| 13 +--
meson_options.txt | 2 +
monitor/hmp-cmds.c | 8 +++-
qapi/ui.json | 24 ++--
ui/console.c | 97 --
6 files changed, 139 insertions(+), 16 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 70a9136ac2..e43c9954b5 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*.
ERST
{
diff --git a/meson.build b/meson.build
index 8df40bfac4..fd550c01ec 100644
--- a/meson.build
+++ b/meson.build
@@ -1112,13 +1112,16 @@ 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
+vnc_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'),
+ vnc_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)
@@ -1537,9 +1540,10 @@ 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_PNG', vnc_png.found())
I think we should be removing CONFIG_VNC_PNG entirely - the VNC
code should just use CONFIG_PNG.
I'd suggest splitting ths into two patches. The first patch
updates meson.build to look for libpng unconditionally and
rename to CONFIG_PNG. The second patch introduces the PNG
support for screendump.
Yes can remove entirely with a combination of
[PATCH 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
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
---
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..14396e9f83 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
+#if defined(CONFIG_VNC) && defined(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
+#if defined(CONFIG_VNC) && defined(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)
[PATCH 2/2] Added parameter to take screenshot with screendump as PNG
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
---
hmp-commands.hx| 11 ++---
monitor/hmp-cmds.c | 8 +++-
qapi/ui.json | 25 ++--
ui/console.c | 100 +++--
4 files changed, 132 insertions(+), 12 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 70a9136ac2..e43c9954b5 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*.
ERST
{
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 8c384dc1b2..c300545f34 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1677,9 +1677,15 @@ 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 = IMAGE_FORMAT_PPM;
+if (input_format != NULL && strcmp(input_format, "png") == 0) {
+format = IMAGE_FORMAT_PNG;
+}
-qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
+qmp_screendump(filename, id != NULL, id, id != NULL, head,
+ input_format != NULL, format, &err);
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..6180d293e6 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 CONF
Re: [PATCH 2/2] Added parameter to take screenshot with screendump as PNG
On 26/02/22 12:37 am, Eric Blake wrote:
On Fri, Feb 25, 2022 at 11:58:30AM +, 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=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w&m=N58ni_R8AQHe3XV2MX9aeKN2mCiEkpdyiqhV71sHDUEIuIk3P1HcXc-QfepnNaIE&s=CM400Sor3Oiio6iq3V_F9jFMCrn8UdbeKKXDnIh2QRw&e=
Signed-off-by: Kshitij Suri
---
+.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*.
Maybe:
as an image *filename*, with default format of PPM.
ERST
{
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 8c384dc1b2..c300545f34 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1677,9 +1677,15 @@ 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 = IMAGE_FORMAT_PPM;
+if (input_format != NULL && strcmp(input_format, "png") == 0) {
+format = IMAGE_FORMAT_PNG;
+}
Instead of open-coding the string-to-enum translation (which will be
hard to located if we expand to a third format down the road), you
should use qapi_enum_parse(&ImageFormat_lookup, ...).
+++ 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
+#endif
#define DEFAULT_BACKSCROLL 512
#define CONSOLE_CURSOR_PERIOD 500
@@ -289,6 +292,87 @@ 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(errp, "Failed to create file from file descriptor");
error_setg_errno() might be nicer to use here, since fdopen() failure sets
errno.
+return false;
+}
+
+png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL,
+ NULL, NULL);
Indentation looks off.
+if (!png_ptr) {
+error_setg(errp, "PNG creation failed. Unable to write struct");
+free(f);
free() is the wrong function to call on FILE*. You meant fclose().
Apologies for this, will be replaced in the upcoming patch.
+return false;
+}
+
+info_ptr = png_create_info_struct(png_ptr);
+
+if (!info_ptr) {
+error_setg(errp, "PNG creation failed. Unable to write info");
+free(f);
and again
+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(pn
Re: [PATCH 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
On 26/02/22 12:42 am, Daniel P. Berrangé wrote:
On Fri, Feb 25, 2022 at 11:58:29AM +, 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
---
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/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index cebd35841a..14396e9f83 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
+#if defined(CONFIG_VNC) && defined(CONFIG_PNG)
This (and every repeated instance) can be just
#ifdef CONFIG_PNG
because the entire file is skipped by the build system
if CONFIG_VNC isn't defined.
Apologies missed that. Will remove the vnc flag conjunction in the
updated patch. Thank you!
/* 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
+#if defined(CONFIG_VNC) && defined(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
+#if defined(CONFIG_VNC) && defined(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
+#if defined(CONFIG_VNC) && defined(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
+#if defined(CONFIG_VNC) && defined(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
+#if defined(CONFIG_VNC) && defined(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
+#if defined(CONFIG_VNC) && defined(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_VNC && 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
+#if defined(CONFIG_VNC) && defined(CONFIG_PNG)
buffer_free(&vs->tight->png);
#endif
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 3ccd33dedc..1bf3790997 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
+#if defined(CONFIG_VNC) && defined(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
+#if defined(CONFIG_VNC) && defined(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..0cabcc2654 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
+#if defined(CONFIG_VNC) && defined(CONFIG_PNG)
Buffer png;
#endif
int levels[4];
--
2.22.3
Regards,
Daniel
Regards,
Kshitij
[PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG.
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
---
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&
[PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
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
---
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
[PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG.
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
---
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&
[PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
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
---
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 (tigh
Re: [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
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
Reviewed-by: Daniel P. Berrangé
---
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.
Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
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
---
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': &#
Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
On 28/03/22 3:22 pm, Daniel P. Berrangé wrote:
On Tue, Mar 22, 2022 at 10:49:53AM +, 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
---
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é
With regards,
Daniel
Thank you for your review! Will update the version and send it in the
following patch
Regards,
Kshitij Suri
[PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
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
Reviewed-by: Daniel P. Berrangé
---
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
[PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
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
Reviewed-by: Daniel P. Berrangé
---
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
+#endif
#define DEFAULT_BACKSCROLL
Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
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
[PATCH v3 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
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
Reviewed-by: Daniel P. Berrangé
---
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
[PATCH v3 2/2] Added parameter to take screenshot with screendump as PNG
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
Reviewed-by: Daniel P. Berrangé
---
diff to v2:
- HMP fixes for png
- Used enum for image format
- Fixed descriptions and updated qemu support versions
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/chan
Re: On patch series version tags, and also cover letters (was: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG)
On 29/03/22 1:29 pm, Markus Armbruster wrote: Kshitij Suri writes: 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. We all make mistakes :) The purpose of the version tag in the subject is to help humans with keeping track of patch submissions. Increment it for every submission. If you need to resend a submission completely unchanged for some reason, you may want to keep the tag and add "RESEND". A cover letter (git format-patch --cover-letter) lets you write an introduction to the whole series. Simple series may not need an introduction, but complex ones do. I always use one except when the "series" is a single patch. Keeping a change log in the cover letter helps people who already reviewed previous iterations. Check out https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2022-2D03_msg03977.html&d=DwIBAg&c=s883GpUCOChKOHiocYtGcg&r=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w&m=D-rls6IoKGB-dpNu3-R_2PSp8HMjagXZU2NC6nqi6DhlurlWsW1vY9ip-7dSaSuv&s=kBAJFSYBE5TTs27r0Ycx7U2orcQfBt6L8Uslbk_X8xY&e= for an example. Not every cover letter needs to be that verbose, of course. Likewise, the level of detail in change logs varies. A good way to get a feel for good cover letters and commit messages is to review patches. What kind of information helps you as a reviewer? That's the kind of information you want to provide with your submissions. Hope this helps! This helps out a lot! Really grateful for this value packed mail! Will keep these points in mind while further upstreaming and versioning. Thank you! Regards, Kshitij Suri
Re: On patch series version tags, and also cover letters (was: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG)
On 29/03/22 2:31 pm, Daniel P. Berrangé wrote: On Tue, Mar 29, 2022 at 09:59:55AM +0200, Markus Armbruster wrote: Kshitij Suri writes: 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. We all make mistakes :) The purpose of the version tag in the subject is to help humans with keeping track of patch submissions. Increment it for every submission. If you need to resend a submission completely unchanged for some reason, you may want to keep the tag and add "RESEND". A cover letter (git format-patch --cover-letter) lets you write an introduction to the whole series. Simple series may not need an introduction, but complex ones do. I always use one except when the "series" is a single patch. Keeping a change log in the cover letter helps people who already reviewed previous iterations. FYI, using the 'git-publish' tool instead of 'git send-email' or 'git format-patch' helps you do all these things. It automatically sets & increments the version number, it prompts for a cover letter and remembers what you wrote next time. With regards, Daniel Oh didn't know about it, will try to use git-publish from now on. Thank you for your guidance for support throughout the issue! Regards, Kshitij Suri
Re: On patch series version tags, and also cover letters (was: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG)
On 29/03/22 3:18 pm, Peter Maydell wrote: On Tue, 29 Mar 2022 at 09:03, Markus Armbruster wrote: A cover letter (git format-patch --cover-letter) lets you write an introduction to the whole series. Simple series may not need an introduction, but complex ones do. I always use one except when the "series" is a single patch. Note that a multi-patch series always needs a cover letter, even if its contents are quite brief. Some of the automatic tooling gets confused by multi-patch series with no cover letter. Conversely, single patches shouldn't have a cover letter, although getting that one wrong doesn't really have much ill effect. thanks -- PMM Oh so regarding this PNG issue which has 2 patches, one to phase out VNC_PNG and one for core logic, should I send with a cover-letter? Thank you for our advice! Regards, Kshitij Suri
[PATCH v3 0/2] Added option to take screenshot with screendump as PNG
This patch series aims to add PNG support using libpng to screendump method. Currently screendump only supports PPM format, which is uncompressed and not standardised. PATCH 1 phases out CONFIG_VNC_PNG parameter and replaces it with CONFIG_PNG which detects libpng support. PATCH 2 contains core logic for PNG creation from pixman using libpng. HMP command equivalent is also implemented in this patch. v2->v3 - HMP implementation fixes for png. - Used enum for image format. - Fixed description and updated QEMU support version. v1->v2: - 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. Kshitij Suri (2): Replacing CONFIG_VNC_PNG with CONFIG_PNG Added parameter to take screenshot with screendump as PNG hmp-commands.hx| 11 ++--- meson.build| 9 ++-- meson_options.txt | 4 +- monitor/hmp-cmds.c | 12 +- qapi/ui.json | 24 +-- ui/console.c | 101 +++-- ui/vnc-enc-tight.c | 18 ui/vnc.c | 4 +- ui/vnc.h | 2 +- 9 files changed, 154 insertions(+), 31 deletions(-) -- 2.22.3
[PATCH v3 2/2] Added parameter to take screenshot with screendump as PNG
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
Reviewed-by: Daniel P. Berrangé
---
diff to v2:
- HMP fixes for png
- Used enum for image format
- Fixed descriptions and updated qemu support versions
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/chan
[PATCH v3 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
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
Reviewed-by: Daniel P. Berrangé
---
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
[PATCH v4 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
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
Reviewed-by: Daniel P. Berrangé
---
diff to v3:
- Added condition to check for libpng only in PNG option is allowed
meson.build| 12 +++-
meson_options.txt | 4 ++--
ui/vnc-enc-tight.c | 18 +-
ui/vnc.c | 4 ++--
ui/vnc.h | 2 +-
5 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/meson.build b/meson.build
index 282e7c4650..0790ccef99 100644
--- a/meson.build
+++ b/meson.build
@@ -1115,14 +1115,16 @@ if gtkx11.found()
x11 = dependency('x11', method: 'pkg-config', required: gtkx11.found(),
kwargs: static_kwargs)
endif
-vnc = not_found
png = not_found
+if get_option('png').allowed() and have_system
+ png = dependency('libpng', required: get_option('png'),
+method: 'pkg-config', kwargs: static_kwargs)
+endif
+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'],
@@ -1554,9 +1556,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 +3640,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_c
[PATCH v4 2/2] Added parameter to take screenshot with screendump as PNG
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
Reviewed-by: Daniel P. Berrangé
---
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
+#endif
#define DEFAULT_BACKSCROLL
[PATCH v4 0/2] Option to take screenshot with screendump as PNG
This patch series aims to add PNG support using libpng to screendump method. Currently screendump only supports PPM format, which is uncompressed and not standardised. PATCH 1 phases out CONFIG_VNC_PNG parameter and replaces it with CONFIG_PNG which detects libpng support. PATCH 2 contains core logic for PNG creation from pixman using libpng. HMP command equivalent is also implemented in this patch. v3->v4 - Added condition to check for libpng only in PNG option is allowed v2->v3 - HMP implementation fixes for png. - Used enum for image format. - Fixed description and updated QEMU support version. v1->v2: - 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. Kshitij Suri (2): Replacing CONFIG_VNC_PNG with CONFIG_PNG Added parameter to take screenshot with screendump as PNG hmp-commands.hx| 11 ++--- meson.build| 12 +++--- meson_options.txt | 4 +- monitor/hmp-cmds.c | 12 +- qapi/ui.json | 24 +-- ui/console.c | 101 +++-- ui/vnc-enc-tight.c | 18 ui/vnc.c | 4 +- ui/vnc.h | 2 +- 9 files changed, 157 insertions(+), 31 deletions(-) -- 2.22.3
Re: [PATCH v3 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
On 30/03/22 2:39 am, Paolo Bonzini wrote:
On 3/29/22 15:42, 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
Reviewed-by: Daniel P. Berrangé
---
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)
Please use
png = not_found
if get_option('png').allowed() and have_system
png = dependency('libpng', required: get_option('png'),
method: 'pkg-config', kwargs: static_kwargs)
endif
as suggested in the review of v2.
Paolo
Apologies, I had missed that. Thank you for your review! i have added
this in the v4 version of patch
Regards,
Kshitij Suri
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 CON
Re: [PATCH v4 2/2] Added parameter to take screenshot with screendump as PNG
On 01/04/22 4:50 pm, Markus Armbruster wrote:
Dave, please have a look at the HMP compatibility issue in
hmp-command.hx below.
Kshitij Suri writes:
Currently screendump only supports PPM format, which is un-compressed and not
standard.
If "standard" means "have to pay a standards organization $$$ to access
the spec", PPM is not standard. If it means "widely supported", it
certainly is. I'd drop "and not standard". Suggestion, not demand.
Makes sense. Will modify it in the updated patch.
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.
Suggest to use imperative mood to describe the commit, and omit details
that aren't necessary here:
Add a "format" parameter to QMP and HMP screendump command
to support PNG image capture using libpng.
Yes, will reduce the verbosity of the commit message.
Example usage:
{ "execute": "screendump", "arguments": { "filename": "/tmp/image",
"format":"png" } }
Providing an example in the commit message is always nice, thanks!
Thank you!
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=oODILSxODcEhktuPJ-SfVt-MW867cpF_TvDe-WJyNRXx84FinSifhtp6-Racosb0&s=89nTa5MLAr16WtPfrm4aYkwWlPuRs6yuaD22dZTE_pM&e=
Signed-off-by: Kshitij Suri
Reviewed-by: Daniel P. Berrangé
---
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?",
Incompatible change: meaning of "screendump ONE TWO" changes from
filename=ONE, device=TWO to filename=ONE, format=TWO.
As HMP is not a stable interface, incompatible change is permissible.
But is this one wise?
Could we add the new argument at the end instead?
.args_type = "filename:F,device:s?,head:i?,format:s?",
Could we do *without* an argument, and derive the format from the
filename extension? .png means format=png, anything else format=ppm.
Would be a bad idea for QMP. Okay for HMP?
Should I go ahead with extracting format from filename provided for HMP?
+.params = "filename [format] [device [head]]",
This tells us that parameter format can be omitted like so
screendump foo.ppm device-id
which isn't true. Better: "filename [format [device [head]]".
Thank you will modify it!
+.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
[PATCH v5 0/2] Option to take screenshot with screendump as PNG
This patch series aims to add PNG support using libpng to screendump method. Currently screendump only supports PPM format, which is uncompressed. PATCH 1 phases out CONFIG_VNC_PNG parameter and replaces it with CONFIG_PNG which detects libpng support. PATCH 2 contains core logic for PNG creation from pixman using libpng. HMP command equivalent is also implemented in this patch. v4->v5 - Modified format as a flag based optional parameter in HMP. v3->v4 - Added condition to check for libpng only in PNG option is allowed v2->v3 - HMP implementation fixes for png. - Used enum for image format. - Fixed description and updated QEMU support version. v1->v2: - 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. Kshitij Suri (2): Replacing CONFIG_VNC_PNG with CONFIG_PNG Added parameter to take screenshot with screendump as PNG hmp-commands.hx| 11 ++--- meson.build| 12 +++--- meson_options.txt | 4 +- monitor/hmp-cmds.c | 12 +- qapi/ui.json | 24 +-- ui/console.c | 101 +++-- ui/vnc-enc-tight.c | 18 ui/vnc.c | 4 +- ui/vnc.h | 2 +- 9 files changed, 157 insertions(+), 31 deletions(-) -- 2.22.3
[PATCH v5 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
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
Reviewed-by: Daniel P. Berrangé
---
meson.build| 12 +++-
meson_options.txt | 4 ++--
ui/vnc-enc-tight.c | 18 +-
ui/vnc.c | 4 ++--
ui/vnc.h | 2 +-
5 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/meson.build b/meson.build
index 282e7c4650..0790ccef99 100644
--- a/meson.build
+++ b/meson.build
@@ -1115,14 +1115,16 @@ if gtkx11.found()
x11 = dependency('x11', method: 'pkg-config', required: gtkx11.found(),
kwargs: static_kwargs)
endif
-vnc = not_found
png = not_found
+if get_option('png').allowed() and have_system
+ png = dependency('libpng', required: get_option('png'),
+method: 'pkg-config', kwargs: static_kwargs)
+endif
+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'],
@@ -1554,9 +1556,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 +3640,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,
[PATCH v5 2/2] Added parameter to take screenshot with screendump as PNG
Currently screendump only supports PPM format, which is un-compressed. Added
a "format" parameter to QMP and HMP screendump command to support PNG image
capture using libpng.
QMP example usage:
{ "execute": "screendump", "arguments": { "filename": "/tmp/image",
"format":"png" } }
HMP example usage:
screendump /tmp/image -f png
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/718
Signed-off-by: Kshitij Suri
Reviewed-by: Daniel P. Berrangé
---
diff to v4:
- Modified format to be an optional flag based parameter in HMP.
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..808020d005 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -244,11 +244,12 @@ ERST
{
.name = "screendump",
-.args_type = "filename:F,device:s?,head:i?",
-.params = "filename [device [head]]",
-.help = "save screen from head 'head' of display device 'device'
"
- "into PPM image 'filename'",
-.cmd= hmp_screendump,
+.args_type = "filename:F,format:-fs,device:s?,head:i?",
+.params = "filename [-f format] [device [head]]",
+.help = "save screen from head 'head' of display device 'device'"
+ "in specified format 'format' as image 'filename'."
+ "Currently only 'png' and 'ppm' formats are supported.",
+ .cmd= hmp_screendump,
.coroutine = true,
},
diff --git a/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..98f0126999 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -157,12 +157,27 @@
##
{ 'command': 'expire_password', 'boxed': true, 'data': 'ExpirePasswordOptions'
}
+##
+# @ImageFormat:
+#
+# Supported image format types.
+#
+# @png: PNG format
+#
+# @ppm: PPM format
+#
+# Since: 7.1
+#
+##
+{ 'enum': 'ImageFormat',
+ 'data': ['ppm', 'png'] }
+
##
# @screendump:
#
-# Write a PPM of the VGA screen to a file.
+# Capture the contents of a screen and write it to a file.
#
-# @filename: the path of a new PPM file to store the image
+# @filename: the path of a new file to store the image
#
# @device: ID of the display device that should be dumped. If this parameter
# is missing, the primary display will be used. (Since 2.12)
@@ -171,6 +186,8 @@
#parameter is missing, head #0 will be used. Also note that the head
#can only be specified in conjunction with the device ID. (Since 2.12)
#
+# @format: image format for screendump. (default: ppm) (Since 7.1)
+#
# Returns: Nothing on success
#
# Since: 0.14
@@ -183,7 +200,8 @@
#
##
{ 'command': 'screendump',
- 'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+ 'data': {'filename': 'str', '*device': 'str', '*head': 'int',
+ '*format': 'ImageFormat'},
'coroutine': true }
##
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
+#endif
#define DEFAULT_BACKSCROLL 512
#define CONSOLE_CURSOR_PERIOD 500
@@ -291,6 +294,89 @@ void graphic_hw_invalidate(QemuConsole
Re: [PATCH v5 0/2] Option to take screenshot with screendump as PNG
Hi, Hope this mail finds everyone well! I have updated the code as required and would be grateful if I could get your reviews for any changes that are needed to be implemented in the patch. In case no change is required, please do let me know the next steps for the same. Regards, Kshitij Suri On 08/04/22 12:43 pm, Kshitij Suri wrote: This patch series aims to add PNG support using libpng to screendump method. Currently screendump only supports PPM format, which is uncompressed. PATCH 1 phases out CONFIG_VNC_PNG parameter and replaces it with CONFIG_PNG which detects libpng support. PATCH 2 contains core logic for PNG creation from pixman using libpng. HMP command equivalent is also implemented in this patch. v4->v5 - Modified format as a flag based optional parameter in HMP. v3->v4 - Added condition to check for libpng only in PNG option is allowed v2->v3 - HMP implementation fixes for png. - Used enum for image format. - Fixed description and updated QEMU support version. v1->v2: - 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. Kshitij Suri (2): Replacing CONFIG_VNC_PNG with CONFIG_PNG Added parameter to take screenshot with screendump as PNG hmp-commands.hx| 11 ++--- meson.build| 12 +++--- meson_options.txt | 4 +- monitor/hmp-cmds.c | 12 +- qapi/ui.json | 24 +-- ui/console.c | 101 +++-- ui/vnc-enc-tight.c | 18 ui/vnc.c | 4 +- ui/vnc.h | 2 +- 9 files changed, 157 insertions(+), 31 deletions(-)
Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG.
On 11/03/22 5:50 pm, Markus Armbruster wrote:
Kshitij Suri 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
---
[...]
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
[PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
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
---
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 (tigh
[PATCH v2 2/2] Added parameter to take screenshot with screendump as
From: "kshitij.suri"
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
---
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 | 24 +--
ui/console.c | 102 +++--
4 files changed, 144 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..a6d994ba2c 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -73,12 +73,27 @@
##
{ 'command': 'expire_password', 'data': {'protocol': 'str', 'time': 'str'} }
+##
+# @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.
+# 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,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
@@ -99,7 +116,8 @@
#
##
{ 'command': 'screendump',
- 'data': {'filename': 'str', '*device': 'str', '*head': 'int'},
+ 'data': {'filename': &
Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG.
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 writes:
On 11/03/22 5:50 pm, Markus Armbruster wrote:
Kshitij Suri 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
[...]
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
Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as
On 16/03/22 6:55 pm, Markus Armbruster wrote:
Kshitij Suri writes:
From: "kshitij.suri"
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=UZZDywEeidD1LndEhKddMf_0v-jePIgMYErGImjYyvjYRJFdnv6LAHgRmZ0IpvIL&s=jc09kdvD1ULKCC9RgwWcsK6eweue3ZkyD8F9kCx5yUs&e=
Signed-off-by: Kshitij Suri
---
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 | 24 +--
ui/console.c | 102 +++--
4 files changed, 144 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);
The second id != NULL looks wrong. Shouldn't it be head != NULL?
Not your patch's fault, of course.
As per hmp-commands.hx input is of format [device [head]] so maybe
works as a pair. Is it alright if I investigate and send in another patch
after this?
+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);
}
This isn't quite right.
qapi_enum_parse() is awkward to use.
If its second argument @input_format is null, it returns its third
argument -1.
Else, it @input_format is a valid enum string, it returns the
enumeration value, i.e. either IMAGE_FORMAT_PPM or IMAGE_FORMAT_PNG.
Else, it sets an error and returns its third argument -1.
If @input_format is null, I suspect your code jumps to
hmp_handle_error() with a null @err. Silently does nothing.
It should default to IMAGE_FORMAT_PPM instead.
I think you want something like this (completely untested):
val = qapi_enum_parse(&ImageFormat_lookup, input_format,
IMAGE_FORMAT_PPM, &err);
if (err) {
goto end;
}
qmp_screendump(filename, id != NULL, id, head != NULL, head,
input_forma
[PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
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
---
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 (tigh
[PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
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
---
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',
+ '
Re: [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
On 22/03/22 3:12 pm, Daniel P. Berrangé wrote: On Tue, Mar 22, 2022 at 08:18:44AM +, 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 --- 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(-) Reviewed-by: Daniel P. Berrangé With regards, Daniel Thank you for this! Regards, Kshitij Suri
Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
On 22/03/22 3:17 pm, Daniel P. Berrangé wrote:
On Tue, Mar 22, 2022 at 08:18:45AM +, 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
---
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
Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
On 22/03/22 3:45 pm, Markus Armbruster wrote:
Daniel P. Berrangé writes:
On Tue, Mar 22, 2022 at 08:18:45AM +, 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
---
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
[PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
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
Reviewed-by: Daniel P. Berrangé
---
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
[PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG
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
---
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/u
Re: [PATCH v2 1/2] Replacing CONFIG_VNC_PNG with CONFIG_PNG
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:
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
---
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
Re: [PATCH v2 2/2] Added parameter to take screenshot with screendump as PNG.
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
---
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
Re: [PULL 2/9] Replacing CONFIG_VNC_PNG with CONFIG_PNG
Hi, Apologies for missing the references. Should I send an updated patch for the same? Regards, Kshitij Suri On 26/04/22 11:05 pm, Richard Henderson wrote: y 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 Reviewed-by: Daniel P. Berrangé Message-Id: <20220408071336.99839-2-kshitij.s...@nutanix.com> [ kraxel: add meson-buildoptions.sh updates ] [ kraxel: fix centos8 testcase ] Signed-off-by: Gerd Hoffmann --- meson_options.txt | 4 ++-- ui/vnc.h | 2 +- ui/vnc-enc-tight.c | 18 +- ui/vnc.c | 4 ++-- meson.build | 12 +++- .../ci/org.centos/stream/8/x86_64/configure | 2 +- scripts/meson-buildoptions.sh | 6 +++--- 7 files changed, 25 insertions(+), 23 deletions(-)
[PATCH v1] Make png parameter to ui/meson.build independent of vnc
Currently png support is dependent on vnc for linking object file to
libpng. This commit makes the parameter independent of vnc as it breaks
system emulator with --disable-vnc unless --disable-png is added.
Fixes: 9a0a119a382867dc9a5c2ae9348003bf79d84af2
Signed-off-by: Kshitij Suri
---
ui/meson.build | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/ui/meson.build b/ui/meson.build
index eba93b41e3..df65dbd0e0 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -40,11 +40,15 @@ vnc_ss.add(files(
'vnc-jobs.c',
'vnc-clipboard.c',
))
-vnc_ss.add(zlib, png, jpeg, gnutls)
+vnc_ss.add(zlib, jpeg, gnutls)
vnc_ss.add(when: sasl, if_true: files('vnc-auth-sasl.c'))
softmmu_ss.add_all(when: vnc, if_true: vnc_ss)
softmmu_ss.add(when: vnc, if_false: files('vnc-stubs.c'))
+if png.found()
+ softmmu_ss.add(png)
+endif
+
ui_modules = {}
if curses.found()
--
2.22.3
Re: [PULL 3/9] Added parameter to take screenshot with screendump as PNG
Hi,
I have sent the fix out at
https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg01980.html.
Would be grateful for your review.
Regards,
Kshitij Suri
On 06/05/22 7:07 pm, Peter Maydell wrote:
On Wed, 27 Apr 2022 at 18:33, Gerd Hoffmann wrote:
From: Kshitij Suri
Currently screendump only supports PPM format, which is un-compressed. Added
a "format" parameter to QMP and HMP screendump command to support PNG image
capture using libpng.
QMP example usage:
{ "execute": "screendump", "arguments": { "filename": "/tmp/image",
"format":"png" } }
HMP example usage:
screendump /tmp/image -f png
Resolves:
https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_qemu-2Dproject_qemu_-2D_issues_718&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=utjv19Ej9Fb0TB7_DX0o3faQ-OAm2ypPniPyqVSoj_w&m=0pq2lv0SrogWXtqObS7lzBUp-Bbw4WYpJvheEOrvaMJrptl2ORkHDcDVKGuyG95V&s=T8YakRJNie_5nF8LIdJvsiSogf9iS7Ytlc205JIcaYQ&e=
Signed-off-by: Kshitij Suri
Reviewed-by: Daniel P. Berrangé
Acked-by: Markus Armbruster
Acked-by: Dr. David Alan Gilbert
Message-Id: <20220408071336.99839-3-kshitij.s...@nutanix.com>
Signed-off-by: Gerd Hoffmann
This change seems to have broken building the system emulator
with --disable-vnc unless I also add --disable-png. Specifically:
diff --git a/ui/console.c b/ui/console.c
index 1752f2ec8897..15d0f6affd4c 100644
--- a/ui/console.c
+++ b/ui/console.c
+png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL,
+ NULL, NULL);
ui/console.c now makes calls to functions from libpng, but
the commit didn't do anything in ui/meson.build to tell
the build system that this object file must now be linked
with libpng. Because vnc *does* correctly state its dependency
on libpng, builds that happen to include vnc will link OK
by accident, but builds without vnc will fail:
$ mkdir build/tst
$ (cd build/tst && ../../configure --target-list=i386-softmmu
--disable-tools --disable-vnc --enable-debug)
$ make -C build/tst -j8
[...]
/usr/bin/ld: libcommon.fa.p/ui_console.c.o: undefined reference to
symbol 'png_init_io@@PNG16_0'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libpng16.so.16: error adding
symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
thanks
-- PMM
Re: [PATCH v1] Make png parameter to ui/meson.build independent of vnc
Thank you! Grateful for it. Regards, Kshitij Suri On 11/05/22 1:16 pm, Paolo Bonzini wrote: +if png.found() + softmmu_ss.add(png) +endif Queued, thanks. However, this can be just "softmmu_ss.add(png) without the if, so I adjusted that. Paolo
