[Spice-devel] [PATCH spice-gtk 1/2] gstreamer: use custom playbin sink

[Frediano Ziglio]

> 
> Use custom playbin sink instead of just appsink.
> In order to allow playbin to choose decoders that requires
> more complex pipelines (e.g. pipeline that requires color
> space conversion & uses gl memory).
> The new sink composed as such:
> ... ! autovideoconvert ! gldownload ! appsink

Adding more plugins add more should add more contraints.
Why GStreamer is not able to choose the best combination?
What happens if the decoder is not able to provide gl texture?

> ---
>  src/channel-display-gst.c | 32 ++++++++++++++++++++++++--------
>  1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index f978602..af87fb5 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -38,6 +38,7 @@ typedef struct SpiceGstDecoder {
>  
>      GstAppSrc *appsrc;
>      GstAppSink *appsink;
> +    GstElement *sinkbin;
>      GstElement *pipeline;
>      GstClock *clock;
>  
> @@ -265,7 +266,7 @@ 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);
> +    gst_object_unref(decoder->sinkbin);
>      gst_object_unref(decoder->pipeline);
>      gst_object_unref(decoder->clock);
>      decoder->pipeline = NULL;
> @@ -346,7 +347,8 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
>      GstAppSinkCallbacks appsink_cbs = { NULL };
>      GstBus *bus;
>  #if GST_CHECK_VERSION(1,9,0)
> -    GstElement *playbin, *sink;
> +    GstElement *playbin, *sinkbin, *convert, *download, *appsink;
> +    GstPad *pad, *ghost_pad;
>      SpiceGstPlayFlags flags;
>      GstCaps *caps;
>  
> @@ -356,26 +358,39 @@ 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");
> +    convert = gst_element_factory_make("autovideoconvert",
> "autovideoconvert");
> +    download = gst_element_factory_make("gldownload", "gldownload");
> +    appsink = gst_element_factory_make("appsink", "appsink");
> +    if ( !appsink || !convert || !download ) {
> +        spice_warning("error upon creation of one of the bin elements");
>          gst_object_unref(playbin);

This is potentially leaking, won't be better to use string based pipeline
instead of manually build like this?

>          return FALSE;
>      }



>  
>      caps = gst_caps_from_string("video/x-raw,format=BGRx");
> -    g_object_set(sink,
> +    g_object_set(appsink,
>                   "caps", caps,
>                   "sync", FALSE,
>                   "drop", FALSE,
>                   NULL);
>      gst_caps_unref(caps);
>  
> +    /* Create advanced playbin sink by binning the appsink, gldownload and

binning? Do you mean "binding"?

> +     * autovideoconvert elements together */
> +    sinkbin = gst_bin_new("sink_bin");
> +    gst_bin_add_many (GST_BIN (sinkbin), convert, download, appsink, NULL);
> +    gst_element_link_many(convert, download, appsink, NULL);
> +    pad = gst_element_get_static_pad (convert, "sink");
> +    ghost_pad = gst_ghost_pad_new ("sink", pad);
> +    gst_pad_set_active (ghost_pad, TRUE);
> +    gst_element_add_pad (sinkbin, ghost_pad);
> +    gst_object_unref (pad);

style

> +
>      g_signal_connect(playbin, "source-setup", G_CALLBACK(app_source_setup),
>      decoder);
>  
>      g_object_set(playbin,
>                   "uri", "appsrc://",
> -                 "video-sink", gst_object_ref(sink),
> +                 "video-sink", gst_object_ref(sinkbin),
>                   NULL);
>  
>      /* Disable audio in playbin */
> @@ -384,7 +399,8 @@ 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->appsink = GST_APP_SINK(appsink);
> +    decoder->sinkbin = sinkbin;
>      decoder->pipeline = playbin;
>  #else
>      gchar *desc;

Frediano