Re: [PATCH spice-gtk v2] Fix overlay for vaapisink

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On Fri, Jan 11, 2019 at 7:08 PM Frediano Ziglio <fziglio@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> To test it probably you'll have to remove the gstreamer registry,
> usually in ~/.cache/gstreamer-1.0/registry.x86_64.bin.
>
> 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              |  2 ++
>  meson.build               |  1 +
>  src/Makefile.am           |  2 ++
>  src/channel-display-gst.c | 14 -------------
>  src/spice-widget.c        | 42 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 47 insertions(+), 14 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 8c5c4d38..54a86681 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -175,6 +175,8 @@ PKG_CHECK_MODULES(GTHREAD, gthread-2.0)
>
>  PKG_CHECK_MODULES(JSON, json-glib-1.0)
>
> +PKG_CHECK_MODULES(LIBVA, libva-x11)

This is a strong dependency that we can't satisfy on all platforms.
You should make it optional.

> +
>  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..8ab32de5 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -134,6 +134,7 @@ if d.found()
>    if host_machine.system() != 'windows'
>      spice_gtk_deps += dependency('epoxy')
>      spice_gtk_deps += dependency('x11')
> +    spice_gtk_deps += dependency('libva-x11')
>    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..a0763fe6 100644
> --- a/src/spice-widget.c
> +++ b/src/spice-widget.c
> @@ -28,6 +28,7 @@
>  #ifdef GDK_WINDOWING_X11
>  #include <X11/Xlib.h>
>  #include <gdk/gdkx.h>
> +#include <va/va_x11.h>
>  #endif
>  #ifdef G_OS_WIN32
>  #include <windows.h>
> @@ -2592,6 +2593,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));
> +
> +#ifdef GDK_WINDOWING_X11
> +        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;
> --
> 2.20.1
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



-- 
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]