[Bug 702722] opencv: add GrabCut segmentation element

[GStreamer (bugzilla.gnome.org)]

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

--- Comment #4 from Miguel (elmiguelao) Casas-Sanchez <miguelecasassanchez at gmail.com> 2013-07-09 14:48:33 UTC ---
(In reply to comment #3)
> Review of attachment 247303 [details]:
> 
> ::: ext/opencv/gstfacedetect.c
> @@ +674,3 @@
> +        GstEvent *ev;
> +        GstStructure *str;
> +        str = gst_structure_new ("face",
> 
> Prefix the structure with the element name at least, GstOpenCVFaceDetect-face
> or something like that.
> 

Done.

> Also this should probably be a GstMeta on the buffer instead of an event

Not necessarily, face detect element could output a face position event at a
lower frame rate than the data stream... But I'd follow you on this.

> 
> ::: ext/opencv/gstgrabcut.cpp
> @@ +181,3 @@
> +IN CASE OF no alpha mask input (all 0's or all 1's), the 'face' \
> +downstream event is used to create a bbox of PR_FG elements.\n\
> +IF nothing is present, then a by default bbox is taken.", "Miguel
> Casas-Sanchez <miguelecasassanchez at gmail.com>");
> 
> Put this into the documentation, not here

Done.

> 
> @@ +276,3 @@
> +  grabcut->facepos.y = 77;
> +  grabcut->facepos.width = 60;
> +  grabcut->facepos.height = 70;
> 
> What are these magic number? :)

They are the magic number of a person in a video conference environment looking
at a laptop's front camera. Is just a failsafe option to avoid empty FG masks,
see next comment.

> 
> @@ +334,3 @@
> +        if (abs (w) > 2)
> +          grabcut->facepos.width = (int) w;
> +        if (abs (h) > 2)
> 
> Why these checks for >2?
> 

In case of misconfigured face detectors, some faces are spurious and would have
widths/heights of 1. This is to filter those out, GrabCut doesn't like too
small seeds... (there's an ASSERT somewhere).

> @@ +377,3 @@
> +  }
> +
> + 
> //////////////////////////////////////////////////////////////////////////////
> 
> No C++/C99 comments please, use /* */

Done. (Keeps coming, sorry)

> 
> @@ +422,3 @@
> +
> +
> +// copied, otherwise only available in C++
> 
> Can't you just use the C++ variants of these? Just make the file a C++ file :)

Yes, actually  I can!

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