[Bug 701421] opencv: add foreground/background segmentation element

[GStreamer (bugzilla.gnome.org)]

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

--- Comment #2 from Miguel (elmiguelao) Casas-Sanchez <miguelecasassanchez at gmail.com> 2013-06-04 18:34:39 UTC ---
(In reply to comment #1)
> Review of attachment 245821 [details]:
> 
> ::: ext/opencv/gstsegmentation.c
> @@ +195,3 @@
> +  g_object_class_install_property (gobject_class, PROP_DISPLAY,
> +      g_param_spec_boolean ("display", "Display",
> +          "Display the foreground as white over background black ",
> 
> Maybe call that property black-white instead? display sounds like it
> enables/disables displaying of the effect

Well, that's what it does, instead of the "normal" output, which is FG/BG in
the alpha channel, "display=true" applies the mask over the input, thus
creating a black and white output. It could be applied as transparency over the
colour image, but in my experience this is a debug parameter used to check if
the movement mask creation is going all right.

> 
> @@ +298,3 @@
> +  /* Codebook method */
> +  segmentation->TcodeBook = (codeBook *)
> +      g_malloc (sizeof (codeBook) *
> 
> This must be released with release_all_images() too, otherwise you leak it

It is released in gst_segmentation_stop(), which in turn calls
release_all_images().

Btw I forgot to delete the two pointers in opencv_wrapper.cpp, I added
(finalise_mog) it now :)

> 
> @@ +401,3 @@
> +  }
> +  /* Create the foreground and background masks using BackgroundSubtractorMOG
> [1], 
> +   *  Gaussian Mixture-based Background/Foreground segmentation slgorithm.
> OpenCV 
> 
> Typo slgorithm

Done.

> 
> @@ +461,3 @@
> +
> +
> +#ifdef CODE_FROM_OREILLY_BOOK
> 
> Why? :)

Well, this code is not mine, so I wanted to make it absolutely clear that I
copied it. It's also mentioned in the first lines of the file, copyright stuff:

 * Except: Parts of code inside the preprocessor define CODE_FROM_OREILLY_BOOK, 
 * which are downloaded from O'Reilly website and adapted.

> 
> ::: ext/opencv/opencv_wrapper.h
> @@ +1,1 @@
> +/*
> 
> You could also just put your element into a .cpp file instead of having this
> wrapper :)

Ah, I didn't know. However, more and more OpenCV functionality comes just with
C++ wrappers, and I have a couple of other elements/ideas in the pipeline that
would also need to have (a bit larger) C++ wrapper. I find it clearer to have
both separated, C for GStreamer and C++ for OpenCV :) but I can merge them if
you think it's best.


I'll upload another patch with the mentioned changes.

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