On Tue, May 16, 2017 at 04:48:17PM +0200, Victor Toso wrote:
> From: Victor Toso <me@xxxxxxxxxxxxxx>
>
> With this patch, we can find all the elements in the registry that are
> video decoders which can handle the predefined GstCaps.
>
> Main benefits are:
> - We don't rely on predefined list of GstElements. We don't need to
> update them;
"on a predefined list of GstElements"
> - debugging: It does help to know what the registry has at runtime;
>
> Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> Acked-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> Signed-off-by: Victor Toso <me@xxxxxxxxxxxxxx>
> ---
> src/channel-display-gst.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index fa0ec90..e675062 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -524,6 +524,54 @@ VideoDecoder* create_gstreamer_decoder(int codec_type, display_stream *stream)
> G_GNUC_INTERNAL
> gboolean gstvideo_has_codec(int codec_type)
> {
> +#if GST_CHECK_VERSION(1,9,0)
Why is this limited to gstreamer 1.9 and newer? (a small note in the
comit log could probably be helpful)
> + GList *all_decoders, *codec_decoders;
> + GstCaps *caps;
> + GstElementFactoryListType type;
> +
> + g_return_val_if_fail(gstvideo_init(), FALSE);
> + g_return_val_if_fail(VALID_VIDEO_CODEC_TYPE(codec_type), FALSE);
> +
> + type = GST_ELEMENT_FACTORY_TYPE_DECODER | GST_ELEMENT_FACTORY_TYPE_MEDIA_VIDEO;
> + all_decoders = gst_element_factory_list_get_elements(type, GST_RANK_NONE);
> + if (all_decoders == NULL) {
> + spice_warning("No video decoders from GStreamer were found");
> + return FALSE;
> + }
> +
> + caps = gst_caps_from_string(gst_opts[codec_type].dec_caps);
> + codec_decoders = gst_element_factory_list_filter(all_decoders, caps, GST_PAD_SINK, TRUE);
> + gst_caps_unref(caps);
> +
> + if (codec_decoders == NULL) {
> + spice_debug("From %u decoders, none can handle '%s'",
> + g_list_length(all_decoders), gst_opts[codec_type].dec_caps);
> + gst_plugin_feature_list_free(all_decoders);
> + return FALSE;
> + }
> +
> + if (spice_util_get_debug()) {
I would split this debugging code in a separate helper, it's as long as
the main code.
> + GList *l;
> + GString *msg = g_string_new(NULL);
> + /* Print list of available decoders to make debugging easier */
> + g_string_printf(msg, "From %3u video decoder elements, %2u can handle caps %12s: ",
> + g_list_length(all_decoders), g_list_length(codec_decoders),
> + gst_opts[codec_type].dec_caps);
> +
> + for (l = codec_decoders; l != NULL; l = l->next) {
> + GstPluginFeature *pfeat = GST_PLUGIN_FEATURE(l->data);
> + g_string_append_printf(msg, "%s, ", gst_plugin_feature_get_name(pfeat));
> + }
> +
> + msg->str[msg->len - 2] = '\0';
I would replace this with
/* Drop trailing ", " */
g_string_truncate(msg, msg->len - 2);
> + spice_debug("%s", msg->str);
> + g_string_free(msg, TRUE);
> + }
> +
> + gst_plugin_feature_list_free(codec_decoders);
> + gst_plugin_feature_list_free(all_decoders);
> + return TRUE;
> +#else
> gboolean has_codec = FALSE;
>
> VideoDecoder *decoder = create_gstreamer_decoder(codec_type, NULL);
If we don't attempt to call create_gstreamer_decoder, aren't we
restricting our testing to a smaller subset of what we used to be doing
(ie test if we can create a pipeline like appsrc ! $caps ! $decoder !
videoconvert ! appsink)? If appsrc or videoconvert are missing, are we
going to catch this after your changes?
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel