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?
Is there a simple way to test the "streaming mode"? A test case like
server/tests/test-display-streaming would be great.
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
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel