[Bug 683995] [0.11] [API change] No parent parameter in GstPadLinkFunction

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=683995
  GStreamer | gstreamer (core) | git

Wim Taymans <wim.taymans> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wim.taymans at gmail.com

--- Comment #1 from Wim Taymans <wim.taymans at gmail.com> 2012-09-14 08:44:01 UTC ---
(In reply to comment #0)
> Created an attachment (id=224288)
 View: https://bugzilla.gnome.org/attachment.cgi?id=224288
 Review: https://bugzilla.gnome.org/review?bug=683995&attachment=224288

> pad: Add parent parameter to link function
> 
> While looking at the GstPadLinkFunction, I found two strange things:
> 
> 1. There is no parent pointer passed, even though every user in the main
> modules seems to fetch its parent as its first action.

It didn't think it was important enough to add, but yes, it would be good.

> 2. The documentation says that a link function on a src pad should call the one
> on it's peer sink pad, that sound pretty painful to implement safely, as the
> object lock is released some other thread could be unlinking/releasing the sink
> pad while this happens.
> 
> If there is a good reason for either of those behaviours ?

It was done in an attempt to make linking/unlinking threadsafe. The idea is
that the linkfunction of the srcpad holds some sort of lock while it calls the
peerpad link function. This makes sure that you only have 1 thread calling both
the link/unlink function of the srcpad and the sinkpad and both pads would be
sure that they are linked together.

If you call both link functions separately, by the time you have called the
linkfunction on the srcpad, the sinkpad could already have been linked to
another pad. Then you have a situation where the srcpad thinks it is linked to
a sinkpad while it really isn't. You could check this after calling the link
function and undo the link but that is messy and also racy.

Unfortunately we could not use the object lock on the srcpad for this because
you can't hold it while calling into the link/unlink function of the peer pad
without nasty deadlocks (if the peer sinkpad, for example, would do anything
with the srcpad). So, the proper way to implement a link/unlink function is to
make a separate lock that you take before calling the peer link function.

It's been a while since I thought about this so I might be missing something
but that's what I remember. Link/unlink functions are really not used much (and
when they are, they can probably go).

> 
> I'm attaching a proposed patch that adds a parent pointer, calls both functions
> independently and rejects the link if the parent is required but gone. Please
> discuss.
> 
> Also, please don't merge this patch without updating the other modules, if you
> think it's a good idea, I can do the fixing.
> 
> The unlink function happens under the object lock, so none of these problems
> apply.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.