[spice-gtk,v7] Fix overlay for vaapisink

Submitted by Snir Sheriber on Jan. 21, 2019, 12:40 p.m.

Details

Message ID 4b3263df-ba71-3d59-062a-5e874966c68d@redhat.com
State New
Headers show
Series "Fix overlay for vaapisink" ( rev: 7 ) in Spice

Not browsing as part of any series.

Commit Message

Snir Sheriber Jan. 21, 2019, 12:40 p.m.
On 1/21/19 2:04 PM, Frediano Ziglio wrote:
>> On 1/21/19 12:21 PM, Frediano Ziglio wrote:
>>>> 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.
>>>>
>>> It crashed for me. Are you sure code is using vaapisink? Print the
>>
>> Yes, first thing I've check, see dot file attached ( I just revert this
>> patch and comment out the vaapisink ranking)
>>
>> After additional check, It seems to stop crashing for me since the
>> overlay control was moved to the widget.
>>
> Which exact patch(es) ?


It's crashing before 8c5bf5dedab3cd5a9c7ac484c0d97d43aeb0056b

If i comment out vaapisink avoidance

> Maybe some minor changes did the trick ?


It is working on upstream + the attached patch


> I cannot see the reason why moving code from one side to another
> would have fix the issue, would be different if the initialization
> would be done at library (spice-glib) initialization but it's done
> lately when the stream arrive so after all libraries are initialized.


It's indeed weird, btw pipelines are identical in both cases


>
>> Snir.
>>
>>> pipeline, sometimes code is using vaapi but not the sink (so Gstreamer
>>> extracts textures and then use xv plugin for instance).
>>>

Patch hide | download patch | download mbox

From 52c6845588c76565ed3e7ad4a0eb498e678d1180 Mon Sep 17 00:00:00 2001
From: Snir Sheriber <ssheribe@redhat.com>
Date: Mon, 21 Jan 2019 14:10:04 +0200
Subject: [PATCH] Revert "Fix overlay for vaapisink" and allow vaapisink

This reverts commit 61f6138bb5c88acda418555a0ebe17b7ffff9a6b.
Also disable vaapisink avoidance
---
 configure.ac              |  5 ----
 meson.build               |  5 ----
 src/Makefile.am           |  2 --
 src/channel-display-gst.c | 13 ++++-----
 src/spice-widget.c        | 57 ---------------------------------------
 5 files changed, 5 insertions(+), 77 deletions(-)

diff --git a/configure.ac b/configure.ac
index 2f63422..5d6a1a0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -175,11 +175,6 @@  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 70dd318..011c871 100644
--- a/meson.build
+++ b/meson.build
@@ -134,11 +134,6 @@  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 a9617d4..abc2f69 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -89,7 +89,6 @@  SPICE_COMMON_CPPFLAGS =						\
 	$(GUDEV_CFLAGS)						\
 	$(SOUP_CFLAGS)						\
 	$(PHODAV_CFLAGS)					\
-	$(LIBVA_CFLAGS)						\
 	$(X11_CFLAGS)					\
 	$(LZ4_CFLAGS)					\
 	$(NULL)
@@ -113,7 +112,6 @@  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 0b871a7..cd67814 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -410,17 +410,14 @@  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.
-         */
-        SPICE_DEBUG("Video is presented using gstreamer's GstVideoOverlay interface");
-
-#if !GST_CHECK_VERSION(1,14,0)
-        /* Avoid using vaapisink if exist since vaapisink could be
+         * 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;
+        /*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");
@@ -428,8 +425,8 @@  static gboolean create_pipeline(SpiceGstDecoder *decoder)
         if (vaapisink) {
             gst_plugin_feature_set_rank(vaapisink, GST_RANK_NONE);
             gst_object_unref(vaapisink);
-        }
-#endif
+        }*/
+        printf("OVERLAY\n");
     }
 
     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 8adcc38..0c6f16f 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -28,9 +28,6 @@ 
 #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>
@@ -2569,40 +2566,6 @@  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)) {
@@ -2624,26 +2587,6 @@  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;
-- 
2.19.1