From: Kevin Pouget <kpouget@redhat.com>
To: Frediano Ziglio <fziglio@redhat.com>
Cc: spice-devel@lists.freedesktop.org, qemu-devel@nongnu.org,
Marc-Andre Lureau <marcandre.lureau@redhat.com>
Subject: Re: [Qemu-devel] [RFC] spice-core: allow setting properties from QMP
Date: Fri, 21 Jun 2019 09:56:17 +0200 [thread overview]
Message-ID: <CADJ1XR3pq78_OCkNkGYnfUJWL+Tet72PZmGOr4gHzMGcDnRvFA@mail.gmail.com> (raw)
In-Reply-To: <2139720774.23871724.1561101408712.JavaMail.zimbra@redhat.com>
On Fri, Jun 21, 2019 at 9:16 AM Frediano Ziglio <fziglio@redhat.com> wrote:
>
> >
> > Hello Eric,
> >
> > > A new command may be okay, however,
> >
> > thanks, I've fix the typos and updated the patch to use an Enum, which
> > indeed makes more sense.
> >
> > I've also updated "spice-query" command to provide the current value
> > of the "video-codec" property,
> > but it made me wonder if I should improve this QMP interface with a
> > json list, or keep the current string-based list
> > ("enc1:codec1;enc2:codec2").
> >
> > I CC the spice-devel list to get their point of view
> >
> > The current behavior is:
> >
> > --> { "execute": "set-spice", "arguments": { "property":
> > "video-codecs", "value": "spice:mjpeg;gstreamer:h264" } }
> > <-- {"return":{},"id":"libvirt-23"}
>
> It looks complicated from the user. Why not just
>
> --> { "execute": "set-spice", "arguments": { "video-codecs": "spice:mjpeg;gstreamer:h264" } }
it makes sense indeed, I've updated the code:
# -> { "execute": "set-spice", "arguments": { "video-codecs": "spice:mjpeg;" }
# <- { "returns": {} }
+{ 'command': 'set-spice',
+ 'data': {'*video-codecs': 'str'},
+ 'if': 'defined(CONFIG_SPICE)' }
---
qapi/ui.json | 27 +++++++++++++++++++++++++--
ui/spice-core.c | 17 +++++++++++++++++
2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 59e412139a..cdbe04bda0 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -211,12 +211,16 @@
#
# @channels: a list of @SpiceChannel for each active spice channel
#
+# @video-codecs: The list of encoders:codecs currently allowed for
+# video streaming (since: ...)
+#
# Since: 0.14.0
##
{ 'struct': 'SpiceInfo',
'data': {'enabled': 'bool', 'migrated': 'bool', '*host': 'str',
'*port': 'int',
'*tls-port': 'int', '*auth': 'str', '*compiled-version': 'str',
- 'mouse-mode': 'SpiceQueryMouseMode', '*channels': ['SpiceChannel']},
+ 'mouse-mode': 'SpiceQueryMouseMode', '*channels': ['SpiceChannel'],
+ 'video-codecs': 'str'},
'if': 'defined(CONFIG_SPICE)' }
##
@@ -257,7 +261,8 @@
# "tls": false
# },
# [ ... more channels follow ... ]
-# ]
+# ],
+# "video-codecs": "spice:mjpeg;gstreamer:h264;"
# }
# }
#
@@ -265,6 +270,24 @@
{ 'command': 'query-spice', 'returns': 'SpiceInfo',
'if': 'defined(CONFIG_SPICE)' }
+##
+# @set-spice:
+#
+# Set Spice properties.
+# @video-codecs: the ;-separated list of video-codecs allowed for
+# spice-server video streaming.
+#
+# Since: ...
+#
+# Example:
+#
+# -> { "execute": "set-spice", "arguments": { "video-codecs": "spice:mjpeg;" }
+# <- { "returns": {} }
+##
+{ 'command': 'set-spice',
+ 'data': {'*video-codecs': 'str'},
+ 'if': 'defined(CONFIG_SPICE)' }
+
##
# @SPICE_CONNECTED:
#
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 63e8694df8..a4b265b663 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -506,6 +506,21 @@ static QemuOptsList qemu_spice_opts = {
},
};
+void qmp_set_spice(bool has_video_codecs, const char *video_codecs,
+ Error **errp)
+{
+ if (has_video_codecs) {
+ int invalid_codecs = spice_server_set_video_codecs(spice_server,
+ video_codecs);
+
+ if (invalid_codecs) {
+ error_setg(errp, "Found %d invalid video-codecs while setting"
+ " spice property 'video-codec=%s'", invalid_codecs,
+ video_codecs);
+ }
+ }
+}
+
SpiceInfo *qmp_query_spice(Error **errp)
{
QemuOpts *opts = QTAILQ_FIRST(&qemu_spice_opts.head);
@@ -555,6 +570,8 @@ SpiceInfo *qmp_query_spice(Error **errp)
SPICE_QUERY_MOUSE_MODE_SERVER :
SPICE_QUERY_MOUSE_MODE_CLIENT;
+ info->video_codecs = spice_server_get_video_codecs(spice_server);
+
/* for compatibility with the original command */
info->has_channels = true;
info->channels = qmp_query_spice_channels();
--
2.21.0
prev parent reply other threads:[~2019-06-21 7:59 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-19 12:30 [Qemu-devel] [RFC] spice-core: allow setting properties from QMP Kevin Pouget
2019-06-19 12:39 ` no-reply
2019-06-19 15:19 ` Eric Blake
2019-06-20 11:54 ` Kevin Pouget
2019-06-21 7:16 ` Frediano Ziglio
2019-06-21 7:56 ` Kevin Pouget [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CADJ1XR3pq78_OCkNkGYnfUJWL+Tet72PZmGOr4gHzMGcDnRvFA@mail.gmail.com \
--to=kpouget@redhat.com \
--cc=fziglio@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=spice-devel@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).