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 04/06/2018 04:12 PM, Frediano Ziglio 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.
Got some time to test, works correctly. Finally I manage to test wayland
and different resolutions/settings.

I have some just minor changes (attached).

Another change to use the new flag would be (didn't still test it):


diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
index 6a90a78..c44f94e 100644
--- a/src/channel-display-priv.h
+++ b/src/channel-display-priv.h
@@ -98,6 +98,7 @@ gboolean gstvideo_has_codec(int codec_type);
  typedef struct display_surface {
      guint32                     surface_id;
      bool                        primary;
+    bool                        streaming_mode;
      enum SpiceSurfaceFmt        format;
      int                         width, height, stride, size;
      uint8_t                     *data;
diff --git a/src/channel-display.c b/src/channel-display.c
index a58119d..7d639b5 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1292,9 +1292,7 @@ static display_stream *display_stream_create(SpiceChannel *channel,
  #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");
+        if (c->primary->streaming_mode) {
              /* stream area location is sent but currently not used */
              g_coroutine_signal_emit(channel, signals[SPICE_DISPLAY_STREAM_AREA], 0,
                                      dest->left, dest->top,
@@ -1829,9 +1827,10 @@ static void display_handle_surface_create(SpiceChannel *channel, SpiceMsgIn *in)
      surface->height = create->height;
      surface->stride = create->width * 4;
      surface->size   = surface->height * surface->stride;
+    surface->streaming_mode = !!(create->flags & SPICE_SURFACE_FLAGS_STREAMING_MODE);

      if (create->flags & SPICE_SURFACE_FLAGS_PRIMARY) {
-        SPICE_DEBUG("primary flags: %x", create->flags);
+        SPICE_DEBUG("surface flags: %x", create->flags);
          surface->primary = true;
          create_canvas(channel, surface);
          if (c->mark_false_event_id != 0) {


Thanks!

I have also made a similar change that uses the flag, seems to work fine.
And then i noticed that if we use the overlay interface we can also avoid
of allocating the canvas, but then if there is no canvas it's a bit more
complicated to fallback in case the overlay interface cannot be applied.
I hope I'll be able to make it work without too many changes, if not I'll
leave it that way.

Snir.

Frediano

---
  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");
+
      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);

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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]