Re: [Spice-devel] [PATCH spice-gtk v4 1/2] Fix overlay for vaapisink

2019-01-15 Thread Frediano Ziglio
> 
> On 1/15/19 12:22 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 
> > ---
> > To test it probably you'll have to remove the gstreamer registry,
> > usually in ~/.cache/gstreamer-1.0/registry.x86_64.bin.
> >
> > 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 | 14 
> >   src/spice-widget.c| 45 +++
> >   5 files changed, 57 insertions(+), 14 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index bba13fba..0f69b3c2 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 acd5dcaf..69bbee57 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 d1de9473..66884a74 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);
> > -}
> >   }
> 
> 
> I think the support gstcontext for vaapi elements was introduced pretty
> lately
> (if i'm not mistaken 1.13.90) so i guess it worth removing this code
> only in higher
> versions
> 

1.13.1. Is there a way to detect the version (better the plugin, not all
GStreamer)? Or a simple check for GStreamer 0.14 would be good?

> 
> >   
> >   g_signal_connect(playbin, "source-setup",
> >   G_CALLBACK(app_so

Re: [Spice-devel] [PATCH spice-gtk v4 1/2] Fix overlay for vaapisink

2019-01-15 Thread Snir Sheriber

On 1/15/19 12:22 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 
---
To test it probably you'll have to remove the gstreamer registry,
usually in ~/.cache/gstreamer-1.0/registry.x86_64.bin.

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 | 14 
  src/spice-widget.c| 45 +++
  5 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/configure.ac b/configure.ac
index bba13fba..0f69b3c2 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 acd5dcaf..69bbee57 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 d1de9473..66884a74 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);
-}
  }



I think the support gstcontext for vaapi elements was introduced pretty 
lately
(if i'm not mistaken 1.13.90) so i guess it worth removing this code 
only in higher

versions


  
  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..69c00558 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -28,6 +28,9 @@
  #ifdef GDK_WINDOWING_X11
  #include 



  #include 
+#ifdef HAVE_LIBVA
+#include 
+#endif
  #endif
  #ifdef G_OS_WIN32
  #include 
@@ -2592,6 +2595,48 @@ 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("GSt

[Spice-devel] [PATCH spice-gtk v4 1/2] Fix overlay for vaapisink

2019-01-15 Thread 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.
To avoid possible thread errors from X11 create a new Display
connection to pass to vaapisink.

Signed-off-by: Frediano Ziglio 
---
To test it probably you'll have to remove the gstreamer registry,
usually in ~/.cache/gstreamer-1.0/registry.x86_64.bin.

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 | 14 
 src/spice-widget.c| 45 +++
 5 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/configure.ac b/configure.ac
index bba13fba..0f69b3c2 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 acd5dcaf..69bbee57 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 d1de9473..66884a74 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..69c00558 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -28,6 +28,9 @@
 #ifdef GDK_WINDOWING_X11
 #include 
 #include 
+#ifdef HAVE_LIBVA
+#include 
+#endif
 #endif
 #ifdef G_OS_WIN32
 #include 
@@ -2592,6 +2595,48 @@ 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(GDK_WINDOWING_X11) && defined(HAVE_LIBVA)
+if (g_strcmp0(context_type, "gst.vaapi.app.Display") == 0) {
+static Display *