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.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
