[Spice-devel] [RFC spice-gtk v3 1/1] Gstreamer: Use GstVideoOverlay if possible
Christophe Fergeau
cfergeau at redhat.com
Tue May 15 15:09:14 UTC 2018
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180515/06124cde/attachment.sig>
More information about the Spice-devel
mailing list