qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Kshitij Suri
Subject: Re: [PATCH v2] Added parameter to take screenshot with screendump as PNG
Date: Fri, 25 Feb 2022 15:05:24 +0530
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.1


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 +0000, Kshitij Suri wrote:
Currently screendump only supports PPM format, which is un-compressed and not
standard. Added a "format" parameter to qemu monitor screendump capabilites
to support PNG image capture using libpng. The param was added in QAPI schema
of screendump present in ui.json along with png_save() function which converts
pixman_image to PNG. HMP command equivalent was also modified to support the
feature.

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

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

Signed-off-by: Kshitij Suri<kshitij.suri@nutanix.com>
---
diff to v1:
    - Removed repeated alpha conversion operation.
    - Modified logic to mirror png conversion in vnc-enc-tight.c file.
    - Added a new CONFIG_PNG parameter for libpng support.
    - Changed input format to enum instead of string.

   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 won't removing the vnc_png option cause problems in the build scripts of
users with the current
version? Instead, can we use the new png meson-option to denote PNG format
for VNC as well while maintaining backward compatibility? The change would
look like
Configure arguments / meson options are not a stable interface
from QEMU. We can change them at any time.
Okay thank you for the clarification! Will send an updated two patch series ASAP.

Regards,
Daniel


Regards,

Kshitij




reply via email to

[Prev in Thread] Current Thread [Next in Thread]