>
> From: Victor Toso <me@xxxxxxxxxxxxxx>
>
> The Playbin can provide the full pipeline which reduces the
> overall maintenance in the code as we don't need to track which
> decoder can work with our stream type.
>
> We need to maintain the GstCaps per SPICE_VIDEO_CODEC_TYPE in order to
> tell Playbin the type of data we expect. This much should be covered
> by spice-protocol and very likely we will need to extend it in the
> future to allow more settings that might not possible to verify at
> runtime.
>
> This patch keeps previous code for compatibility reasons.
>
> Note that we have to wait Playbin to emit "source-setup" in order to
> configure GstAppSrc with the capabilities of input stream. If in the
> unlikely event of frames arriving while GstAppSrc is not setup, we
> will drop those frames.
>
> Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> Signed-off-by: Victor Toso <me@xxxxxxxxxxxxxx>
> ---
> src/channel-display-gst.c | 97
> ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 95 insertions(+), 2 deletions(-)
>
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index dbdcef8..7e2b195 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -50,6 +50,9 @@ typedef struct SpiceGstDecoder {
> guint timer_id;
> } SpiceGstDecoder;
>
> +/* FIXME: With gstreamer version 1.9.0 and higher, we are using playbin to
> + * create the pipeline for us and for that reason we don't need to keep
> track of
> + * decoder's name anymore. */
> static struct {
> const gchar *dec_name;
> const gchar *dec_caps;
> @@ -82,6 +85,21 @@ G_STATIC_ASSERT(G_N_ELEMENTS(gst_opts) <=
> SPICE_VIDEO_CODEC_TYPE_ENUM_END);
> #define VALID_VIDEO_CODEC_TYPE(codec) \
> (codec > 0 && codec < G_N_ELEMENTS(gst_opts))
>
> +typedef enum {
> + GST_PLAY_FLAG_VIDEO = (1 << 0),
> + GST_PLAY_FLAG_AUDIO = (1 << 1),
> + GST_PLAY_FLAG_TEXT = (1 << 2),
> + GST_PLAY_FLAG_VIS = (1 << 3),
> + GST_PLAY_FLAG_SOFT_VOLUME = (1 << 4),
> + GST_PLAY_FLAG_NATIVE_AUDIO = (1 << 5),
> + GST_PLAY_FLAG_NATIVE_VIDEO = (1 << 6),
> + GST_PLAY_FLAG_DOWNLOAD = (1 << 7),
> + GST_PLAY_FLAG_BUFFERING = (1 << 8),
> + GST_PLAY_FLAG_DEINTERLACE = (1 << 9),
> + GST_PLAY_FLAG_SOFT_COLORBALANCE = (1 << 10),
> + GST_PLAY_FLAG_FORCE_FILTERS = (1 << 11),
> +} GstPlayFlags;
> +
__ __ __ ___ _______
| | | | | |/ / | ____|
| | | | | ' / | |__
| | | | | < | __|
| `----.| | | . \ | |____
|_______||__| |__|\__\ |_______|
> /* ---------- SpiceGstFrame ---------- */
>
> typedef struct SpiceGstFrame {
> @@ -297,13 +315,79 @@ static gboolean handle_pipeline_message(GstBus *bus,
> GstMessage *msg, gpointer v
> return TRUE;
> }
>
> +#if GST_CHECK_VERSION(1,9,0)
> +static void app_source_setup(GstElement *pipeline, GstElement *source,
> SpiceGstDecoder *decoder)
> +{
> + GstCaps *caps;
> +
> + /* - We schedule the frame display ourselves so set sync=false on
> appsink
> + * so the pipeline decodes them as fast as possible. This will also
> + * minimize the risk of frames getting lost when we rebuild the
> + * pipeline.
> + * - Set max-bytes=0 on appsrc so it does not drop frames that may be
> + * needed by those that follow.
> + */
> + caps =
> gst_caps_from_string(gst_opts[decoder->base.codec_type].dec_caps);
> + g_object_set(source,
> + "caps", caps,
> + "is-live", TRUE,
> + "format", GST_FORMAT_TIME,
> + "max-bytes", 0,
> + "block", TRUE,
> + NULL);
> + gst_caps_unref(caps);
> + decoder->appsrc = GST_APP_SRC(gst_object_ref(source));
> +}
> +#endif
> +
> static gboolean create_pipeline(SpiceGstDecoder *decoder)
> {
> - gchar *desc;
> GstAppSinkCallbacks appsink_cbs = { NULL };
> - GError *err = NULL;
> GstBus *bus;
> +#if GST_CHECK_VERSION(1,9,0)
> + GstElement *playbin, *sink;
> + gint flags;
> + GstCaps *caps;
>
> + playbin = gst_element_factory_make("playbin", "playbin");
> + if (playbin == NULL) {
> + spice_warning("error upon creation of 'playbin' element");
> + 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,
> + "caps", caps,
> + "sync", FALSE,
> + "drop", FALSE,
> + NULL);
> + gst_caps_unref(caps);
> +
> + g_object_set(playbin,
> + "uri", "appsrc://",
> + "video-sink", gst_object_ref(sink),
> + NULL);
> +
> + g_signal_connect(playbin, "source-setup", G_CALLBACK(app_source_setup),
> decoder);
> +
I would still put this before setting the uri property.
Documentation state that "This signal is usually emitted from the context
of a GStreamer streaming thread." which looks like "we are free to change in
the future".
> + /* Disable audio in playbin */
> + g_object_get(playbin, "flags", &flags, NULL);
> + flags &= ~(GST_PLAY_FLAG_AUDIO | GST_PLAY_FLAG_TEXT);
> + g_object_set(playbin, "flags", flags, NULL);
> +
> + decoder->appsrc = NULL;
if this was not NULL you have a leak, if this is NULL is a noop.
> + decoder->appsink = GST_APP_SINK(sink);
> + decoder->pipeline = playbin;
> +#else
> + gchar *desc;
> + GError *err = NULL;
>
> /* - We schedule the frame display ourselves so set sync=false on
> appsink
> * so the pipeline decodes them as fast as possible. This will also
> @@ -329,6 +413,7 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
>
> decoder->appsrc =
> GST_APP_SRC(gst_bin_get_by_name(GST_BIN(decoder->pipeline), "src"));
> decoder->appsink =
> GST_APP_SINK(gst_bin_get_by_name(GST_BIN(decoder->pipeline), "sink"));
> +#endif
>
> appsink_cbs.new_sample = new_sample;
> gst_app_sink_set_callbacks(decoder->appsink, &appsink_cbs, decoder,
> NULL);
> @@ -456,6 +541,14 @@ static gboolean
> spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
> return FALSE;
> }
>
> +#if GST_CHECK_VERSION(1,9,0)
> + if (decoder->appsrc == NULL) {
> + spice_warning("Error: Playbin has not yet initialized the Appsrc
> element");
> + stream_dropped_frame_on_playback(decoder->base.stream);
> + return TRUE;
> + }
> +#endif
> +
> /* ref() the frame data for the buffer */
> frame->ref_data(frame->data_opaque);
> GstBuffer *buffer =
> gst_buffer_new_wrapped_full(GST_MEMORY_FLAG_PHYSICALLY_CONTIGUOUS,
Otherwise looks good
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel