Hi,
On Thu, Jan 19, 2017 at 09:09:58AM +0100, Pavel Grunt wrote:
> 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 ?
Yes, with only one value at this time.
> > 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);
You are right... :(
> > 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.
Did not that think far ahead in the implementation. spice-gtk will need
to store a list of video-codecs in its order of preference and that's
what we are going to send.
Probably the 'sending' part will be moved to a new function and this API
will only change this list and call the 'sending' function.
I can re-implement it to follow the above, if you think it is better.
>
> >
> > If applications really need it, we can extend in the future.
>
> function like
> _change_preferred_video_codec_types(SpiceChannel *channel,
> GArray *codec_types)
But that means we are exporting it to the application, what is the
benefit? Do you think gnome-boxes/remote-viewer/spicy might need it?
> is basically the same and you would avoid creating a new function for
> sending the message (for the smart detection you mentioned in 1)
Yeah, doing this way might imply fewer changes in the future.
toso
>
> 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
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel