[Spice-devel] [PATCH spice-gtk v3] Fix overlay for vaapisink
Marc-André Lureau
marcandre.lureau at gmail.com
Fri Jan 11 17:35:33 UTC 2019
Hi
On Fri, Jan 11, 2019 at 9:22 PM Frediano Ziglio <fziglio at redhat.com> wrote:
>
> > Hi
> >
> > On Fri, Jan 11, 2019 at 8:36 PM Frediano Ziglio <fziglio at redhat.com> 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 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 | 14 -------------
> > > src/spice-widget.c | 44 +++++++++++++++++++++++++++++++++++++++
> > > 5 files changed, 56 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/configure.ac b/configure.ac
> > > index 8c5c4d38..ead6c1bb 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 37377680..6e96cca8 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 b876530c..784b18d0 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..01ee83cb 100644
> > > --- a/src/channel-display-gst.c
> > > +++ b/src/channel-display-gst.c
> > > @@ -406,22 +406,8 @@ 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
> > > - * 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");
> > > - }
> > > - if (vaapisink) {
> > > - gst_plugin_feature_set_rank(vaapisink, GST_RANK_NONE);
> > > - gst_object_unref(vaapisink);
> > > - }
> > > }
> > >
> > > 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 af0301c1..85707704 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>
> > > @@ -2592,6 +2595,47 @@ 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);
> > > + g_debug("got need context %s from %s", context_type,
> > > GST_MESSAGE_SRC_NAME(msg));
> >
> > ack with s/g_debug/SPICE_DEBUG
> >
>
> I'll do.
> Do you want to wait after the 0.36 release?
> I'll wait Snir opinion, he did lot of work on this area (direct
> rendering).
It's ok in v0.36 imho.
Ok let's wait for Snir to review.
>
> > > +
> > > +#if defined(GDK_WINDOWING_X11) && defined(HAVE_LIBVA)
> > > + if (g_strcmp0(context_type, "gst.vaapi.app.Display") == 0) {
> > > + 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);
> > > +
> > > + gst_element_set_context(GST_ELEMENT(msg->src), context);
> > > + gst_context_unref(context);
> > > + }
> > > +#endif
> > > + break;
> > > + }
> > > default:
> > > /* not being handled */
> > > break;
>
> Frediano
--
Marc-André Lureau
More information about the Spice-devel
mailing list