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

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

 



Hi,

On 1/14/19 11:48 AM, Frediano Ziglio wrote:
Hi,


On 1/11/19 6:36 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@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 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));

Maybe worth mentioning it's gstreamer related (Gstreamer: ?)

Added.

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

It seems there is some kind of a race, every few runs it gets frozen and
then crashes, i looked a bit,

could not found a clear cause.

I had similar issues in the past but patch was quite different.
Also I had Fedora 28, now I have Fedora 29, which OS are you running?


f29

(I guess it should not matter much but i'm using cinnamon not gnome)


How often does it happen?


Very often, every 3-4 consecutive runs


Does it happen doing something specific like screen resize?


No, when it happens it does not event get to the point of a clear image, immediately freezes.

fwiw i disabled the resize related callbacks and it still happened.



One time i got these errors but may not be always related since it
usually crashes without any hints


** (remote-viewer:19435): CRITICAL **: 11:44:18.845:
gst_vaapi_window_reconfigure: assertion 'window != NULL' failed

** (remote-viewer:19435): CRITICAL **: 11:44:18.845:
gst_vaapi_window_get_size: assertion 'window != NULL' failed


Snir.

None of these.
Are you sure your machine is using VAAPI, did you dump the pipeline?
window != NULL seems to suggest there's no overlay involved.


This specific warning i got only once, usually i get it without it, i do have vaapi, and seems

it sets the vaapicontext correctly in replay to the context msg


Snir.


Frediano
_______________________________________________
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]