[Spice-devel] [PATCH spice-gtk] gst: Fix detection of h264 stream decoder

[Pavel Grunt]

On Wed, 2017-07-19 at 15:23 +0200, Victor Toso wrote:
> Hi,
> 
> On Wed, Jul 19, 2017 at 02:59:00PM +0200, Pavel Grunt wrote:
> > GStreamer's avdec_h264 needs h264parse to be able to process H264 video
> > streams. However the check for elements through GstRegistry does not
> > take into account the possible pipeline of elements (like "h264parse !
> > avdec_h264").
> > 
> > Fix that by checking for the elements by their name.
> 
> As I mentioned before, given the extra complexity that this is
> requiring, I'm not convinced that it is a good fix because it relies on
> the fact that we know the elements that we need (hard coded).
> 
> 1-) An user might have a different subset of plugins, like the fluendo
>     ones or a different 3rd party ones. Do that work when we check for
>     h264parse avdec_h264? I don't think so as it should be different
>     plugins (one that people pay to have in their system)

This checks for presence of all the requirred plugins, for both h264parse and
avdec_h264. the user must have both to detect it.

It fixes the "regression" caused by the GstRegistry patch by checking for the
elements like it was done before (but without parsing the full pipeline)

> 
> 2-) If playbin just needs an extra plugin, we don't check it and that
>     means the pipeline will fail even if the caps were enabled.

It is about the plugin combinations already tested in the past. We have their
name (and their usage in a pipeline) in the header file

> 
> I do think the right approach is as I mentioned before, fix the
> filtering to show all decoders for h264 and that's it. If the pipeline
> fails, that's fine because it is *hard* to *ensure* that it'll work
> anyway without an actual video stream.
> 
> > ---
> >  src/channel-display-gst.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> > index 5042fc8..a9a044a 100644
> > --- a/src/channel-display-gst.c
> > +++ b/src/channel-display-gst.c
> > @@ -666,6 +666,31 @@ gboolean gstvideo_has_codec(int codec_type)
> >      codec_decoders = gst_element_factory_list_filter(all_decoders, caps,
> > GST_PAD_SINK, TRUE);
> >      gst_caps_unref(caps);
> > 
> > +    /* Check for the presence of decoding elements that could be filter
> > out.
> > +     * TODO: Improve filtering to avoid this...
> > +     */
> > +    {
> > +        GList *decoder_elements = NULL;
> > +        gchar **decoders_by_names;
> > +        guint i = 0;
> > +        decoders_by_names = g_strsplit(gst_opts[codec_type].dec_name, "!",
> > -1);
> > +        for (i = 0; decoders_by_names[i] != NULL; i++) {
> > +            GstElementFactory *element =
> > gst_element_factory_find(g_strstrip(decoders_by_names[i]));
> > +            if (element == NULL) {
> > +                gst_plugin_feature_list_free(decoder_elements);
> > +                decoder_elements = NULL;
> > +                break;
> > +            }
> > +            if (g_list_find(codec_decoders, element)) {
> > +                gst_object_unref(element);
> > +            } else {
> > +                decoder_elements = g_list_append(decoder_elements,
> > element);
> > +            }
> > +        }
> > +        codec_decoders = g_list_concat(codec_decoders, decoder_elements);
> > +        g_strfreev(decoders_by_names);
> > +    }
> > +
> >      if (codec_decoders == NULL) {
> >          spice_debug("From %u decoders, none can handle '%s'",
> >                      g_list_length(all_decoders),
> > gst_opts[codec_type].dec_caps);
> > -- 
> > 2.13.3
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel