[Spice-devel] [PATCH spice-gtk v7] Fix overlay for vaapisink
Snir Sheriber
ssheribe at redhat.com
Sun Jan 20 10:05:05 UTC 2019
Hi,
Just came to me that it may fail if HAVE_LIBVA is not defined && version
is >1.14 and so i checked and
it seems to work also without the context handling, Is it possible that
application's context handling is
not needed? I couldn't find any related vaapisink bug that was closed
recently.
Snir.
On 1/15/19 4:28 PM, Snir Sheriber wrote:
> 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