[Spice-devel] [PATCH spice-gtk v4 5/7] display-gst: check GstRegistry for decoding elements

[Victor Toso]

Hi,

On Wed, Jul 05, 2017 at 02:08:37AM +0000, Daimon Wang wrote:
> Hi.
> >G_GNUC_INTERNAL
> >gboolean gstvideo_has_codec(int codec_type)
> >{
> >-    gboolean has_codec = FALSE;
> >+    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);>
> >
> >-    VideoDecoder *decoder = create_gstreamer_decoder(codec_type, NULL);
> >-    if (decoder) {
> >-        has_codec = TRUE;
> >-        decoder->destroy(decoder);
> >+    type = GST_ELEMENT_FACTORY_TYPE_DECODER | GST_ELEMENT_FACTORY_TYPE_MEDIA_VIDEO;
> >+    all_decoders = gst_element_factory_list_get_elements(type, GST_RANK_NONE);
> Playbin is using GST_RANK_MARGINAL to filter its elements. Maybe we could follow that?

Why exactly?

* Documentation for gst_element_factory_list_get_elements()

 "Get a list of factories that match the given type . Only elements with
  a rank greater or equal to minrank will be returned. The list of
  factories is returned by decreasing rank."

In my system, running:
$ gst-inspect-1.0 -a | grep -i rank | grep -i none | wc -l
827

That would mean 827 (about 50%) elements being removed.

Not sure about the playbin code you have mention, but here we want to
check in the GstRegistry if there is any element that can decode our
media data and we should check all elements for that.

    toso

>
>     On Wednesday, June 28, 2017 8:43 PM, Victor Toso <victortoso at redhat.com> wrote:
>  
> 
>  From: Victor Toso <me at victortoso.com>
> 
> 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 a predefined list of GstElements. We don't need to
>   update them;
> - debugging: It does help to know what the registry has at runtime;
> 
> Signed-off-by: Victor Toso <victortoso at redhat.com>
> Acked-by: Frediano Ziglio <fziglio at redhat.com>
> Signed-off-by: Victor Toso <me at victortoso.com>
> ---
>  src/channel-display-gst.c | 56 ++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 50 insertions(+), 6 deletions(-)
> 
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index 57f0eb7..1b40002 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -521,16 +521,60 @@ VideoDecoder* create_gstreamer_decoder(int codec_type, display_stream *stream)
>     return (VideoDecoder*)decoder;
>  }
>  
> +static void gstvideo_debug_available_decoders(int codec_type,
> +                                              GList *all_decoders,
> +                                              GList *codec_decoders)
> +{
> +    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));
> +    }
> +
> +    /* Drop trailing ", " */
> +    g_string_truncate(msg, msg->len - 2);
> +    spice_debug("%s", msg->str);
> +    g_string_free(msg, TRUE);
> +}
> +
>  G_GNUC_INTERNAL
>  gboolean gstvideo_has_codec(int codec_type)
>  {
> -    gboolean has_codec = FALSE;
> +    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);
>  
> -    VideoDecoder *decoder = create_gstreamer_decoder(codec_type, NULL);
> -    if (decoder) {
> -        has_codec = TRUE;
> -        decoder->destroy(decoder);
> +    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;
>     }
>  
> -    return has_codec;
> +    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())
> +        gstvideo_debug_available_decoders(codec_type, all_decoders, codec_decoders);
> +
> +    gst_plugin_feature_list_free(codec_decoders);
> +    gst_plugin_feature_list_free(all_decoders);
> +    return TRUE;
>  }
> -- 
> 2.13.0
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> 
>    
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170710/5d95a885/attachment.sig>