[Spice-devel] [PATCH spice-gtk v7] Fix overlay for vaapisink
Snir Sheriber
ssheribe at redhat.com
Tue Jan 15 14:28:16 UTC 2019
Hi,
On 1/15/19 3:03 PM, Frediano Ziglio wrote:
> The vaapisink plugin to support overlay requires the application
> to provide the proper context. If you don't do so the plugin will
> cause a crash of the application.
> To avoid possible thread errors from X11 create a new Display
> connection to pass to vaapisink.
>
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
> To test it probably you'll have to remove the gstreamer registry,
> usually in ~/.cache/gstreamer-1.0/registry.x86_64.bin.
>
> Changes since v6:
> - rebase on master.
>
> Changes since v5:
> - test GStreamer version to use or not vaapisink.
>
> Changes since v4:
> - factor out a create_vaapi_context.
>
> Changes since v3:
> - none, add a patch to fix another issue.
>
> Changes since v2:
> - remove hard dependency to libva-x11.
>
> Changes since v1:
> - updated to master;
> - use a different X11 connection as discussed in a previous thread;
> - remove some comments, now obsoleted;
> - fixed direct X11 link, now code is in spice-widget (as it should be);
> - better libva linking, using now build systems.
> ---
> configure.ac | 5 ++++
> meson.build | 5 ++++
> src/Makefile.am | 2 ++
> src/channel-display-gst.c | 8 ++++--
> src/spice-widget.c | 57 +++++++++++++++++++++++++++++++++++++++
> 5 files changed, 75 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 5d6a1a01..2f634223 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -175,6 +175,11 @@ PKG_CHECK_MODULES(GTHREAD, gthread-2.0)
>
> PKG_CHECK_MODULES(JSON, json-glib-1.0)
>
> +PKG_CHECK_EXISTS([libva-x11], [
> + PKG_CHECK_MODULES(LIBVA, libva-x11)
> + AC_DEFINE([HAVE_LIBVA], [1], [Define if libva is available])
> +])
> +
> AC_ARG_ENABLE([webdav],
> AS_HELP_STRING([--enable-webdav=@<:@auto/yes/no@:>@],
> [Enable webdav support @<:@default=auto@:>@]),
> diff --git a/meson.build b/meson.build
> index 94d660a6..d7062af2 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -134,6 +134,11 @@ if d.found()
> if host_machine.system() != 'windows'
> spice_gtk_deps += dependency('epoxy')
> spice_gtk_deps += dependency('x11')
> + d = dependency('libva-x11', required: false)
> + if d.found()
> + spice_gtk_deps += d
> + spice_gtk_config_data.set('HAVE_LIBVA', '1')
> + endif
> endif
> spice_gtk_has_gtk = true
> endif
> diff --git a/src/Makefile.am b/src/Makefile.am
> index abc2f694..a9617d47 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -89,6 +89,7 @@ SPICE_COMMON_CPPFLAGS = \
> $(GUDEV_CFLAGS) \
> $(SOUP_CFLAGS) \
> $(PHODAV_CFLAGS) \
> + $(LIBVA_CFLAGS) \
> $(X11_CFLAGS) \
> $(LZ4_CFLAGS) \
> $(NULL)
> @@ -112,6 +113,7 @@ SPICE_GTK_LIBADD_COMMON = \
> $(CAIRO_LIBS) \
> $(X11_LIBS) \
> $(LIBM) \
> + $(LIBVA_LIBS) \
> $(NULL)
>
> SPICE_GTK_SOURCES_COMMON = \
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index 2f556fe4..5b7b776a 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -406,14 +406,17 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
> } else {
> /* handle has received, it means playbin will render directly into
> * widget using the gstvideooverlay interface instead of app-sink.
> - * Also avoid using vaapisink if exist since vaapisink could be
> + */
> + SPICE_DEBUG("Video is presented using gstreamer's GstVideoOverlay interface");
> +
> +#if !GST_CHECK_VERSION(1,14,0)
It indeed possible to check plugin version as was done in "[PATCH
spice-gtk v3] gst: check pulsesrc version >= 1.14.5"
but i think GST_CHECK_VERSION is enough here
ACK
> + /* Avoid using vaapisink if exist since vaapisink could be
> * buggy when it is combined with playbin. changing its rank to
> * none will make playbin to avoid of using it.
> */
> GstRegistry *registry = NULL;
> GstPluginFeature *vaapisink = NULL;
>
> - SPICE_DEBUG("Video is presented using gstreamer's GstVideoOverlay interface");
> registry = gst_registry_get();
> if (registry) {
> vaapisink = gst_registry_lookup_feature(registry, "vaapisink");
> @@ -422,6 +425,7 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
> gst_plugin_feature_set_rank(vaapisink, GST_RANK_NONE);
> gst_object_unref(vaapisink);
> }
> +#endif
> }
>
> g_signal_connect(playbin, "source-setup", G_CALLBACK(app_source_setup), decoder);
> diff --git a/src/spice-widget.c b/src/spice-widget.c
> index 0c6f16fd..8adcc384 100644
> --- a/src/spice-widget.c
> +++ b/src/spice-widget.c
> @@ -28,6 +28,9 @@
> #ifdef GDK_WINDOWING_X11
> #include <X11/Xlib.h>
> #include <gdk/gdkx.h>
> +#ifdef HAVE_LIBVA
> +#include <va/va_x11.h>
> +#endif
> #endif
> #ifdef G_OS_WIN32
> #include <windows.h>
> @@ -2566,6 +2569,40 @@ static void queue_draw_area(SpiceDisplay *display, gint x, gint y,
> }
>
> #if defined(GDK_WINDOWING_X11)
> +
> +#if defined(HAVE_LIBVA)
> +static GstContext *create_vaapi_context(void)
> +{
> + static Display *x11_display = NULL;
> + static VADisplay va_display = NULL;
> +
> + // note that if VAAPI do not get the context for the
> + // overlay it crashes the entire program!
> + GdkDisplay *display = gdk_display_get_default();
> + g_assert_nonnull(display);
> +
> + // Compute display pointers
> + if (!x11_display && GDK_IS_X11_DISPLAY(display)) {
> + x11_display = gdk_x11_display_get_xdisplay(display);
> + // for thread problems we need a different Display,
> + // VAAPI access the Display from another thread
> + x11_display = XOpenDisplay(XDisplayString(x11_display));
> + g_assert_nonnull(x11_display);
> + va_display = vaGetDisplay(x11_display);
> + g_assert_nonnull(va_display);
> + }
> +
> + GstContext *context = gst_context_new("gst.vaapi.app.Display", FALSE);
> + GstStructure *structure = gst_context_writable_structure(context);
> + if (x11_display) {
> + gst_structure_set(structure, "x11-display", G_TYPE_POINTER, x11_display, NULL);
> + }
> + gst_structure_set(structure, "va-display", G_TYPE_POINTER, va_display, NULL);
> +
> + return context;
> +}
> +#endif
> +
> static void gst_sync_bus_call(GstBus *bus, GstMessage *msg, SpiceDisplay *display)
> {
> switch(GST_MESSAGE_TYPE(msg)) {
> @@ -2587,6 +2624,26 @@ static void gst_sync_bus_call(GstBus *bus, GstMessage *msg, SpiceDisplay *displa
> }
> break;
> }
> + case GST_MESSAGE_NEED_CONTEXT:
> + {
> + const gchar *context_type;
> +
> + gst_message_parse_context_type(msg, &context_type);
> + SPICE_DEBUG("GStreamer: got need context %s from %s", context_type,
> + GST_MESSAGE_SRC_NAME(msg));
> +
> +#if defined(HAVE_LIBVA)
> + if (g_strcmp0(context_type, "gst.vaapi.app.Display") == 0) {
> + GstContext *context = create_vaapi_context();
> +
> + if (context) {
> + gst_element_set_context(GST_ELEMENT(msg->src), context);
> + gst_context_unref(context);
> + }
> + }
> +#endif
> + break;
> + }
> default:
> /* not being handled */
> break;
More information about the Spice-devel
mailing list