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

[Francois Gouget]

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?


[...]
> > +static const EnumNames video_codec_names[] = {
> > +    {SPICE_VIDEO_CODEC_TYPE_MJPEG, "mjpeg"},
> > +    {SPICE_VIDEO_CODEC_TYPE_VP8, "vp8"},
> > +    {0, NULL},
> > +};
> > +
> > +static int video_codec_caps[] = {
> > +    SPICE_DISPLAY_CAP_CODEC_MJPEG,
> > +    SPICE_DISPLAY_CAP_CODEC_VP8,
> > +};
> 
> 2 VP8 references which belong to a later commit

Done.


[...]
> > +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?


-- 
Francois Gouget <fgouget at codeweavers.com>