[Bug 748858] opencv: cvequalizehist: add color support

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=748858

Thiago Sousa Santos <thiagossantos at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #302828|none                        |needs-work
             status|                            |

--- Comment #3 from Thiago Sousa Santos <thiagossantos at gmail.com> ---
Review of attachment 302828:
 --> (https://bugzilla.gnome.org/review?bug=748858&attachment=302828)

A nice addition, thanks for the patch.

Some work is still needed, please see the comments in the review.

::: ext/opencv/gstcvequalizehist.c
@@ +74,3 @@
+  cvReleaseImage (&filter->plane_r);
+  cvReleaseImage (&filter->plane_g);
+  cvReleaseImage (&filter->plane_b);

This seems far too late to release those. It should be released when going from
PAUSED to READY state. I'd recommend implementing the stop method from
GstBaseTransform.

@@ +104,3 @@
+  gst_caps_append (caps, gst_opencv_caps_from_cv_image_type (CV_8UC1));
+  templ = gst_pad_template_new ("sink", GST_PAD_SINK, GST_PAD_ALWAYS,
+      gst_caps_ref (caps));

gst_pad_template_new doesn't take ownership of the caps so you don't need an
extra ref here

@@ +106,3 @@
+      gst_caps_ref (caps));
+  gst_element_class_add_pad_template (element_class, templ);
+  templ = gst_pad_template_new ("src", GST_PAD_SRC, GST_PAD_ALWAYS, caps);

And then you need to unref the caps after this part.

It might just be simpler to keep the static pad templates as they were and just
add RGB and BGR to the list of supported video formats.

@@ +127,3 @@
+  filter->plane_r = cvCreateImage (cvSize (in_width, in_height), in_depth, 1);
+  filter->plane_g = cvCreateImage (cvSize (in_width, in_height), in_depth, 1);
+  filter->plane_b = cvCreateImage (cvSize (in_width, in_height), in_depth, 1);

The caps can be changed during playback and you would leak the previous planes.
You should release those before creating new ones.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.