[Spice-devel] [PATCH v2 spice-gtk 1/2] Gstreamer: Control GstVideoOverlay from the widget

Snir Sheriber ssheribe at redhat.com
Sun Dec 16 15:18:53 UTC 2018


This patch is changing the way GstVideoOverlay is being set.
Once pipeline is created a pointer is passed to the widget using
GObject signal, so we can set there the overlay interface and call
its functions from widget callbacks. This allows to solve issues
like resizing the window.

Signed-off-by: Snir Sheriber <ssheribe at redhat.com>
---

Main changes from previous patch (according to comments):
-Split patch
-Move to weak refs
-Do not return window handle back to decoder


Since unref of the overlay element is done from gstreamer's
thread it is not safe to use GObject's weak ptr for it from 
other threads, this is the reason i used GWeakRef for it.
AFAIU for the pipeline it should be fine to use regular weak
reference so i used weak pointer. (would be better to be
consistent?)

---
 src/channel-display-gst.c  | 30 ++++------------
 src/channel-display-priv.h |  7 ++++
 src/channel-display.c      | 40 ++++++++++++---------
 src/spice-marshal.txt      |  2 +-
 src/spice-widget-priv.h    |  8 +++++
 src/spice-widget.c         | 73 ++++++++++++++++++++++++++++++++------
 6 files changed, 108 insertions(+), 52 deletions(-)

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 2c07f35..767f6b1 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -43,8 +43,6 @@ typedef struct SpiceGstDecoder {
     GstElement *pipeline;
     GstClock *clock;
 
-    guintptr win_handle;
-
     /* ---------- Decoding and display queues ---------- */
 
     uint32_t last_mm_time;
@@ -334,20 +332,6 @@ static gboolean handle_pipeline_message(GstBus *bus, GstMessage *msg, gpointer v
         g_free(filename);
         break;
     }
-    case GST_MESSAGE_ELEMENT: {
-        if (gst_is_video_overlay_prepare_window_handle_message(msg)) {
-            GstVideoOverlay *overlay;
-
-            SPICE_DEBUG("prepare-window-handle msg received (handle: %" G_GUINTPTR_FORMAT")",
-                        decoder->win_handle);
-            if (decoder->win_handle != 0) {
-                overlay = GST_VIDEO_OVERLAY(GST_MESSAGE_SRC(msg));
-                gst_video_overlay_set_window_handle(overlay, decoder->win_handle);
-                gst_video_overlay_handle_events(overlay, false);
-            }
-        }
-        break;
-    }
     default:
         /* not being handled */
         break;
@@ -396,14 +380,11 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
         return FALSE;
     }
 
-    /* Will try to get window handle in order to apply the GstVideoOverlay
-     * interface, setting overlay to this window will happen only when
-     * prepare-window-handle message is received
+    /* Passing the pipeline to widget, try to get window handle and
+     * set the GstVideoOverlay interface, setting overlay to the window
+     * will happen only when prepare-window-handle message is received
      */
-    decoder->win_handle = get_window_handle(decoder->base.stream);
-    SPICE_DEBUG("Creating Gstreamer pipeline (handle for overlay %s)\n",
-                decoder->win_handle ? "received" : "not received");
-    if (decoder->win_handle == 0) {
+    if (!hand_pipeline_to_widget(decoder->base.stream, GST_PIPELINE(playbin))) {
         sink = gst_element_factory_make("appsink", "sink");
         if (sink == NULL) {
             spice_warning("error upon creation of 'appsink' element");
@@ -432,6 +413,7 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
         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");
@@ -576,7 +558,7 @@ static void spice_gst_decoder_destroy(VideoDecoder *video_decoder)
  * 3) As soon as the GStreamer pipeline no longer needs the compressed frame it
  *    will call frame->unref_data() to free it.
  *
- * If GstVideoOverlay is used (win_handle was obtained by pipeline creation):
+ * If GstVideoOverlay is used (window handle was obtained successfully at the widget):
  *   4) Decompressed frames will be renderd to widget directly from gstreamer's pipeline
  *      using some gstreamer sink plugin which implements the GstVideoOverlay interface
  *      (last step).
diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
index c1b3fe5..8d9ff8d 100644
--- a/src/channel-display-priv.h
+++ b/src/channel-display-priv.h
@@ -32,6 +32,10 @@
 #include "common/quic.h"
 #include "common/rop3.h"
 
+#ifdef HAVE_GSTVIDEO
+#include "gst/gst.h"
+#endif
+
 G_BEGIN_DECLS
 
 typedef struct display_stream display_stream;
@@ -202,6 +206,9 @@ void stream_dropped_frame_on_playback(display_stream *st);
 #define SPICE_UNKNOWN_STRIDE 0
 void stream_display_frame(display_stream *st, SpiceFrame *frame, uint32_t width, uint32_t height, int stride, uint8_t* data);
 guintptr get_window_handle(display_stream *st);
+#ifdef HAVE_GSTVIDEO
+gboolean hand_pipeline_to_widget(display_stream *st,  GstPipeline *pipeline);
+#endif
 
 
 G_END_DECLS
diff --git a/src/channel-display.c b/src/channel-display.c
index 7c663cb..6b6a172 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -29,6 +29,7 @@
 #include "spice-session-priv.h"
 #include "channel-display-priv.h"
 #include "decode.h"
+#include "gst/gst.h"
 
 /**
  * SECTION:channel-display
@@ -88,7 +89,7 @@ enum {
     SPICE_DISPLAY_INVALIDATE,
     SPICE_DISPLAY_MARK,
     SPICE_DISPLAY_GL_DRAW,
-    SPICE_DISPLAY_STREAMING_MODE,
+    SPICE_DISPLAY_OVERLAY,
 
     SPICE_DISPLAY_LAST_SIGNAL,
 };
@@ -453,26 +454,27 @@ static void spice_display_channel_class_init(SpiceDisplayChannelClass *klass)
                      G_TYPE_UINT, G_TYPE_UINT, G_TYPE_UINT, G_TYPE_UINT);
 
     /**
-     * SpiceDisplayChannel::streaming-mode:
+     * SpiceDisplayChannel::gst-video-overlay
      * @display: the #SpiceDisplayChannel that emitted the signal
-     * @streaming_mode: %TRUE when it's streaming mode
+     * @pipeline: pointer to gstreamer's pipeline
      *
-     * Return: handle for the display window if possible
+     * Return: valid window handle on success
      *
-     * The #SpiceDisplayChannel::streaming-mode signal is emitted when
-     * spice server is working in streaming mode.
+     * The #SpiceDisplayChannel::gst-video-overlay signal is emitted when
+     * pipeline is ready and can be passed to widget to regeister gstreamer
+     * overlay interface and other gstreamer callbacks.
      *
-     * Since 0.35
+     * Since 0.36
      **/
-    signals[SPICE_DISPLAY_STREAMING_MODE] =
-        g_signal_new("streaming-mode",
+    signals[SPICE_DISPLAY_OVERLAY] =
+        g_signal_new("gst-video-overlay",
                      G_OBJECT_CLASS_TYPE(gobject_class),
                      0, 0,
                      NULL, NULL,
-                     g_cclosure_user_marshal_POINTER__BOOLEAN,
-                     G_TYPE_POINTER,
+                     g_cclosure_user_marshal_BOOLEAN__POINTER,
+                     G_TYPE_BOOLEAN,
                      1,
-                     G_TYPE_BOOLEAN);
+                     GST_TYPE_PIPELINE);
 
     channel_set_handlers(SPICE_CHANNEL_CLASS(klass));
 }
@@ -1391,14 +1393,18 @@ void stream_display_frame(display_stream *st, SpiceFrame *frame,
     }
 }
 
-guintptr get_window_handle(display_stream *st)
+#ifdef HAVE_GSTVIDEO
+gboolean hand_pipeline_to_widget(display_stream *st, GstPipeline *pipeline)
 {
-   void* handle = 0;
+    gboolean res = false;
 
-   g_signal_emit(st->channel, signals[SPICE_DISPLAY_STREAMING_MODE], 0,
-                 st->surface->streaming_mode, &handle);
-   return st->surface->streaming_mode ? (guintptr)handle : 0;
+    if (st->surface->streaming_mode) {
+        g_signal_emit(st->channel, signals[SPICE_DISPLAY_OVERLAY], 0,
+                      pipeline, &res);
+    }
+    return res;
 }
+#endif
 
 /* after a sequence of 3 drops, push a report to the server, even
  * if the report window is bigger */
diff --git a/src/spice-marshal.txt b/src/spice-marshal.txt
index b3a8e69..92087c5 100644
--- a/src/spice-marshal.txt
+++ b/src/spice-marshal.txt
@@ -13,4 +13,4 @@ BOOLEAN:UINT,POINTER,UINT
 BOOLEAN:UINT,UINT
 VOID:OBJECT,OBJECT
 VOID:BOXED,BOXED
-POINTER:BOOLEAN
+BOOLEAN:POINTER
diff --git a/src/spice-widget-priv.h b/src/spice-widget-priv.h
index 96f6c1d..651d306 100644
--- a/src/spice-widget-priv.h
+++ b/src/spice-widget-priv.h
@@ -32,6 +32,10 @@
 #include "spice-common.h"
 #include "spice-gtk-session.h"
 
+#ifdef HAVE_GSTVIDEO
+#include <gst/video/videooverlay.h>
+#endif
+
 G_BEGIN_DECLS
 
 #define DISPLAY_DEBUG(display, fmt, ...) \
@@ -149,6 +153,10 @@ struct _SpiceDisplayPrivate {
     } egl;
 #endif // HAVE_EGL
     double scroll_delta_y;
+#ifdef HAVE_GSTVIDEO
+    GWeakRef                overlay_element_weak_ref;
+    GstPipeline             *pipeline;
+#endif
 };
 
 int      spice_cairo_image_create                 (SpiceDisplay *display);
diff --git a/src/spice-widget.c b/src/spice-widget.c
index 9bb4221..ae83420 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -2115,10 +2115,18 @@ static void realize(GtkWidget *widget)
 
 static void unrealize(GtkWidget *widget)
 {
-    spice_cairo_image_destroy(SPICE_DISPLAY(widget));
+    SpiceDisplay *display = SPICE_DISPLAY(widget);
+
+    spice_cairo_image_destroy(display);
 #if HAVE_EGL
-    if (SPICE_DISPLAY(widget)->priv->egl.context_ready)
-        spice_egl_unrealize_display(SPICE_DISPLAY(widget));
+    if (display->priv->egl.context_ready)
+        spice_egl_unrealize_display(display);
+#endif
+#ifdef HAVE_GSTVIDEO
+    SpiceDisplayPrivate *d = display->priv;
+
+    g_weak_ref_set(&d->overlay_element_weak_ref, NULL);
+    g_clear_weak_pointer(&d->pipeline);
 #endif
 
     GTK_WIDGET_CLASS(spice_display_parent_class)->unrealize(widget);
@@ -2547,26 +2555,71 @@ static void queue_draw_area(SpiceDisplay *display, gint x, gint y,
                                x, y, width, height);
 }
 
-static void* prepare_streaming_mode(SpiceChannel *channel, bool streaming_mode, gpointer data)
+#if defined(HAVE_GSTVIDEO) && defined(GDK_WINDOWING_X11)
+static void gst_sync_bus_call(GstBus *bus, GstMessage *msg, gpointer data)
+{
+    switch(GST_MESSAGE_TYPE(msg)) {
+    case GST_MESSAGE_ELEMENT: {
+        if (gst_is_video_overlay_prepare_window_handle_message(msg)) {
+            if (!g_getenv("DISABLE_GSTVIDEOOVERLAY") &&
+                GDK_IS_X11_DISPLAY(gdk_display_get_default())) {
+                SpiceDisplay *display = data;
+                GdkWindow *window = gtk_widget_get_window(GTK_WIDGET(display));
+
+                if (window && gdk_window_ensure_native(window)) {
+                    SpiceDisplayPrivate *d = display->priv;
+                    GstElement *overlay_element;
+
+                    g_weak_ref_set(&d->overlay_element_weak_ref, GST_ELEMENT(GST_MESSAGE_SRC(msg)));
+                    overlay_element = g_weak_ref_get(&d->overlay_element_weak_ref);
+                    gst_video_overlay_set_window_handle(GST_VIDEO_OVERLAY(overlay_element), (uintptr_t)GDK_WINDOW_XID(window));
+                    gst_video_overlay_handle_events(GST_VIDEO_OVERLAY(overlay_element), false);
+                    gst_object_unref(overlay_element);
+                    return;
+                }
+            }
+        }
+        break;
+    }
+    default:
+        /* not being handled */
+        break;
+    }
+}
+#endif
+
 {
 #ifdef GDK_WINDOWING_X11
+/* This callback should pass to the widget a pointer of the pipeline
+ * so that we can set pipeline and overlay related calls from here.
+ */
+static gboolean set_overlay(SpiceChannel *channel, void* pipeline_ptr, gpointer data)
+{
+#if defined(HAVE_GSTVIDEO) && defined(GDK_WINDOWING_X11)
     SpiceDisplay *display = data;
     SpiceDisplayPrivate *d = display->priv;
 
-    /* GstVideoOverlay will currently be used only under x */
+    /* GstVideoOverlay is currently used only under x */
     if (!g_getenv("DISABLE_GSTVIDEOOVERLAY") &&
-        streaming_mode &&
         GDK_IS_X11_DISPLAY(gdk_display_get_default())) {
         GdkWindow *window;
 
         window = gtk_widget_get_window(GTK_WIDGET(display));
         if (window && gdk_window_ensure_native(window)) {
+            GstBus *bus;
+
             gtk_stack_set_visible_child_name(d->stack, "gst-area");
-            return (void*)(uintptr_t)GDK_WINDOW_XID(window);
+            d->pipeline = pipeline_ptr;
+            g_object_add_weak_pointer(pipeline_ptr, (gpointer*)&(d->pipeline)); // Taking a weak ref although pipeline is not used again
+            bus = gst_pipeline_get_bus(GST_PIPELINE(d->pipeline));
+            gst_bus_enable_sync_message_emission(bus);
+            g_signal_connect (bus, "sync-message", G_CALLBACK (gst_sync_bus_call), data);
+            gst_object_unref(bus);
+            return true;
         }
     }
 #endif
-    return NULL;
+    return false;
 }
 
 static void invalidate(SpiceChannel *channel,
@@ -2936,8 +2989,8 @@ static void channel_new(SpiceSession *s, SpiceChannel *channel, SpiceDisplay *di
         spice_g_signal_connect_object(channel, "notify::monitors",
                                       G_CALLBACK(spice_display_widget_update_monitor_area),
                                       display, G_CONNECT_AFTER | G_CONNECT_SWAPPED);
-        spice_g_signal_connect_object(channel, "streaming-mode",
-                                      G_CALLBACK(prepare_streaming_mode), display, G_CONNECT_AFTER);
+        spice_g_signal_connect_object(channel, "gst-video-overlay",
+                                      G_CALLBACK(set_overlay), display, G_CONNECT_AFTER);
         if (spice_display_channel_get_primary(channel, 0, &primary)) {
             primary_create(channel, primary.format, primary.width, primary.height,
                            primary.stride, primary.shmid, primary.data, display);
-- 
2.19.1



More information about the Spice-devel mailing list