Re: [RFC spice-gtk 1/1] Gstreamer: Use GstVideoOverlay if possible

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

 



Hi,


On 07/24/2018 06:47 PM, Marc-André Lureau wrote:
Hi

On Sun, Mar 11, 2018 at 10:44 AM, Snir Sheriber <ssheribe@xxxxxxxxxx> wrote:
Currently when gstreamer is used to decode a full-screen
stream sent from the server, the decoding frames are being
forced to RBGA format and pushed using appsink to be scaled
and rendered to screen.

Today most of the gstreamer sinks supports the GstVideoOverlay
interface which allows to render directly from the pipeline to
a window by a given windowing system id (i.e. xid). This patch
makes playbin to use this feature if possible.
---
 src/channel-display-gst.c | 71 +++++++++++++++++++++++++++++++++++++----------
 src/channel-display.c     | 54 +++++++++++++++++++++++++++++++++++
 src/spice-widget.c        | 46 ++++++++++++++++++++++++++++++
 3 files changed, 156 insertions(+), 15 deletions(-)

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index c6280d3..6fa19e0 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -265,7 +265,8 @@ static void free_pipeline(SpiceGstDecoder *decoder)

     gst_element_set_state(decoder->pipeline, GST_STATE_NULL);
     gst_object_unref(decoder->appsrc);
-    gst_object_unref(decoder->appsink);
+    if (decoder->appsink)
+        gst_object_unref(decoder->appsink);
     gst_object_unref(decoder->pipeline);
     gst_object_unref(decoder->clock);
     decoder->pipeline = NULL;
@@ -308,7 +309,18 @@ static gboolean handle_pipeline_message(GstBus *bus, GstMessage *msg, gpointer v
         break;
     }
     default:
-        /* not being handled */
+        if (gst_is_video_overlay_prepare_window_handle_message(msg)) {
+            GstVideoOverlay *overlay;
+            guintptr handle = 0;
+
+            g_object_get(decoder->base.stream->channel, "handle", &handle, NULL);
+            SPICE_DEBUG("prepare-window-handle msg received (handle: %lu)", handle);
+            if (handle != 0) {
+                overlay = GST_VIDEO_OVERLAY(GST_MESSAGE_SRC(msg));
+                gst_video_overlay_set_window_handle(overlay, handle);
+            }
+        }
+        /* else- not being handled */
         break;
     }
     return TRUE;
@@ -348,6 +360,7 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
 #if GST_CHECK_VERSION(1,9,0)
     GstElement *playbin, *sink;
     SpiceGstPlayFlags flags;
+    guintptr handle;
     GstCaps *caps;

     playbin = gst_element_factory_make("playbin", "playbin");
@@ -356,26 +369,53 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
         return FALSE;
     }

-    sink = gst_element_factory_make("appsink", "sink");
-    if (sink == NULL) {
-        spice_warning("error upon creation of 'appsink' element");
-        gst_object_unref(playbin);
-        return FALSE;
-    }
-
-    caps = gst_caps_from_string("video/x-raw,format=BGRx");
-    g_object_set(sink,
+    g_object_get(decoder->base.stream->channel, "handle", &handle, NULL);
+    SPICE_DEBUG("Creating Gstreamer pipline (handle for overlay %s)\n",
+                handle ? "received" : "not received");
+    if (handle == 0) {
+        sink = gst_element_factory_make("appsink", "sink");
+        if (sink == NULL) {
+            spice_warning("error upon creation of 'appsink' element");
+            gst_object_unref(playbin);
+            return FALSE;
+        }
+        caps = gst_caps_from_string("video/x-raw,format=BGRx");
+        g_object_set(sink,
                  "caps", caps,
                  "sync", FALSE,
                  "drop", FALSE,
                  NULL);
-    gst_caps_unref(caps);
+        gst_caps_unref(caps);
+        g_object_set(playbin,
+                 "video-sink", gst_object_ref(sink),
+                 NULL);
+
+        decoder->appsink = GST_APP_SINK(sink);
+    } else {
+     /*
+      * handle has received, it means playbin will render directly into
+      * widget using the gstoverlay 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);

     g_object_set(playbin,
                  "uri", "appsrc://",
-                 "video-sink", gst_object_ref(sink),
                  NULL);

     /* Disable audio in playbin */
@@ -384,7 +424,6 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
     g_object_set(playbin, "flags", flags, NULL);

     g_warn_if_fail(decoder->appsrc == NULL);
-    decoder->appsink = GST_APP_SINK(sink);
     decoder->pipeline = playbin;
 #else
     gchar *desc;
@@ -417,7 +456,9 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
 #endif

     appsink_cbs.new_sample = new_sample;
-    gst_app_sink_set_callbacks(decoder->appsink, &appsink_cbs, decoder, NULL);
+    if (decoder->appsink) {
+        gst_app_sink_set_callbacks(decoder->appsink, &appsink_cbs, decoder, NULL);
+    }
     bus = gst_pipeline_get_bus(GST_PIPELINE(decoder->pipeline));
     gst_bus_add_watch(bus, handle_pipeline_message, decoder);
     gst_object_unref(bus);
diff --git a/src/channel-display.c b/src/channel-display.c
index b41093b..8e63764 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -70,6 +70,7 @@ struct _SpiceDisplayChannelPrivate {
     GArray                      *monitors;
     guint                       monitors_max;
     gboolean                    enable_adaptive_streaming;
+    guintptr                    handle;
     SpiceGlScanout scanout;
 };

@@ -83,6 +84,7 @@ enum {
     PROP_MONITORS,
     PROP_MONITORS_MAX,
     PROP_GL_SCANOUT,
+    PROP_HANDLE,
 };

 enum {
@@ -91,6 +93,7 @@ enum {
     SPICE_DISPLAY_INVALIDATE,
     SPICE_DISPLAY_MARK,
     SPICE_DISPLAY_GL_DRAW,
+    SPICE_DISPLAY_STREAM_AREA,

     SPICE_DISPLAY_LAST_SIGNAL,
 };
@@ -227,6 +230,10 @@ static void spice_display_get_property(GObject    *object,
         g_value_set_static_boxed(value, spice_display_channel_get_gl_scanout(channel));
         break;
     }
+    case PROP_HANDLE: {
+        g_value_set_ulong(value, c->handle);
+        break;
+    }
     default:
         G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
         break;
@@ -238,7 +245,14 @@ static void spice_display_set_property(GObject      *object,
                                        const GValue *value,
                                        GParamSpec   *pspec)
 {
+    SpiceDisplayChannel *channel = SPICE_DISPLAY_CHANNEL(object);
+    SpiceDisplayChannelPrivate *c = channel->priv;
+
     switch (prop_id) {
+    case PROP_HANDLE: {
+         c->handle = g_value_get_ulong(value);
+         break;
+    }
     default:
         G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
         break;
@@ -338,6 +352,16 @@ static void spice_display_channel_class_init(SpiceDisplayChannelClass *klass)
                             G_PARAM_READABLE |
                             G_PARAM_STATIC_STRINGS));

+    g_object_class_install_property
+         (gobject_class, PROP_HANDLE,
+         g_param_spec_ulong("handle",
+                            "Display handle",
+                            "Display handle",
+                            0, G_MAXULONG, 0,
+                            G_PARAM_WRITABLE |
+                            G_PARAM_READABLE |
+                            G_PARAM_STATIC_STRINGS));
+
     /**
      * SpiceDisplayChannel::display-primary-create:
      * @display: the #SpiceDisplayChannel that emitted the signal
@@ -454,6 +478,27 @@ static void spice_display_channel_class_init(SpiceDisplayChannelClass *klass)
                      4,
                      G_TYPE_UINT, G_TYPE_UINT, G_TYPE_UINT, G_TYPE_UINT);

+    /**
+     * SpiceDisplayChannel::stream-area:
+     * @display: the #SpiceDisplayChannel that emitted the signal
+     * @x: x position
+     * @y: y position
+     * @width: width
+     * @height: height
+     *
+     * The #SpiceDisplayChannel::stream-area signal is emitted when
+     * the rectangular region x/y/w/h is known as an area of a
+     * video stream to be received.
+     **/
+    signals[SPICE_DISPLAY_STREAM_AREA] =
+        g_signal_new("stream-area",
+                     G_OBJECT_CLASS_TYPE(gobject_class),
+                     0, 0, NULL, NULL,
+                     g_cclosure_user_marshal_VOID__UINT_UINT_UINT_UINT,
+                     G_TYPE_NONE,
+                     4,
+                     G_TYPE_UINT, G_TYPE_UINT, G_TYPE_UINT, G_TYPE_UINT);
+
     g_type_class_add_private(klass, sizeof(SpiceDisplayChannelPrivate));

     channel_set_handlers(SPICE_CHANNEL_CLASS(klass));
@@ -1261,6 +1306,15 @@ static display_stream *display_stream_create(SpiceChannel *channel, uint32_t sur
 #endif
     default:
 #ifdef HAVE_GSTVIDEO
+        if (c->primary->width == (dest->right - dest->left) &&
+            c->primary->height == (dest->bottom - dest->top)) {
+            SPICE_DEBUG("It seems to be a FULL-SCREEN stream");
+            /* stream area location is sent but currently not used */
+            g_coroutine_signal_emit(channel, signals[SPICE_DISPLAY_STREAM_AREA], 0,
+                                    dest->left, dest->top,
+                                    c->primary->width, c->primary->height);
+        //TODO: may not finished before creating decoder? also there is no fallback, assuming success
+        }
         st->video_decoder = create_gstreamer_decoder(codec_type, st);
 #endif
         break;
diff --git a/src/spice-widget.c b/src/spice-widget.c
index 1e7add4..73a77b7 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -612,6 +612,29 @@ G_GNUC_END_IGNORE_DEPRECATIONS
 #endif
 #endif

+static void
+gst_area_realize(GtkGLArea *area, gpointer user_data)
+{
+//TODO: needs rework, currently works only under X
+#ifdef GDK_WINDOWING_X11
+    SpiceDisplay *display = SPICE_DISPLAY(user_data);
+    SpiceDisplayPrivate *d = display->priv;
+    GdkWindow *window = gtk_widget_get_window(GTK_WIDGET(display));
+
+    if (window) {
+#if GTK_CHECK_VERSION(2,18,0)
+        if (!gdk_window_ensure_native (window)) {
+            g_warning("Couldn't create native window needed for GstVideoOverlay!");
+            return;
+        }
+#endif
+        g_object_set(G_OBJECT (d->display),
+                    "handle", GDK_WINDOW_XID(window),
+                    NULL);
+    }
+#endif
+}
+
 static void
 drawing_area_realize(GtkWidget *area, gpointer user_data)
 {
@@ -660,6 +683,13 @@ G_GNUC_BEGIN_IGNORE_DEPRECATIONS
 G_GNUC_END_IGNORE_DEPRECATIONS
 #endif
 #endif
+
+    area = gtk_drawing_area_new();
+    g_object_connect(area,
+                     "signal::realize", gst_area_realize, display,
+                     NULL);
+    gtk_stack_add_named(d->stack, area, "gst-area");
Was there a good reason to use a seperate drawing area rather than the
existing one?

There were some reasons for this, iirc there was an issue with the signals
that was needed to be disabled to avoid interference with the drawings (i
had a version where it uses the same drawing area but eventually it was
dropped) and also at first i was trying both gl-area and drawing-area so it was
easier to switch.
(Also please notice that this patch is not the recent version which was pushed)
 
Is there a simple way to test the "streaming mode"? A test case like
server/tests/test-display-streaming would be great.

Could be tested by using spice-streaming-agent that streams mjpeg to a client that was
built with disabled builtin_mjpeg (so it will use the spice-gst-decoder)
or
By using spice-streaming-agent that streams h264/vp8 video stream using the gst-plugin
which was recently sent to ML and it is still not pushed upstream.

Currently I'm not aware to any similar\other test option that is available upstream.

Snir.


Thanks

+
     gtk_widget_show_all(widget);

     g_signal_connect(display, "grab-broken-event", G_CALLBACK(grab_broken), NULL);
@@ -2589,6 +2619,20 @@ static void queue_draw_area(SpiceDisplay *display, gint x, gint y,
                                x, y, width, height);
 }

+static void pop_stream_area(SpiceChannel *channel, guint x, guint y,
+                            guint width, guint height, gpointer data)
+{
+/* Currently it works only in X11 environment - do not set area visible if it's not X11 */
+#ifdef GDK_WINDOWING_X11
+    SpiceDisplay *display = data;
+    SpiceDisplayPrivate *d = display->priv;
+
+    if (GDK_IS_X11_DISPLAY(gdk_display_get_default())) {
+        gtk_stack_set_visible_child_name(d->stack, "gst-area");
+    }
+#endif
+}
+
 static void invalidate(SpiceChannel *channel,
                        gint x, gint y, gint w, gint h, gpointer data)
 {
@@ -2953,6 +2997,8 @@ static void channel_new(SpiceSession *s, SpiceChannel *channel, gpointer data)
         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, "stream-area",
+                                      G_CALLBACK(pop_stream_area), display, 0);
         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.14.3

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

    

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