[Spice-devel] [spice-gtk v1 1/2] Use libva to check video hardware accel capabilities
Victor Toso
victortoso at redhat.com
Mon Feb 19 14:14:03 UTC 2018
Hi,
On Mon, Feb 19, 2018 at 02:44:04PM +0100, Christophe Fergeau wrote:
> Hey,
>
> On Thu, Feb 15, 2018 at 10:05:04AM +0100, Victor Toso wrote:
> > From: Victor Toso <me at victortoso.com>
> >
> > Libva is an implementation for VA-API.
> >
> > This can be used to automatically send to the server the
> > preferred video codecs as the client would prefer streams
> > with video codecs that can be decoded with gpu support.
> >
> > We can also use the profiles to detect and set upper limit for
> > video streams quality.
> > e.g: Don't start UHD video stream if client's hardware don't
> > support it.
> >
> > This patch makes usage of libva in spice-session and exposes this
> > information to all available channel-displays with the internal
> > function spice_session_get_hw_accel_video_codecs()
>
> This assumes that HW accelerated video decoding is going to go
> through libva when using GStreamer.
Not really. I think it is fairly possible to query hw
capabilities with libva while doing hardware decoding without
libva (nvdec element for NVidia, for instance) but I haven't
tested that.
> I assume it's not possible to directly query GStreamer to know
> what it can hardware decode?
Not really, although I have asked [0] - We still need some
support from GStreamer to detect if we are doing hardware or
software decoding (somehow!) without having to check elements as
this does not scale well, I think.
[0] https://bugzilla.gnome.org/show_bug.cgi?id=782741
Note also that libva here in spice-gtk could tell that we do have
support to vp8/vp9/h264 hw decoding but in fact there is no
element installed in GStreamer to do that.
I plan to send a v2 including the GStreamer check to, at least,
be sure that we are sending the preferred video codecs based on
hw capability + gstreamer support + the fixes from this review
soon :)
>
> >
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> > configure.ac | 20 +++++++
> > src/Makefile.am | 12 ++++
> > src/spice-session-priv.h | 1 +
> > src/spice-session.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 172 insertions(+)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 2a14055..0b0db0f 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -321,6 +321,25 @@ AC_SUBST(Z_LIBS)
> > SPICE_CHECK_SMARTCARD
> > AM_CONDITIONAL([WITH_SMARTCARD], [test "x$have_smartcard" = "xyes"])
> >
> > +AC_ARG_ENABLE([libva],
> > + AS_HELP_STRING([--enable-libva=@<:@auto/yes/no@:>@], [Enable auto detection of hardware accelerate video decoding support @<:@default=auto@:>@]),
> > + [],
> > + [enable_libva="auto"])
> > +AS_IF([test "x$enable_libva" != "xno"],
> > + [PKG_CHECK_MODULES(LIBVA, [libva >= 1.0.0],
> > + [AC_DEFINE([HAVE_LIBVA], 1, [Have libva support?])
> > + enable_libva="yes"],
> > + [AS_IF([test "x$enable_libva" = "xyes"],
> > + AC_MSG_ERROR([Auto detection of hardware accelerated video decoding explicitly requested, but some required packages are not available]))
> > + enable_libva="no"
> > + ])
> > + PKG_CHECK_MODULES([LIBVA_X11], [libva-x11 >= 1.0.0])
> > + PKG_CHECK_MODULES([LIBVA_WAYLAND], [libva-wayland >= 1.0.0])
> > + PKG_CHECK_MODULES([GDK_X11], [gdk-x11-3.0])
> > + PKG_CHECK_MODULES([GDK_WAYLAND], [gdk-wayland-3.0])
>
> I don't think we'll necessarily have all of these installed?
> PKG_CHECK_MODULES will error out if any of these is missing.
I'll test and remove them, thanks!
>
> > +])
> > +AM_CONDITIONAL([HAVE_LIBVA], [test "x$enable_libva" = "xyes"])
> > +
> > AC_ARG_ENABLE([usbredir],
> > AS_HELP_STRING([--enable-usbredir=@<:@auto/yes/no@:>@],
> > [Enable usbredir support @<:@default=auto@:>@]),
> > @@ -635,6 +654,7 @@ AC_MSG_NOTICE([
> > DBus: ${have_dbus}
> > WebDAV support: ${have_phodav}
> > LZ4 support: ${have_lz4}
> > + Libva support: ${enable_libva}
> >
> > Now type 'make' to build $PACKAGE
> >
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index 4b6e46d..7b74220 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -75,6 +75,9 @@ SPICE_COMMON_CPPFLAGS = \
> > $(PIXMAN_CFLAGS) \
> > $(PULSE_CFLAGS) \
> > $(GTK_CFLAGS) \
> > + $(GDK_CFLAGS) \
> > + $(GDK_X11_CFLAGS) \
> > + $(GDK_WAYLAND_CFLAGS) \
> > $(CAIRO_CFLAGS) \
> > $(GLIB2_CFLAGS) \
> > $(GIO_CFLAGS) \
> > @@ -88,6 +91,9 @@ SPICE_COMMON_CPPFLAGS = \
> > $(GUDEV_CFLAGS) \
> > $(SOUP_CFLAGS) \
> > $(PHODAV_CFLAGS) \
> > + $(LIBVA_CFLAGS) \
> > + $(LIBVA_X11_CFLAGS) \
> > + $(LIBVA_WAYLAND_CFLAGS) \
> > $(X11_CFLAGS) \
> > $(LZ4_CFLAGS) \
> > $(NULL)
> > @@ -195,6 +201,12 @@ libspice_client_glib_2_0_la_LIBADD = \
> > $(USBREDIR_LIBS) \
> > $(GUDEV_LIBS) \
> > $(PHODAV_LIBS) \
> > + $(GDK_LIBS) \
> > + $(GDK_X11_LIBS) \
> > + $(GDK_WAYLAND_LIBS) \
>
> Adding GDK_* to libspice_client_glib_2_0_la_LIBADD does not look good.
> Maybe you want to move this to SpiceGtkSession rather than SpiceSession.
True, I'll do.
>
> > + $(LIBVA_LIBS) \
> > + $(LIBVA_X11_LIBS) \
> > + $(LIBVA_WAYLAND_LIBS) \
> > $(NULL)
> >
> > if WITH_POLKIT
> > diff --git a/src/spice-session-priv.h b/src/spice-session-priv.h
> > index 03005aa..7137cf6 100644
> > --- a/src/spice-session-priv.h
> > +++ b/src/spice-session-priv.h
> > @@ -100,6 +100,7 @@ void spice_session_set_main_channel(SpiceSession *session, SpiceChannel *channel
> > gboolean spice_session_set_migration_session(SpiceSession *session, SpiceSession *mig_session);
> > SpiceAudio *spice_audio_get(SpiceSession *session, GMainContext *context);
> > const gchar* spice_audio_data_mode_to_string(gint mode);
> > +const GArray *spice_session_get_hw_accel_video_codecs(SpiceSession *session);
> > G_END_DECLS
> >
> > #endif /* __SPICE_CLIENT_SESSION_PRIV_H__ */
> > diff --git a/src/spice-session.c b/src/spice-session.c
> > index 2aabf58..e26d375 100644
> > --- a/src/spice-session.c
> > +++ b/src/spice-session.c
> > @@ -23,6 +23,19 @@
> > #include <gio/gunixsocketaddress.h>
> > #endif
> > #include "common/ring.h"
> > +#ifdef HAVE_LIBVA
> > +#include <gdk/gdk.h>
> > +#include <va/va.h>
> > +#include <va/va_str.h>
> > +#ifdef GDK_WINDOWING_WAYLAND
> > +#include <gdk/gdkwayland.h>
> > +#include <va/va_wayland.h>
> > +#endif
> > +#ifdef GDK_WINDOWING_X11
> > +#include <gdk/gdkx.h>
> > +#include <va/va_x11.h>
> > +#endif
> > +#endif
> >
> > #include "spice-client.h"
> > #include "spice-common.h"
> > @@ -33,6 +46,7 @@
> > #include "spice-uri-priv.h"
> > #include "channel-playback-priv.h"
> > #include "spice-audio-priv.h"
> > +#include "channel-display-priv.h"
> >
> > struct channel {
> > SpiceChannel *channel;
> > @@ -116,6 +130,9 @@ struct _SpiceSessionPrivate {
> > guint8 uuid[16];
> > gchar *name;
> > SpiceImageCompression preferred_compression;
> > +
> > + /* Array of SpiceVideoCodecType with hw accelerated video decoding capability */
> > + GArray *video_codecs;
> >
> > /* associated objects */
> > SpiceAudio *audio_manager;
> > @@ -248,6 +265,7 @@ spice_image_compress_get_type (void)
> > static guint signals[SPICE_SESSION_LAST_SIGNAL];
> >
> > static void spice_session_channel_destroy(SpiceSession *session, SpiceChannel *channel);
> > +static void spice_session_check_video_hw_caps(SpiceSession *session);
> >
> > static void update_proxy(SpiceSession *self, const gchar *str)
> > {
> > @@ -299,6 +317,9 @@ static void spice_session_init(SpiceSession *session)
> > SPICE_DEBUG("Could not initialize SpiceUsbDeviceManager - %s", err->message);
> > g_clear_error(&err);
> > }
> > +
> > + session->priv->video_codecs = NULL;
>
> Here, session->priv->video_codecs should already be NULL.
Indeed
>
> > + spice_session_check_video_hw_caps(session);
> > }
> >
> > static void
> > @@ -2801,3 +2822,121 @@ gboolean spice_session_set_migration_session(SpiceSession *session, SpiceSession
> >
> > return TRUE;
> > }
> > +
> > +G_GNUC_INTERNAL
> > +const GArray *spice_session_get_hw_accel_video_codecs(SpiceSession *session)
> > +{
> > + g_return_val_if_fail(SPICE_IS_SESSION(session), NULL);
> > + return session->priv->video_codecs;
> > +}
> > +
> > +static void
> > +spice_session_check_video_hw_caps(SpiceSession *session)
> > +{
> > +#ifdef HAVE_LIBVA
> > + VADisplay va_dpy = NULL;
> > + VAStatus va_status;
> > + GdkDisplay *display;
> > + int major_version, minor_version;
> > + GArray *codecs;
> > + const gchar *last_profile = NULL;
> > + VAProfile *profile_list = NULL;
> > + int num_profiles, max_num_profiles, i;
> > + int num_entrypoint;
> > +
> > + display = gdk_display_get_default();
> > + spice_debug("Display: %s", gdk_display_get_name(display));
> > +
> > +#ifdef GDK_WINDOWING_X11
> > + if (GDK_IS_X11_DISPLAY(display))
> > + va_dpy = vaGetDisplay(gdk_x11_display_get_xdisplay(display));
> > +#endif
> > +#ifdef GDK_WINDOWING_WAYLAND
> > + if (GDK_IS_WAYLAND_DISPLAY(display))
> > + va_dpy = vaGetDisplayWl(gdk_wayland_display_get_wl_display(display));
> > +#endif
> > +
> > + if (va_dpy == NULL) {
> > + spice_warning("Failed to get VADisplay, unable to detect hardware capabilities");
>
> g_warning? but maybe g_debug() is enough?
Okay
>
> > + return;
> > + }
> > +
> > + va_status = vaInitialize(va_dpy, &major_version, &minor_version);
> > + if (va_status != VA_STATUS_SUCCESS) {
> > + spice_warning("Failed to initialize libva");
> > + return;
> > + }
> > +
> > + max_num_profiles = vaMaxNumProfiles(va_dpy);
> > + profile_list = g_new(VAProfile, max_num_profiles);
> > +
> > + if (!profile_list) {
> > + spice_warning("libva: failed to allocate memory for profile list");
>
> g_new will never return NULL.
But if max_num_profiles is 0, profile_list shall be NULL but
yeah, in this case the warning is misleading. I'll move the
check. Thanks!
> > + vaTerminate(va_dpy);
> > + return;
> > + }
> > +
> > + va_status = vaQueryConfigProfiles(va_dpy, profile_list, &num_profiles);
> > + if (va_status != VA_STATUS_SUCCESS) {
> > + spice_warning("libva: failed to query profiles");
> > + g_free(profile_list);
> > + vaTerminate(va_dpy);
> > + return;
> > + }
> > +
> > + codecs = g_array_new(FALSE, FALSE, sizeof(gint));
> > + for (i = 0; i < num_profiles; i++) {
> > + int j;
> > + VAEntrypoint entrypoints[50];
> > + VAProfile profile = profile_list[i];
> > + const char *profile_str = vaProfileStr(profile);
> > +
> > + /* Spice protocol does not support different profiles for a given codec
> > + * at the moment, which means that we can jump to the next codec. */
> > + if (last_profile != NULL &&
> > + g_ascii_strncasecmp(profile_str + strlen("VAProfile"),
> > + last_profile,
> > + strlen(last_profile)) == 0)
>
> This is repeated twice, might deserve a small helper for improved
> readability if you can come up with a good name.
I'll try
>
> > + continue;
> > +
> > + va_status = vaQueryConfigEntrypoints(va_dpy, profile, entrypoints, &num_entrypoint);
> > + if (va_status == VA_STATUS_ERROR_UNSUPPORTED_PROFILE)
> > + continue;
> > + else if (va_status != VA_STATUS_SUCCESS) {
> > + spice_warning("Error on vaQueryConfigEntrypoints()");
> > + break;
> > + }
> > +
> > + /* Find if current profile has decoding support */
> > + for (j = 0; j < num_entrypoint; j++) {
> > + int k;
> > +
> > + if (entrypoints[j] != VAEntrypointVLD)
> > + continue;
> > +
> > + /* Found decoding entrypoing, check if it is supported by Spice protocol */
> > + for (k = 1; k < SPICE_VIDEO_CODEC_TYPE_ENUM_END; k++) {
> > + if (g_ascii_strncasecmp(profile_str + strlen("VAProfile"),
> > + gst_opts[k].name,
> > + strlen(gst_opts[k].name)) == 0) {
> > + last_profile = gst_opts[k].name;
> > + g_array_append_val(codecs, k);
> > + spice_debug("Support to decode %s found with profile %s",
> > + gst_opts[k].name, profile_str);
> > + break;
> > + }
> > + }
> > + break;
> > + }
> > + }
> > +
> > + if (codecs->len > 0) {
> > + g_clear_pointer(&session->priv->video_codecs, g_array_unref);
> > + session->priv->video_codecs = g_array_ref(codecs);
>
> This also needs to be _unref'ed in _dispose or _finalize.
True, I forgot about it. Thanks again!
Cheers,
toso
>
> Christophe
>
> > + }
> > +
> > + g_array_unref(codecs);
> > + g_free(profile_list);
> > + vaTerminate(va_dpy);
> > +#endif
> > +}
> > --
> > 2.16.1
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180219/f2aa21c0/attachment.sig>
More information about the Spice-devel
mailing list