Re: gst-plugins-base: decodebin2: Don' t use the same element multiple times in the same chain

[Jan Schmidt-6]

Hi,

In the past, we've seen files that have at least 2 layers of id3, or
even id3 wrapped around avi and the streams contain id3. This commit
sounds like it will break those.

J.

On Sat, 2011-02-26 at 14:41 -0800, Sebastian Dröge wrote:

> Module: gst-plugins-base
> Branch: master
> Commit: 5058f792263e4214f4655c5ed5956ebfbeb17c99
> URL:    http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=5058f792263e4214f4655c5ed5956ebfbeb17c99
>
> Author: Sebastian Dröge <[hidden email]>
> Date:   Sat Feb 26 23:39:03 2011 +0100
>
> decodebin2: Don't use the same element multiple times in the same chain
>
> This is going to lead to an infinite loop of this element and can easily
> happen with parsers that accept their own src caps on the sinkpad.
>
> ---
>
>  gst/playback/gstdecodebin2.c |   25 +++++++++++++++++++++++++
>  1 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/gst/playback/gstdecodebin2.c b/gst/playback/gstdecodebin2.c
> index caa3c42..339ec91 100644
> --- a/gst/playback/gstdecodebin2.c
> +++ b/gst/playback/gstdecodebin2.c
> @@ -1667,6 +1667,8 @@ connect_pad (GstDecodeBin * dbin, GstElement * src, GstDecodePad * dpad,
>      GstElement *element;
>      GstPad *sinkpad;
>      gboolean subtitle;
> +    gboolean skip = FALSE;
> +    GList *l;
>  
>      /* Set dpad target to pad again, it might've been unset
>       * below but we came back here because something failed
> @@ -1678,6 +1680,29 @@ connect_pad (GstDecodeBin * dbin, GstElement * src, GstDecodePad * dpad,
>      /* Remove selected factory from the list. */
>      g_value_array_remove (factories, 0);
>  
> +    /* First check if this is a factory for which we have already
> +     * created an element in this chain. If we have, we skip this
> +     * factory because it will lead to an infinite loop. Examples
> +     * for this are parsers that accept their own src caps on the
> +     * sinkpad again.
> +     */
> +    CHAIN_MUTEX_LOCK (chain);
> +    for (l = chain->elements; l; l = l->next) {
> +      GstElement *otherelement = GST_ELEMENT_CAST (l->data);
> +
> +      if (gst_element_get_factory (otherelement) == factory) {
> +        skip = TRUE;
> +        continue;
> +      }
> +    }
> +    CHAIN_MUTEX_UNLOCK (chain);
> +    if (skip) {
> +      GST_DEBUG_OBJECT (dbin,
> +          "Skipping factory '%s' because it was already used in this chain",
> +          gst_plugin_feature_get_name (GST_PLUGIN_FEATURE_CAST (factory)));
> +      continue;
> +    }
> +
>      /* emit autoplug-select to see what we should do with it. */
>      g_signal_emit (G_OBJECT (dbin),
>          gst_decode_bin_signals[SIGNAL_AUTOPLUG_SELECT],
>
> _______________________________________________
> gstreamer-commits mailing list
> [hidden email]
> http://lists.freedesktop.org/mailman/listinfo/gstreamer-commits

--
Jan Schmidt <[hidden email]>

_______________________________________________
gstreamer-devel mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

[Sebastian Dröge-7]

On Sun, 2011-02-27 at 23:43 +1100, Jan Schmidt wrote:
> Hi,
>
> In the past, we've seen files that have at least 2 layers of id3, or
> even id3 wrapped around avi and the streams contain id3. This commit
> sounds like it will break those.

The commit would break two layers of id3 but not mp3 with id3 inside avi
with another id3 layer.

I've pushed 4ac6f5ff831fe1ee9b7005c707c6ea1527240ddb to fix this, now
only parsers are prevented to be plugged twice in the same chain. This
should be a good fix because only parsers are not going to change the
data and as such should be the only element class that can cause the
infinite loop behaviour.

_______________________________________________
gstreamer-devel mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/gstreamer-devel