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