[Spice-devel] [RFC spice-gtk 1/1] Gstreamer: Use GstVideoOverlay if possible
Snir Sheriber
ssheribe at redhat.com
Thu Jul 26 06:58:55 UTC 2018
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 at redhat.com> 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180726/fcf50aa5/attachment-0001.html>
More information about the Spice-devel
mailing list