[gstreamer-bugs] [Bug 374682] [PATCH] collectpads should only consider linked pads to prevent blocking

[GStreamer (bugzilla.gnome.org)]

Do not reply to this via email (we are currently unable to handle email
responses and they get discarded).  You can add comments to this bug at
http://bugzilla.gnome.org/show_bug.cgi?id=374682

  GStreamer | gstreamer (core) | Ver: 0.10.10





------- Comment #4 from René Stadler  2006-11-13 17:49 UTC -------
(In reply to comment #3)
[...]
> I have no specific situation, but would occur e.g. when unlinking a pad from
> videomixer (if the stream in question no longer needs mixing in), and not
> linking to another pad.

Doing so would be a severe bug on the application side.  It has to release the
pad it requested.  If this does not work correctly, we should look at
videomixer (and adder).  It might be more complicated in the case of muxers.

> In short, the 'safety' (against blocking; with indeed other effects) this patch
> tries to bring would probably not be needed *if*:
> - (permanently) unlinking a (typically request) pad would be consistently
> followed by releasing this request pad

Application problem, see above.  "Fixing" this would only lead to more
applications getting away with improper behaviour, further complicating things
for everyone.

> - muxing elements would properly cater for this by implementing releasing a
> request pad, i.e. implement a release_pad function.  If a muxer does not to
> this; then gstreamer core (in gst_element_release_request_pad) will just fall
> back to removing the pad from the element, and this pad would still be in the
> collectpads collection (but no longer in the element!) --> blocking, which is
> then known for sure not to be the intention or a good thing

Sounds like a lot of muxers are broken then.  If this is the case, they should
be fixed.  It's probably a bit of cut-and-paste work because there is no
special base class for muxers to derive from (maybe it's time to create one?).

> It can indeed be argued that neither of the above is really responsibility of
> the core (and am as such hardly adamant about this patch), but a matter of
> 'proper programming', either in the applications or in the plugins.  So,
> abiding by those rules above, the current behaviour would cause no problems. 
> It would also at least require patches as in e.g. #374479 and #374658 (and
> possibly more?) to bring a few plugins up to specs/rules.

(mentioning the bug numbers with "bug" in front to generate correct autolinks:
bug #374479 and bug #374658)

> I hope this explains some concerns a bit better.
> [maybe core could somehow act differently in gst_element_release_request_pad
> and/or the need for release_pad should be emphasized more to CollectPads
> users?]

Changing gst_element_release_request_pad cannot solve this, as it cannot know
how the pad is used.  I think CollectPads documentation is clear enough
(gst_collect_pads_add_pad even mentions explicitely that it will hold a
reference to the pad).

Like you supposed, the problem we discuss here is not a deficiency of core but
rather a bug in muxers that got replicated widely (as there is no convenient
base class, where this probably belongs and could be fixed centrally).  Using
CollectPads means more or less to have the pads be managed by CollectPads
itself, and the default GstElement behaviour of release_pad interfers with that
as you pointed out.  If a request pad is to be released, it has to be removed
from the pad collection, too.  Good catch I'd say.


-- 
Configure bugmail: http://bugzilla.gnome.org/userprefs.cgi?tab=email