Hi Victor,
On Thu, 2017-01-19 at 08:46 +0100, Victor Toso wrote:
> Hi,
>
> On Wed, Jan 18, 2017 at 11:27:40AM +0100, Pavel Grunt wrote:
> > Hi Victor,
> >
> > On Fri, 2017-01-06 at 09:18 +0100, Victor Toso wrote:
> > > From: Victor Toso <me@xxxxxxxxxxxxxx>
> > >
> > > * SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE
> > >
> > > This message was introduced in protocol 0.12.13 to establish
> > > client
> > > side preference on video codec to be used in streams.
> > >
> > > At this moment, we only introduce a new API [0] to select *the*
> > > preferred video codec for client; In a later time, it should be
> > > possible to use this message upon connection in order to to give
> > > higher priority for video codecs with better performance and
> > > with
> > > hardware decoding capabilities.
> > >
> > > [0] spice_display_change_preferred_video_codec_type()
> >
> > Why not support more than one codec in the api? It could accept a
> > string which would be translated to the message. iow the message
> > allows to handle more than one codec, the api should do the same.
>
> I did it this way for two reasons:
>
> 1-) spice-gtk will get smarter about which video-codecs have hw
> decoding. Overall, that means that the API
> spice_display_change_preferred_video_codec_type() will be used
> mostly for testing.
client will use SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE to tell
the info to the server, no ?
>
> 2-) spice-server is the one that really controls the streaming.
Not sure about it :) A client can easily force server to stream in one
format by setting channel caps (set just SPICE_DISPLAY_CAP_CODEC_H264
and the server will stream only using h264);
> When the
> priority patch gets in it would be very likely that, in near
> future,
> we would give high priority for video-codecs with hw encoding -
> this
> makes the client-side options only matter if it is aligned with
> spice-server preferences.
>
> TL;DR - I would say this API is mostly for testing and we should try
> harder to detect hw encoding capabilities in spice-server and hw
> decoding capabilities in spice-gtk.
How would you inform the server about hw accel ? By calling this api
instead of implementing another function sending the same message.
>
> If applications really need it, we can extend in the future.
function like
_change_preferred_video_codec_types(SpiceChannel *channel,
GArray *codec_types)
is basically the same and you would avoid creating a new function for
sending the message (for the smart detection you mentioned in 1)
Pavel
>
> toso
>
> >
> > Pavel
> > >
> > > Note that host preference for encoding is expected to be
> > > considered
> > > first then the client's preference.
> > >
> > > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> > > ---
> > > configure.ac | 2 +-
> > > doc/reference/spice-gtk-sections.txt | 1 +
> > > src/channel-display.c | 37
> > > ++++++++++++++++++++++++++++++++++++
> > > src/channel-display.h | 1 +
> > > src/map-file | 1 +
> > > src/spice-glib-sym-file | 1 +
> > > 6 files changed, 42 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/configure.ac b/configure.ac
> > > index f3e7f8d..aa60e91 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -69,7 +69,7 @@ AC_CHECK_LIBM
> > > AC_SUBST(LIBM)
> > >
> > > AC_CONFIG_SUBDIRS([spice-common])
> > > -PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >=
> > > 0.12.12])
> > > +PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >=
> > > 0.12.13])
> > >
> > > COMMON_CFLAGS='-I${top_builddir}/spice-common/
> > > -I${top_srcdir}/spice-common/ ${SPICE_PROTOCOL_CFLAGS}'
> > > AC_SUBST(COMMON_CFLAGS)
> > > diff --git a/doc/reference/spice-gtk-sections.txt
> > > b/doc/reference/spice-gtk-sections.txt
> > > index 386e737..4e090f6 100644
> > > --- a/doc/reference/spice-gtk-sections.txt
> > > +++ b/doc/reference/spice-gtk-sections.txt
> > > @@ -168,6 +168,7 @@ spice_display_get_gl_scanout
> > > spice_display_gl_draw_done
> > > spice_display_get_primary
> > > spice_display_change_preferred_compression
> > > +spice_display_change_preferred_video_codec_type
> > > spice_gl_scanout_free
> > > <SUBSECTION Standard>
> > > SPICE_DISPLAY_CHANNEL
> > > diff --git a/src/channel-display.c b/src/channel-display.c
> > > index ca56cd1..1d71fd3 100644
> > > --- a/src/channel-display.c
> > > +++ b/src/channel-display.c
> > > @@ -525,6 +525,43 @@ void
> > > spice_display_change_preferred_compression(SpiceChannel
> > > *channel,
> > > gint comp
> > > }
> > >
> > > /**
> > > + * spice_display_change_preferred_video_codec:
> > > + * @channel: a #SpiceDisplayChannel
> > > + * @compression: a #SpiceVideoCodecType
> > > + *
> > > + * Tells the spice server to change the preferred video codec
> > > type
> > > for
> > > + * streaming in @channel. Application can set only one
> > > preferred
> > > video codec.
> > > + *
> > > + * Since: 0.34
> > > + */
> > > +void
> > > spice_display_change_preferred_video_codec_type(SpiceChannel
> > > *channel, gint codec_type)
> > > +{
> > > + SpiceMsgOut *out;
> > > + SpiceMsgcDisplayPreferredVideoCodecType *msg;
> > > +
> > > + g_return_if_fail(SPICE_IS_DISPLAY_CHANNEL(channel));
> > > + g_return_if_fail(codec_type >=
> > > SPICE_VIDEO_CODEC_TYPE_MJPEG);
> > > + g_return_if_fail(codec_type <
> > > SPICE_VIDEO_CODEC_TYPE_ENUM_END);
> > > +
> > > + if (!spice_channel_test_capability(channel,
> > > SPICE_DISPLAY_CAP_PREF_VIDEO_CODEC_TYPE)) {
> > > + CHANNEL_DEBUG(channel, "does not have capability to
> > > change
> > > the preferred video codec type");
> > > + return;
> > > + }
> > > +
> > > + CHANNEL_DEBUG(channel, "changing preferred video codec type
> > > to
> > > %d", codec_type);
> > > +
> > > + msg =
> > > g_malloc0(sizeof(SpiceMsgcDisplayPreferredVideoCodecType)
> > > +
> > > + (sizeof(SpiceVideoCodecType) * 1));
> > > + msg->num_of_codecs = 1;
> > > + msg->codecs[0] = codec_type;
> > > +
> > > + out = spice_msg_out_new(channel,
> > > SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE);
> > > + out->marshallers-
> > > >msgc_display_preferred_video_codec_type(out-
> > > > marshaller, msg);
> > >
> > > + spice_msg_out_send_internal(out);
> > > + g_free(msg);
> > > +}
> > > +
> > > +/**
> > > * spice_display_get_gl_scanout:
> > > * @channel: a #SpiceDisplayChannel
> > > *
> > > diff --git a/src/channel-display.h b/src/channel-display.h
> > > index ad82a16..fccf228 100644
> > > --- a/src/channel-display.h
> > > +++ b/src/channel-display.h
> > > @@ -149,6 +149,7 @@
> > > gboolean spice_display_get_primary(SpiceChannel *channel,
> > > guint32 surface
> > > SpiceDisplayPrimary
> > > *primary);
> > >
> > > void spice_display_change_preferred_compression(SpiceChannel
> > > *channel, gint compression);
> > > +void
> > > spice_display_change_preferred_video_codec_type(SpiceChannel
> > > *channel, gint codec_type);
> > >
> > > GType spice_gl_scanout_get_type (void)
> > > G_GNUC_CONST;
> > > void spice_gl_scanout_free (SpiceGlScanout
> > > *scanout);
> > > diff --git a/src/map-file b/src/map-file
> > > index 3d92153..31cafc2 100644
> > > --- a/src/map-file
> > > +++ b/src/map-file
> > > @@ -21,6 +21,7 @@ spice_channel_type_to_string;
> > > spice_client_error_quark;
> > > spice_cursor_channel_get_type;
> > > spice_display_change_preferred_compression;
> > > +spice_display_change_preferred_video_codec_type;
> > > spice_display_channel_get_type;
> > > spice_display_get_gl_scanout;
> > > spice_display_get_grab_keys;
> > > diff --git a/src/spice-glib-sym-file b/src/spice-glib-sym-file
> > > index 473c5ca..d73f799 100644
> > > --- a/src/spice-glib-sym-file
> > > +++ b/src/spice-glib-sym-file
> > > @@ -19,6 +19,7 @@ spice_channel_type_to_string
> > > spice_client_error_quark
> > > spice_cursor_channel_get_type
> > > spice_display_change_preferred_compression
> > > +spice_display_change_preferred_video_codec_type
> > > spice_display_channel_get_type
> > > spice_display_get_gl_scanout
> > > spice_display_get_primary
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel