[Spice-devel] [RFC spice-gtk 1/1] Gstreamer: Use GstVideoOverlay if possible
Snir Sheriber
ssheribe at redhat.com
Sun Apr 8 08:12:12 UTC 2018
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);
More information about the Spice-devel
mailing list