[Spice-devel] [RFC PATCH spice-gtk] Fix overlay for vaapisink

[Frediano Ziglio]

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.
Note that overlay message should be handled synchronously and
not asynchronously so gst_bus_set_sync_handler is used.
To avoid possible thread errors from X11 call XInitThreads before
any X11 calls.

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.

Linking directly to X11 or Gtk should be avoided and made in spice-gtk
instead. This require a bit of refactory of the interaction between
spice-gtk and spice-glib which after the direct rendering supports
seems a bit broken.

Currently we don't support Wayland. Tried to pass the wl_surface instead
of the XID but got some weird message about X11 errors (weird because
X11 should not be used at all! Maybe some gstreamer plugin is connected
to X11 emulation in wayland? Need investigating, unsetting, also from
gstreamer documentation is not clear what gstreamer is expecting
for gst_video_overlay_set_window_handle on wayland).

Note that this patch also add a dependency to libva. This could be
removed using dlopen/dlsym.

Performance are so better that you don't need to measure much!
---
 src/Makefile.am           |  2 ++
 src/channel-display-gst.c | 71 ++++++++++++++++++++++++++++++---------
 2 files changed, 58 insertions(+), 15 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 1bb6f9bf..c4cd19a7 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -196,6 +196,8 @@ libspice_client_glib_2_0_la_LIBADD =					\
 	$(USBREDIR_LIBS)						\
 	$(GUDEV_LIBS)							\
 	$(PHODAV_LIBS)							\
+	$(GTK_LIBS)							\
+	-lva-x11 -lX11							\
 	$(NULL)
 
 if WITH_POLKIT
diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 2c07f350..70539719 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -28,6 +28,9 @@
 #include <gst/app/gstappsink.h>
 #include <gst/video/gstvideometa.h>
 
+#include <gdk/gdkx.h>
+#include <va/va_x11.h>
+
 
 typedef struct SpiceGstFrame SpiceGstFrame;
 
@@ -299,6 +302,14 @@ static void free_pipeline(SpiceGstDecoder *decoder)
     decoder->pipeline = NULL;
 }
 
+// See https://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/GstGLDisplay.html
+// (look for XInitThreads). We call it into a constructor to make sure
+// we call before any X11 function.
+SPICE_CONSTRUCTOR_FUNC(i18n_init)
+{
+    XInitThreads();
+}
+
 static gboolean handle_pipeline_message(GstBus *bus, GstMessage *msg, gpointer video_decoder)
 {
     SpiceGstDecoder *decoder = video_decoder;
@@ -334,6 +345,19 @@ static gboolean handle_pipeline_message(GstBus *bus, GstMessage *msg, gpointer v
         g_free(filename);
         break;
     }
+    default:
+        /* not being handled */
+        break;
+    }
+    return TRUE;
+}
+
+static GstBusSyncReply
+handle_pipeline_message_sync(GstBus *bus, GstMessage *msg, gpointer video_decoder)
+{
+    SpiceGstDecoder *decoder = video_decoder;
+
+    switch (GST_MESSAGE_TYPE(msg)) {
     case GST_MESSAGE_ELEMENT: {
         if (gst_is_video_overlay_prepare_window_handle_message(msg)) {
             GstVideoOverlay *overlay;
@@ -348,11 +372,41 @@ static gboolean handle_pipeline_message(GstBus *bus, GstMessage *msg, gpointer v
         }
         break;
     }
+    case GST_MESSAGE_NEED_CONTEXT:
+    {
+        const gchar *context_type;
+
+        gst_message_parse_context_type(msg, &context_type);
+        spice_debug("got need context %s from %s\n", context_type, GST_MESSAGE_SRC_NAME(msg));
+
+        if (g_strcmp0(context_type, "gst.vaapi.app.Display") == 0) {
+            static Display *x11_display = NULL;
+            static VADisplay va_display = NULL;
+
+            if (!x11_display) {
+                x11_display = gdk_x11_get_default_xdisplay();
+                g_assert_nonnull(x11_display);
+            }
+            if (!va_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);
+            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);
+        }
+        break;
+    }
     default:
         /* not being handled */
         break;
     }
-    return TRUE;
+    return GST_BUS_PASS;
 }
 
 #if GST_CHECK_VERSION(1,9,0)
@@ -425,21 +479,7 @@ 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;
-
-        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);
@@ -494,6 +534,7 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
     }
     bus = gst_pipeline_get_bus(GST_PIPELINE(decoder->pipeline));
     gst_bus_add_watch(bus, handle_pipeline_message, decoder);
+    gst_bus_set_sync_handler(bus, handle_pipeline_message_sync, decoder, NULL);
     gst_object_unref(bus);
 
     decoder->clock = gst_pipeline_get_clock(GST_PIPELINE(decoder->pipeline));
-- 
2.17.2