[Bug 722511] tq: tee element with embedded queue elements on srcpads

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=722511
  GStreamer | gst-plugins-bad | git

--- Comment #15 from Andrey Utkin <me at andrey-utkin.pp.ua> 2014-01-20 15:36:44 UTC ---
(In reply to comment #14)
> > What about a situation that direct synchronous execution is not performed
> > because the pad is blocked by some data flow, and asynchronous execution does
> > not take place also because that data flow act which prevented synchronous
> > execution was LAST on that pad?
> 
> Well, that would mean it would be blocked forever in the sink for some reason
> (like PAUSED forever or a deadlock). If it was the last buffer, at some point
> gst_pad_push() will return... and then the IDLE probe will be called.

Good if so, it's just not described anywhere.

> > > > > Well, maybe your block probe was called because of the upstream QoS event...
> > > > > and then you removed the tee branch from there... i.e. the streaming thread of
> > > > > the queue in the worst case.
> > > > 
> > > > So, on higher level of view, does that "if" actually helps GstTq to work the
> > > > right way in general case?
> > > > 
> > > > Would it be right if, before releasing tq pad, application blocks a sinkpad
> > > > connected to tq so that nothing passes?
> > > 
> > > No, the application should not need to worry about anything and just call
> > > gst_element_release_pad(). And internally you care for all that.
> > 
> > Well, the context was that this "if" is internal to GstTq, but not a something
> > that should be added in application code. And the reason it is added is to
> > avoid joining queue thread from itself.
> 
> Yeah, can you get a backtrace for the joining thread from itself problem? It
> indicates a conceptual problem elsewhere imo

Assume the application wants to release a requested pad of tq.
It is done my setting up a probe callback that does
gst_element_release_request_pad (tq, pad) and returns ...REMOVE.
The most probable thread on which this callback could execute, if all is linked
and playing, is the pushing thread of queue behind the subject tq pad.
So in gst_tq_release_pad(), we have to account this, and avoid doing
...set_state (queue, ...NULL) straight in the procedure. Instead, what is
currently implemented is doing it from internal tee's srcpad probe.
I think there's no conceptual problem at this point.
But, as you have confirmed my observation, in this environment the callback can
be executed by upstream-directed flow, exactly the tee's srcpad probe callback
in this case. And this can result in tee srcpad probe callback being executed
by the queue thread, and doing ...set_state (queue, ...NULL) will deadlock or
at last give a critical assertion warning.

I haven't found a possibility to exclude upstream-directed cases by bitmask
passed to ...add_probe(), so i have excluded these cases as "if" in the
procedure itself.

-- 
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.