GstCollectPads-related freeze in pads mgmt

[Andrey Utkin]

2014/1/5 Sebastian Dröge <sebastian at centricular.com>:
> On Fr, 2014-01-03 at 19:41 +0200, Andrey Utkin wrote:
>> 2014/1/3 Sebastian Dröge <sebastian at centricular.com>:
>> > There are no plans for simplifying this currently, simply because nobody
>> > knows how to simplify it. If you have any suggestions please write them
>> > down :)
>>
>> The only suggestion i can give is to declare strong guarantees on the
>> simplest API calls.
>> For example, let gst_element_release_pad(),  gst_pad_unlink(),
>> gst_element_unlink() guarantee that no state corruption or deadlock is
>> possible, from whatever thread is it called.
>
> You mean they should just fail cleanly if called in the wrong situation?
> I agree, yes. Detecting all wrong situations is not trivial though :)

I mean also the elimination of "wrong situation" entity. "You
shouldn't unlink pads from main thread, do it from probe callback" -
such advice should be gone.
I understand that not everything can be done beautyfully, but after
all that's public API! It should be beautyful. That's not about
long-living core ugly hack, with which only experienced core
developers must deal (like masquerading in Asterisk). That's about
things with which any user has to deal with.
For example with tee. In my previous bug i proposed the patch that has
broken releasing tee pad from probe callback, but releasing them from
"main thread" was working just fine. I believe it is possible to
develop such locking control that both ways can work simultaneously.

>> I think this is more possible to implement using recursive mutexes, which
>> seem to be not used in gstreamer currently.
>
> They are used in quite a few places, e.g. the stream lock is a recursive
> mutex. The problem with the idea you seem to have is that it would mean
> that the functions mentioned above would just deadlock in situations
> where it would be wrong to call them, and the mutex would have no effect
> in other situations. Not really an improvement IMHO. You can emulate
> that now already if you just take the pad's stream lock before calling
> those functions ;)

Again, my idea includes to forbid to consider some API user's actions
as "wrong action".

> Also recursive mutexes are quite expensive, they should be used with
> care and only in situations where necessary.

Human life and project development life is also expensive.
If recursive mutex helps us to make API user to forget about probe
callbacks (if what he wants is not exactly them), it is a win. Human
mind is limited in entities he can account simultaneously. Removing
threads from accounting here simplifies thinking a lot.

>> Revising functionality guarantees requires revising all the code, but
>> i think it is worthwhile.
>
> I think what you want is some convenience API like insertbin, just for
> more/other use cases. I think your tee-queue bin is a good example of
> such a convenience API. Not sure where it should be put in the GStreamer
> modules and I think there are some improvements necessary first, but
> maybe you can provide a patch against gst-plugins-bad for this that just
> adds it as a new plugin for now. I'll review it then :)

I want all GStreamer API to be convenient.
Ok, i will provide a patch against plugins-bad at some moment, but let
us figure out and fix $SUBJ first.

I understand that my proposal adds to internal complexity (and to
active work with this complexity), but i've looked into core code, it
is very complex anyway, but still providing insufficient posibilities
IMO.

-- 
Andrey Utkin