[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 #9 from Miguel (elmiguelao) Casas-Sanchez <miguelecasassanchez at gmail.com> 2013-06-10 12:53:16 UTC ---
(In reply to comment #8)
> Review of attachment 246359 [details]:
> 
> ::: ext/opencv/gstsegmentation.cpp
> @@ +135,3 @@
> +      {0, NULL, NULL},
> +    };
> +    etype = g_enum_register_static ("GstSegmentationMethod", values);
> 
> The type name should probably contain the element name, otherwise there is risk
> of conflict
> 
> @@ +350,3 @@
> +gst_segmentation_release_all_pointers (GstSegmentation * filter)
> +{
> +  cvReleaseImage (&filter->cvRGBA);
> 
> You should set at least this one here to NULL, otherwise there's risk for
> double free. Or is cvReleaseImage() doing that already?

It should do it
[http://opencv.willowgarage.com/documentation/c/core_utility_and_system_functions_and_macros.html#cvFree];
at core, is just (a couple of) free() calls.

> 
> @@ +363,3 @@
> +
> +static GstFlowReturn
> +gst_segmentation_transform_ip (GstBaseTransform * btrans, GstBuffer * buf)
> 
> Note that GstVideoFilter has its own vfunc for this:
>   GstFlowReturn (*transform_frame)    (GstVideoFilter *filter,
>                                        GstVideoFrame *inframe, GstVideoFrame
> *outframe);
>   GstFlowReturn (*transform_frame_ip) (GstVideoFilter *trans, GstVideoFrame
> *frame);
> 
> You might want to use that, or use GstOpenCVVideoFilter

I did so and also I changed set_caps() with set_info(), I think in the line of
the GstVideoFilter.

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