[Spice-devel] [PATCH spice-gtk v4 1/2] Fix overlay for vaapisink

Frediano Ziglio fziglio at redhat.com
Tue Jan 15 12:08:04 UTC 2019


> 
> On 1/15/19 12:22 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 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 | 14 ------------
> >   src/spice-widget.c        | 45 +++++++++++++++++++++++++++++++++++++++
> >   5 files changed, 57 insertions(+), 14 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index bba13fba..0f69b3c2 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 acd5dcaf..69bbee57 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 d1de9473..66884a74 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);
> > -        }
> >       }
> 
> 
> I think the support gstcontext for vaapi elements was introduced pretty
> lately
> (if i'm not mistaken 1.13.90) so i guess it worth removing this code
> only in higher
> versions
> 

1.13.1. Is there a way to detect the version (better the plugin, not all
GStreamer)? Or a simple check for GStreamer 0.14 would be good?

> 
> >   
> >       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..69c00558 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,48 @@ 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(GDK_WINDOWING_X11) && defined(HAVE_LIBVA)
> > +        if (g_strcmp0(context_type, "gst.vaapi.app.Display") == 0) {
> 
> 
> Not critical and also can be done later but maybe worth to implement in
> a create_context function
> so later we can have something like:
> 
> create_vaapi_context()
> 
> create_gl_context()
> 
> ...
> 
> 
> Snir
> 

Done, I'll send a new patch

> > +            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


More information about the Spice-devel mailing list