[Spice-devel] [spice v10 06/27] server: Let the administrator pick the video encoder and codec

[Christophe Fergeau]

On Fri, Mar 04, 2016 at 12:04:45AM +0100, Francois Gouget wrote:
> On Thu, 3 Mar 2016, Christophe Fergeau wrote:
> [...]
> > > +void display_channel_set_video_codecs(DisplayChannel *display, GArray *video_codecs)
> > > +{
> > > +    spice_return_if_fail(display);
> > > +
> > > +    g_array_set_size(display->video_codecs, 0);
> > > +    g_array_insert_vals(display->video_codecs, 0, video_codecs->data, video_codecs->len);
> > > +}
> > 
> > I would do something like
> > g_array_unref(display->video_codecs);
> > display->video_codecs = g_array_ref(video_codecs);
> 
> This implies treating the video_codecs GArray as an immutable object. 
> The object we pass through the dispatch message is the one 
> reds_set_video_codecs() modifies so this should be changed there too. 
> That could work. Is there a way to indicate we don't want this GArray to 
> be modified?

Ah, I forgot to mention it in the previous email, but indeed, this needs to be changed
both here and in reds_set_video_codecs() at the same time. I don't think
GArray can be marked as immutable to avoid mistakes. If you prefer
to keep explicit copies so that it's clear that the data is not going to
be modified through another pointer, that's fine with me.
> 
> 
> [...]
> > > +SPICE_GNUC_VISIBLE int spice_server_set_video_codecs(SpiceServer *s, const char *video_codecs)
> [...]
> > Regarding the public API this can be tweaked later, but I'm not sure at
> > all this is going to be enough. I suspect when integrating this into
> > QEMU, libvirt will need to know whether gstreamer is supported, and
> > maybe the codecs which are available. If so, this would mean we would
> > get a nicer API than this string based one which coud be used instead.
> > 
> > I haven't done much research on this yet though, so maybe this is
> > enough, will need to check :)
> 
> Yes, they may prefer to split this up at the XML level. Do you know of 
> settings where libvirt queries whether certain values are supported or 
> not?

In general, QEMU is querying what a given binary supports, it's doing it
for disable copy and paste support, Marc-André recently sent patches to
detect the GL parameters, ... This was briefly discussed in
https://lists.freedesktop.org/archives/spice-devel/2015-January/018692.html
and the few mails after this one.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160304/5e373632/attachment.sig>