GNOME Bugzilla – Bug 754148
OpenCV port remaining elements to C++ and new API
Last modified: 2015-10-02 18:09:31 UTC
The remaining elements of OpenCV should be ported to C ++ language, because three elements, facedetect, handdetect and faceblur, were modified with the new API 2.4.11 of Opencv to work in old and new versions. Handdetect -> https://bugzilla.gnome.org/show_bug.cgi?id=752528 Facedetect -> https://bugzilla.gnome.org/show_bug.cgi?id=748377 Faceblur -> https://bugzilla.gnome.org/show_bug.cgi?id=753994
Created attachment 310135 [details] [review] Remove unused and useless functions to achieve the objective. The cvSmooth cvNot functions and do not have the correct input parameters. Furthermore, cvSmooth function is not necessary for edge detection, because the Canny function makes the step of smoothing the image. And cvNot function is useless because there aren't changes if this function is eliminated. https://bugzilla.gnome.org/show_bug.cgi?id=754148
Created attachment 310141 [details] [review] Remove unused and useless functions The cvSmooth cvNot functions and do not have the correct input parameters. Furthermore, cvSmooth function is not necessary for edge detection, because the Canny function makes the step of smoothing the image. And cvNot function is useless because there aren't changes if this function is eliminated. https://bugzilla.gnome.org/show_bug.cgi?id=754148
Created attachment 310181 [details] [review] Rename gstedgedetect.c to gstedgedetect.cpp for consistency. Change the file extension to cpp and add it into Makefile for consistency with other elements of opencv. https://bugzilla.gnome.org/show_bug.cgi?id=754148
Review of attachment 310141 [details] [review]: Tested and patch makes sense. Looks good.
Review of attachment 310181 [details] [review]: I don't think this is about consistency but about using the C++ API of OpenCV. Patch is good. Just change commit message to say this switches the element to C++ since that is what OpenCV supports only now. Tested.
Review of attachment 310181 [details] [review]: Sorry. Meant need-work.
Review of attachment 310181 [details] [review]: After second thought. Commit message is fine like this.
Review of attachment 310141 [details] [review]: Merged.
Created attachment 310274 [details] [review] Change gstretinex.c to C++. Change the gstretinex.c file to cpp and add it into Makefile. It is necessary to migrate the retinex element to C++, because new Opencv API leaves obsolete functions like cvSmooth. This element uses this function. You can see in this link: http://docs.opencv.org/modules/imgproc/doc/filtering.html? highlight=cvsmooth#void cvSmooth(const CvArr* src, CvArr* dst, int smoothtype, int size1, int size2, double sigma1, double sigma2) https://bugzilla.gnome.org/show_bug.cgi?id=754148 Ready for review.
Created attachment 310321 [details] [review] Rename gstcvsmooth.c to gstcvsmooth.cpp for consistency. Change the file extension to cpp and add it into Makefile for consistency with other elements of opencv. https://bugzilla.gnome.org/show_bug.cgi?id=754148 Ready for review
Created attachment 310322 [details] [review] need to migrate to new version of OpenCV 2.4.11. It is necessary to migrate the gstcvsmooth element to C++, because new Opencv API leaves obsolete functions like cvSmooth. This element uses this function. You can see in this link: http://docs.opencv.org/modules/imgproc/doc/filtering.html? highlight=cvsmooth#void cvSmooth(const CvArr* src, CvArr* dst, int smoothtype, int size1, int size2, double sigma1, double sigma2) https://bugzilla.gnome.org/show_bug.cgi?id=754148 Ready for review.
Created attachment 310372 [details] [review] Rename gstskindetect.c to gstskindetect.cpp for consistency. Change the file extension to cpp and add it into Makefile for consistency with other elements of opencv and because Opencv not support C language in new API 2.4.11. https://bugzilla.gnome.org/show_bug.cgi?id=754148 Ready for review.
Review of attachment 310274 [details] [review]: Looks good.
Review of attachment 310321 [details] [review]: Can you change the subject of this patch to "cvsmooth: change to C++"?
Review of attachment 310322 [details] [review]: Can you explain why you are adding a PROP_PIXEL_DIAMETER? Is this necessary to migrate to the new version of OpenCV?
One of the functions added due to cvSmooth becomes obsolete, has one input parameter (PROP_PIXEL_DIAMETER) that did not exist in cvSmooth. For this I think that enters the reason for migrating to C ++.
Created attachment 310383 [details] [review] Change to C++ Change the file extension to cpp and add it into Makefile for consistency with other elements of opencv and because Opencv not support C language in new API 2.4.11. https://bugzilla.gnome.org/show_bug.cgi?id=754148 Only I changed the summary and description of this patch. Ready for review.
Created attachment 310386 [details] [review] opencvutils: Add extern C, for the opencv elements work. G_BEGIN_DECLS and G_END_DECLS added within gstopencvutils.h for to allow the header is linked in C++. https://bugzilla.gnome.org/show_bug.cgi?id=754148
Review of attachment 310322 [details] [review]: Then you should explain this in the commit message. (New property). In line 190: g_object_class_install_property (gobject_class, PROP_HEIGHT, You have PROP_HEIGHT twice, I think you meant PROP_PIXEL_DIAMETER.
Review of attachment 310372 [details] [review]: Please change the commit message. Renaming the file is the action, "Change skindetect to C++" is the purpose. Actions are explicit when you look at a commit diff, you want to read the purpose in the commit message to understand why.
Review of attachment 310322 [details] [review]: Wait. Should the recent patch that you sent make this one obsolete? There is a checkmark you have to tick, declaring it made a previous patch obsolete.
Review of attachment 310386 [details] [review]: Patch looks good. Commit message is broken though: "Adding G_BEGIN_DECLS and G_END_DECLS to gstopencvutils.h to allow C-style linking".
(In reply to Luis de Bethencourt from comment #19) > Review of attachment 310322 [details] [review] [review]: > > Then you should explain this in the commit message. (New property). > > In line 190: > g_object_class_install_property (gobject_class, PROP_HEIGHT, > > You have PROP_HEIGHT twice, I think you meant PROP_PIXEL_DIAMETER. Yes, is PROP_PIXEL_DIAMETER, sorry, I change this now.
Created attachment 310422 [details] [review] cvsmooth: need to migrate to new version of OpenCV 2.4.11. It is necessary to migrate the gstcvsmooth element to C++, because new Opencv API leaves obsolete functions like cvSmooth. This element uses this function. Therefore, it was replaced by the GaussianBlur, blur, medianBlur and bilateralFilter functions. bilateralFilter function has an input parameter, is diameter of each pixel. For this reason a new property is added, PROP_PIXEL_DIAMETER. You can see in this link: http://docs.opencv.org/modules/imgproc/doc/filtering.html? highlight=cvsmooth#void cvSmooth(const CvArr* src, CvArr* dst, int smoothtype, int size1, int size2, double sigma1, double sigma2) https://bugzilla.gnome.org/show_bug.cgi?id=754148 Only I changed the commit message and I fixed line 190 where I made a mistake.
Review of attachment 310383 [details] [review]: Looks good.
Review of attachment 310422 [details] [review]: Thanks for fixing this.
Created attachment 310427 [details] [review] skindetect: Change gstskindetect to C++. Change the file extension to cpp and add it into Makefile for consistency with other elements of opencv and because Opencv not support C language in new API 2.4.11. https://bugzilla.gnome.org/show_bug.cgi?id=754148 Only I changed commit message.
Created attachment 310428 [details] [review] opencvutils: add extern C, for the opencv elements work. Adding G_BEGIN_DECLS and G_END_DECLS to gstopencvutils.h to allow C-style linking. https://bugzilla.gnome.org/show_bug.cgi?id=754148
Only I changed commit message of previous patch.
Review of attachment 310427 [details] [review]: Looks good.
Review of attachment 310428 [details] [review]: Thanks Vanessa.
Created attachment 310815 [details] [review] cvdilate: Change gstcvdilate to C++. Change the gstcvdilate.c file extension to cpp and add it into Makefile for consistency with other elements of opencv and because Opencv not support C language in new API 2.4.11. https://bugzilla.gnome.org/show_bug.cgi?id=754148 Ready for review.
Review of attachment 310815 [details] [review]: Looks good. Thanks Vanessa :)
Review of attachment 310181 [details] [review]: commit 1e14ceedb38e063e4c5558e8aa28681a57049c13 Author: Vanessa Chipirrás Navalón <vchipirras6@gmail.com> Date: Fri Aug 28 13:42:29 2015 +0200 edgedetect: Rename gstedgedetect.c to gstedgedetect.cpp for consistency. Change the file extension to cpp and add it into Makefile for consistency with other elements of opencv. https://bugzilla.gnome.org/show_bug.cgi?id=754148
Review of attachment 310274 [details] [review]: commit 936665dfd65f1a58aa35d137b19b2d3eec4f4784 Author: Vanessa Chipirrás Navalón <vchipirras6@gmail.com> Date: Fri Oct 2 17:48:47 2015 +0100 retinex: Change retinex to C++ Change the gstretinex.c file to cpp and add it into Makefile. It is necessary to migrate the retinex element to C++, because new Opencv API leaves obsolete functions like cvSmooth. This element uses this function. You can see in this link: http://docs.opencv.org/modules/imgproc/doc/filtering.html? highlight=cvsmooth#void cvSmooth(const CvArr* src, CvArr* dst, int smoothtype, int size1, int size2, double sigma1, double sigma2) https://bugzilla.gnome.org/show_bug.cgi?id=754148
Review of attachment 310383 [details] [review]: commit ae55a537e6b4244be4bec086bb512bdb6cf8ac0a Author: Vanessa Chipirrás Navalón <vchipirras6@gmail.com> Date: Fri Oct 2 18:10:32 2015 +0100 cvsmooth: port to C++ Change the file extension to cpp and add it into Makefile for consistency with other elements of opencv and because Opencv not support C language in new API 2.4.11. https://bugzilla.gnome.org/show_bug.cgi?id=754148
Review of attachment 310422 [details] [review]: Same as above commit which is already merged.
Review of attachment 310428 [details] [review]: commit 1c9763c0eb3e4f89bc6871582aca7212c44d3c64 Author: Vanessa Chipirrás Navalón <vchipirras6@gmail.com> Date: Mon Aug 31 17:43:26 2015 +0200 opencvutils: add extern C, for the opencv elements work. Adding G_BEGIN_DECLS and G_END_DECLS to gstopencvutils.h to allow C-style linking. https://bugzilla.gnome.org/show_bug.cgi?id=754148
Review of attachment 310427 [details] [review]: commit 7f9aef31cb380fead3f90b645b5175fd59521701 Author: Vanessa Chipirrás Navalón <vchipirras6@gmail.com> Date: Fri Oct 2 18:50:45 2015 +0100 skindetect: Change gstskindetect to C++ Change the file extension to cpp and add it into Makefile for consistency with other elements of opencv and because Opencv not support C language in new API 2.4.11. https://bugzilla.gnome.org/show_bug.cgi?id=754148
Review of attachment 310815 [details] [review]: commit f8e11fe656a360004f9874befd1d217cda407659 Author: Vanessa Chipirrás Navalón <vchipirras6@gmail.com> Date: Fri Oct 2 19:02:26 2015 +0100 cvdilate: Change gstcvdilate to C++ Change the gstcvdilate.c file extension to cpp and add it into Makefile for consistency with other elements of opencv and because Opencv not support C language in new API 2.4.11. https://bugzilla.gnome.org/show_bug.cgi?id=754148