[Bug 775288] opencv: base opencv video filter class does not fully implement the video filter contract

[GStreamer (GNOME Bugzilla)]

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

--- Comment #13 from Nicolas Dufresne (stormer) <nicolas at ndufresne.ca> ---
(In reply to Philippe Renon from comment #11)
> Great analysis. Matches what I found and cleaned up while doing my single
> massive patch.
> 
> I'll take over once you are done and will propose small incremental patches.
> 
> Please also consider merging related patches in :
> - https://bugzilla.gnome.org/show_bug.cgi?id=774576
> and
> - https://bugzilla.gnome.org/show_bug.cgi?id=772822

Adding to my ToDo.

(In reply to Philippe Renon from comment #12)
> About naming:
> 
> 1/ Some elements are prefixed by cv (cvlaplace, cvsobel) while others are
> not (edgedetect, facedetect). What would be the preferred convention ? Don't
> worry, if I rename them it will be one of the last patch...

I personally prefer prefixing everything with cv, but it's not like we have any
other element that do edgetectect or facedetect. Might make users angry if we
rename. It's something we need to discuss and decide before a future move to
-good.

> 
> 2/ When passing a GstOpencvVideoFilter element as an argument, the argument
> name should not be trans or what not. I suggest to use cvfilter (same as
> vfilter for a GstVideoFilter).

This is cosmetic, but I agree, while at it, we should normalise. We could have:
  - trans: for GstBaseTransform
  - filter: For GstVideoFilter
  - cvfilter: for GstOpencvVideoFilter

While at it (assuming we go ahead and break the API), we could rename from
GstOpencvVideoFilter to GstOpencvFilter. OpenCV is about video so it's
redundant.

> 
> 3/ Most concrete elements, do something like this:
>     GstCvSobel *filter = GST_CV_SOBEL (base);
> According to 2/ it should be:
>     GstCvSobel *filter = GST_CV_SOBEL (cvfilter);
> And better (?) :
>     GstCvSobel *cvsobel = GST_CV_SOBEL (base);

This makes absolutely no difference. The value of the pointer is exactly the
same. The inheritance in GObject is just a C cast combined with a type check.
If you are certain that the type is right and require performance you could
just do:
      GstCvSobel *filter = (GstCvSobel *) base;

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