[Bug 704760] opencv: disparity-map calculation element

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=704760
  GStreamer | gst-plugins-bad | 1.x

--- Comment #4 from Sebastian Dröge (slomo) <slomo at circular-chaos.org> 2013-07-28 15:16:19 UTC ---
(In reply to comment #2)
> (In reply to comment #1)
> > Review of attachment 249926 [details] [details]:
> > 
> > ::: ext/opencv/gstdisparity.cpp
> > @@ +161,3 @@
> > +      {METHOD_SGBM, "Semi-global block matching algorithm", "sgbm"},
> > +      {METHOD_VAR, "Variational matching algorithm", "svar"},
> > +      {METHOD_GC, "Graph=cut based matching algorithm", "sgc"},
> > 
> > Should this be Graph=cut or Graph-cut?
> > 
> Graph-cut :)
> 
> > @@ +262,3 @@
> > +  gst_pad_set_chain_function (filter->sinkpad_right,
> > +      GST_DEBUG_FUNCPTR (gst_disparity_chain_right));
> > +  GST_PAD_SET_PROXY_CAPS (filter->sinkpad_right);
> > 
> > I don't think this makes negotiation happen correctly. At the moment when one
> > of the pads has caps, the other one has to return exactly these caps as a
> > result of the CAPS query and can't accept anything else anymore. Apart from
> > that it is PROXY_CAPS, yes. So implement a query function on both pads that
> > does this and does not call gst_pad_query_default() for the CAPS query.
> > 
> 
> Hmm I don't fully understand, I thought at the moment the negotiation would 
> happen via sink_event, so perhaps I should modify that function so on second
> call
> it returns the format accepted in the first call, and keep the caps proxied?

Yes, that's what videomixer does for example. Proxy the CAPS query always,
unless a CAPS event arrived already. In that case return the caps that were set
previously.

> > @@ +318,3 @@
> > +
> > +  switch (GST_EVENT_TYPE (event)) {
> > +    case GST_EVENT_CAPS:
> > 
> > All this needs some blocking, the CAPS events will arrive from two different
> > threads and can happen at the same time
> > 
> 
> Added a mutex on both sides to make this a critical section. I guess
> gst_disparity_init() is called before any event handling (rtfm for me?) ;)

Yes, that's called before anybody even has a pointer to the instance :)


> > So for the GCond handling there are two things missing. This can easily
> > deadlock right now. First of all you should always put the waiting in a loop,
> > i.e. while (fs->buffer_left) g_cond_wait().
> 
> The cond should keep the chain_{left,right} fully blocked until the other chain
> arrives at
> the rendezvous. What would be the situation in which a chain function blocked
> at the cond 
> variable would wake up _not_ being signalled by the other chain function?

Flushes and the pipeline being shut down.

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