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

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

 



On Wed, Apr 25, 2018 at 03:43:14PM +0300, Snir Sheriber 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.
> 
> Setting the DISABLE_GSTVIDEOOVERLAY environment variable will
> make gstreamer to avoid of using the gstvideooverlay interface.
> ---
>  src/channel-display-gst.c  | 92 +++++++++++++++++++++++++++++++++++-----------
>  src/channel-display-priv.h |  2 +
>  src/channel-display.c      | 35 ++++++++++++++++++
>  src/channel-display.h      |  3 ++
>  src/spice-marshal.txt      |  1 +
>  src/spice-widget.c         | 33 +++++++++++++++++
>  6 files changed, 145 insertions(+), 21 deletions(-)
> 
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index 0d7aabb..8029e34 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -41,6 +41,8 @@ typedef struct SpiceGstDecoder {
>      GstElement *pipeline;
>      GstClock *clock;
>  
> +    guintptr win_handle;
> +
>      /* ---------- Decoding and display queues ---------- */
>  
>      uint32_t last_mm_time;
> @@ -265,7 +267,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;
> @@ -306,6 +309,18 @@ 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: %" PRIuPTR ")", 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);
> +            }
> +        }
> +        break;
> +    }
>      default:
>          /* not being handled */
>          break;
> @@ -342,7 +357,6 @@ static void app_source_setup(GstElement *pipeline G_GNUC_UNUSED,
>  
>  static gboolean create_pipeline(SpiceGstDecoder *decoder)
>  {
> -    GstAppSinkCallbacks appsink_cbs = { NULL };
>      GstBus *bus;
>  #if GST_CHECK_VERSION(1,9,0)
>      GstElement *playbin, *sink;
> @@ -355,26 +369,56 @@ 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,
> +    /* 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
> +     */
> +    decoder->win_handle = get_window_handle(decoder->base.stream);
> +    SPICE_DEBUG("Creating Gstreamer pipline (handle for overlay %s)\n",
> +                decoder->win_handle ? "received" : "not received");
> +    if (decoder->win_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 gstvideoooverlay 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);
> +        }
> +    }

This part seems to set some gstreamer global state, I don't think this
needs to be done every time "create_pipeline()" is called. This looks
like this could go in gstvideo_init().

>  
>      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 */

[snip]

> diff --git a/src/spice-widget.c b/src/spice-widget.c
> index 73db593..251b29c 100644
> --- a/src/spice-widget.c
> +++ b/src/spice-widget.c
> +static void* prepare_streaming_mode(SpiceChannel *channel, bool streaming_mode, gpointer data)
> +{
> +#ifdef GDK_WINDOWING_X11
> +    SpiceDisplay *display = data;
> +    SpiceDisplayPrivate *d = display->priv;
> +
> +    /* GstVideoOverlay will currently be 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) {
> +#if GTK_CHECK_VERSION(2,18,0)
> +            if (gdk_window_ensure_native (window))
> +#endif
> +            {
> +                gtk_stack_set_visible_child_name(d->stack, "gst-area");
> +                return (void*)GDK_WINDOW_XID(window);
> +            }
> +        }
> +    }
> +#endif
> +    return NULL;
> +}
> +
>  static void invalidate(SpiceChannel *channel,
>                         gint x, gint y, gint w, gint h, gpointer data)
>  {
> @@ -2962,6 +2993,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, "streaming-mode",
> +                                      G_CALLBACK(prepare_streaming_mode), display, G_CONNECT_AFTER);

Rather than having this signal which is used by ChannelDisplay to call a
method on a GtkWidget it should not have any access to, have you
considered adding code to spice-widget.c::realize() and
spice-widget.c::unrealize() which would call
channel_display_set_window_handle(display->priv->display, handle);
?

Christophe

Attachment: signature.asc
Description: PGP signature

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