Hi
On Fri, Jun 6, 2025 at 11:16 AM Daniel P. Berrangé <berra...@redhat.com>
wrote:
> On Fri, Jun 06, 2025 at 06:10:31AM +0000, Kasireddy, Vivek wrote:
> > Hi Daniel,
> >
> > > Subject: Re: [PATCH v5 2/7] ui/spice: Add an option for users to
> provide a
> > > preferred codec
> > >
> > > On Wed, May 28, 2025 at 10:11:13PM -0700, Vivek Kasireddy wrote:
> > > > Giving users an option to choose a particular codec will enable
> > > > them to make an appropriate decision based on their hardware and
> > > > use-case.
> > > >
> > > > Cc: Gerd Hoffmann <kra...@redhat.com>
> > > > Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
> > > > Cc: Dmitry Osipenko <dmitry.osipe...@collabora.com>
> > > > Cc: Frediano Ziglio <fredd...@gmail.com>
> > > > Cc: Dongwon Kim <dongwon....@intel.com>
> > > > Cc: Michael Scherle <michael.sche...@rz.uni-freiburg.de>
> > > > Signed-off-by: Vivek Kasireddy <vivek.kasire...@intel.com>
> > > > ---
> > > > qemu-options.hx | 5 +++++
> > > > ui/spice-core.c | 12 ++++++++++++
> > > > 2 files changed, 17 insertions(+)
> > > >
> > > > diff --git a/qemu-options.hx b/qemu-options.hx
> > > > index 7eb8e02b4b..fcddb583c9 100644
> > > > --- a/qemu-options.hx
> > > > +++ b/qemu-options.hx
> > > > @@ -2281,6 +2281,7 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice,
> > > > "
> [,streaming-video=[off|all|filter]][,disable-copy-paste=on|off]\n"
> > > > "
> [,disable-agent-file-xfer=on|off][,agent-mouse=[on|off]]\n"
> > > > " [,playback-compression=[on|off]][,seamless-
> > > migration=[on|off]]\n"
> > > > + " [,video-codecs=<encoder>:<codec>\n"
> > > > " [,gl=[on|off]][,rendernode=<file>]\n"
> > > > " enable spice\n"
> > > > " at least one of {port, tls-port} is
> mandatory\n",
> > > > @@ -2369,6 +2370,10 @@ SRST
> > > > ``seamless-migration=[on|off]``
> > > > Enable/disable spice seamless migration. Default is off.
> > > >
> > > > + ``video-codecs=<encoder>:<codec>``
> > > > + Provide the preferred codec the Spice server should use.
> > > > + Default would be spice:mjpeg.
> > >
> > > This looks like two distinct settings overloaded into one command
> > > line parameter, which is a design anti-pattern.
> > >
> > > Why can't this be done as separate parameters
> > The Spice server API used by Qemu (spice_server_set_video_codecs)
> > to set the preferred codec requires the video-codecs string to be in
> > encoder:codec format. AFAIK, there is no other option or API to set
> > the encoder and codec values separately.
>
> QEMU can accept the separate parameters and format them into the string
> format that the spice API requires so our public API is not impacted
> by this spice design choice.
>
>
Apparently you cannot mix and match freely, it has a rather fixed set of
actually working values.
See here and related code:
https://gitlab.freedesktop.org/spice/spice/-/blob/master/server/reds.cpp?ref_type=heads#L3468
Tbh, I don't think the encoder matters much, and I don't know why it was
decided to associate it with video codec names.
Maybe the spice API should provide a simpler form: accept only codec names.
In the meantime, qemu should perhaps add the "working" encoder prefixes
(spice: for mjpeg, gstreamer: for others) itself and not expose any extra
option to the user?
