GNOME Bugzilla – Bug 748377
OpenCV face detection does not work with OpenCV newer than 2.4.10
Last modified: 2015-08-21 12:27:43 UTC
$ gst-launch-1.0 autovideosrc ! video/x-raw,width=320,height=240 ! videoconvert ! facedetect min-size-width=60 min-size-height=60 ! d3dvideosink OpenCV Error: Unspecified error (The node does not represent a user object (unknown type?)) in cvRead, file F:/e43/src/mingw/opencv-2.4.11-2/opencv-2.4.11/modules/core/src/persistence.cpp, line 5008 terminate called after throwing an instance of 'cv::Exception' what(): F:/e43/src/mingw/opencv-2.4.11-2/opencv-2.4.11/modules/core/src/persistence.cpp:5008: error: (-2) The node does not represent a user object (unknown type?) in function cvRead This seems to be happening because cascade format was changed in 2.4.11. Previously it was: <haarcascade_frontalface_default type_id="opencv-haar-classifier"> Now it is: <cascade type_id="opencv-cascade-classifier"><stageType>BOOST</stageType> This was caused by https://github.com/Itseez/opencv/commit/7efc917ee0a57bb88855ac1c043f0d0f7bc8d873 Googling suggests that there's a new cascade loader that can handle these new cascades, but i haven't been able to figure the details.
Dug a bit deeper. The loader that was spoken of is CascadeClassifier class and its load() method. Looking at it i see that it first tries to load the cascade via a combination of a FileStorage object and the method read() of CascadeClassifier itself (which leafs through the nodes) and if that fails, then it falls back to cvLoad(). AFAICS, there's no C equivalent for this. The detection method for CascadeClassifier is detectMultiScale(). Internally it calls cvHaarDetectObjectsForROC() for old-style cascades, which is what gstfacedetect calls (indirectly; it calls cvHaarDetectObjects(), which just a convenience wrapper for cvHaarDetectObjectsForROC()). If converting gstfacedetect to C++ is acceptable, it shouldn't be too hard.
Will look into this.
Hello, I tried to reproduce the bug on this machine: Linux 3.13.0-49-generic #83-Ubuntu SMP Fri Apr 10 20:14:51 UTC 2015 i686 This is the command I executed: $ gst-launch-1.0 autovideosrc ! video/x-raw,width=320,height=240 ! videoconvert ! facedetect min-size-width=60 min-size-height=60 ! videoconvert ! autovideosink I don't get any bug, it works all right. You can see it here: http://i.imgur.com/5jMV0om.png
(In reply to Vanessa Chipirrás Navalón from comment #3) > Hello, > > I tried to reproduce the bug on this machine: Linux 3.13.0-49-generic > #83-Ubuntu SMP Fri Apr 10 20:14:51 UTC 2015 i686 Which version of opencv do you have installed? I've looked up Ubuntu package database, and even Vivid (which seems to be the latest release, scheduled to come out this month) has opencv-2.4.9, which is still older than 2.4.11. This bug only happens on 2.4.11 and newer.
Yes. You're right, I had the version 2.4.9 of opencv. I already installed version 2.4.11 of opencv, and now I get the bug. Probably some API changes between versions opencv is the problem. I'll get to investigate it.
Hi, I have been researching the library for new versions of opencv. The structure of the classifier (xml file) has changed, and opencv haven't a method to load the new structure in C,it is possible C ++ language only. The method for charging the classifier of older versions is cvLoad () and the method of loading new structure in C ++ is called load (). Is it possible to change the gstfacedetect.c file, and other files in C ++ to fix this bug?
If C++ is better supported by the library, I think it would be a good idea to port all the elements to C++. Thiago, any opinion for against this ?
For all I know, OpenCV now uses the C++ as the main API so I agree on porting it to C++.
Great. I agree. Vanessa, port the element to C++
I have ported the file gstfacedetect.c to C++, trying that the changes do not interfere with other existing files in the opencv folder. Now I'm fixing some problems that I got.
Cool! Keep at it, great work.
Hello, I uploaded my repository only those files that I have changed, the new files aren't definitive. Here I leave the link. https://github.com/vanechipi/Shifting-To-Language-C-.git Until now face detection works when the face appears in front of the camera. But I get an exception when entering negative values of a rectangle in the second parameter of the constructor Mat class called "roi". Still I don't know why I get negative values of the properties of the rectangle. I know my exception is generated on line 691 and 580. I'm working on it.
Sorry for my github link above. The correct link to github is as follows: https://github.com/vanechipi/gst-plugins-bad.git I did not mention before, eyes detection works well, and paint the ellipse in them. You can see it here: http://imgur.com/A0oqGKb
Great work Vanessa. Will review soon.
Completing the migration the file 'gstfacedetect.c' to C ++ language. Detects the face, eyes, nose and mouth properly. It is ready to be reviewed. The link of the repository is: https://github.com/vanechipi/gst-plugins-bad.git Link test image: http://imgur.com/na0HpBm
I've tested this and it work. Will review the code in a few hours. Anybody has any comments or if the code is good can I merge?
Nice, thanks for working on this! Didn't review in depth, but looks fine overall. Some small style nitpicks: - the two commits (rename plus change code to C++) should be squashed into one - the bug number (ideally the whole bugzilla bug url) should go into the commit message at the bottom, but not into the summary line - the code should ideally still be indented with GStreamer indentation style. Not sure if running gst-indent on the .cpp file will just work, maybe just try it. Otherwise not so important, can be fixed later.
oh, Ok!! Thanks for the corrections!
Created attachment 308109 [details] Test image
Created attachment 308110 [details] Test Image
Tim, I followed your suggestions, thank you very much. This is the result of my new commit in my repository. https://github.com/vanechipi/gst-plugins-bad/commit/5f972d3e5875da82a9f4c95c8b79f8565c525941
About gst-indent does not work for .cpp files
Add unit test of facedetect. It is ready to be reviewed. I created four utilities of the detected features. 1- If you hide your mouth, the volume of video is low. 2- If you hide the nose, the volume of the video is up. 3- If you hide the full face, the video stops. You can see proof of this here: https://www.youtube.com/watch?v=pxzejNKV_WQ In my repository: https://github.com/vanechipi/gst-plugins-bad/commit/79598c72fdcbe795ac6d62c08a8aa836cd2a67de
Created attachment 308214 [details] [review] Load new Cascade Classiffier
Created attachment 308215 [details] [review] Add unit test
Review of attachment 308214 [details] [review]: Hi Vanessa. Thanks for your work! Few comments bellow: ::: ext/opencv/gstfacedetect.cpp @@ +600,3 @@ break; case GST_FACEDETECT_UPDATES_ON_CHANGE: + if (!faces.empty() && (faces.size() > 0)) { Shouldn't !faces.empty() be enough? There are quite some occurrences of this redundant check throughout your code. @@ +631,3 @@ } + for(unsigned int i = 0; i < faces.size(); ++i){ Use space after for, if, etc. Or at least be consistent with your style choice. @@ +670,3 @@ + rey = r.y; + rew = r.width; + reh = r.height / 2; Above code seems to be repeated quite a few times. If possible, Please consider factoring it out. @@ +716,3 @@ + "eyes->y", G_TYPE_UINT, rey + sr.y, + "eyes->width", G_TYPE_UINT, sr.width, + "eyes->height", G_TYPE_UINT, sr.height, NULL); Same as above, please consider factoring it out @@ +770,3 @@ + h = sr.height / 2; + center.x = cvRound ((rex + sr.x + w)); + center.y = cvRound ((rey + sr.y + h)); Same
Vanessa, Follow Reynaldo's suggestion in a new commit. Since these problems were there originally. Reynaldo, thanks for the review :)
Quite some of those lines are been shuffled around or even modded already. It might make sense to handle it on the same patch. But yeah, not a blocker at all.
I think any factoring out of stuff should be done in a second patch, with this patch just being the syntax changes needed to port the existing code over to C++. That makes it easier to review the changes and if something breaks it'll be easier to figure out later if the breakage was caused by the refactoring or by the porting.
Review of attachment 308214 [details] [review]: commit 78d0c5f01ef8d361c66a346aa670ce35a94c247e Author: Vanessa Chipirrás <vchipirras6@gmail.com> Date: Fri Jul 31 13:45:43 2015 +0100 facedetect: need to migrate to C++ The cascade classifier changes its structure on new version of OpenCV. The need to migrate to C++ to utilize the new load method of OpenCV which allows to load the new classifiers. https://bugzilla.gnome.org/show_bug.cgi?id=748377
Great work Vanessa. Congratulations on your first GStreamer commit :)
Review of attachment 308215 [details] [review]: commit 02b9daafdf64ab205ed725f5496dfa5ce860eacc Author: Vanessa Chipirrás <vchipirras6@gmail.com> Date: Fri Jul 31 17:31:15 2015 +0100 facedetect: Add unit test I created four utilities of the detected features: 1- If you hide your mouth, the volume of video is low. 2- If you hide the nose, the volume of the video is up. 3- If you hide the full face, the video stops. You can see proof of this here: https://www.youtube.com/watch?v=pxzejNKV_WQ https://bugzilla.gnome.org/show_bug.cgi?id=748377
... and the second!
What's left to be done here?
Hi Sebastian, There is a small issue I am fixing right now and Tim asked me to clean the unit test a bit. Will close this bug once those two things are done.
OK. Small issue fixed. http://www.google.com/url?q=http%3A%2F%2Fcgit.freedesktop.org%2Fgstreamer%2Fgst-plugins-bad%2Fcommit%2F%3Fid%3D3a765e0805688847e36863f5ea4b30c4e19ae6d3&sa=D&sntz=1&usg=AFQjCNE328ev56wMudmPmMwN53HCDWQD9w Closing this bug. Well done Vanessa :)
Btw, delete is not a function, don't add parenthesis to it, it's confusing. > + delete (cascade); Should have been > + delete cascade;
Hello, thanks for to correct and check my code. I uploaded to my repository 3 new commits. These changes are about simplifying few lines of code and eliminate certain redundancies. These commits are ready for review. https://github.com/vanechipi/gst-plugins-bad/commit/d65a43d1f6c16d33707bb91429eec9f181ac94ce https://github.com/vanechipi/gst-plugins-bad/commit/79d6fcc8ddaa7eac0ab5b0198253eb7b469f7c7d https://github.com/vanechipi/gst-plugins-bad/commit/a528dda0e9fa268ab3e9c67631c253a4e66f2c91
Created attachment 309027 [details] [review] Redundancy exists in code Ready for review.
Created attachment 309028 [details] [review] Simplify repeated code Ready for review
Created attachment 309029 [details] [review] Refactor the code. Ready for review.
Review of attachment 309027 [details] [review]: "Checks the vector is not empty and the vector size is greater than zero at a time." In the commit message you want to explain what is wrong and how you are fixing it. Read a bunch of commit messages in git log to get a feel for how people write them.
Please, adopt a proper C++ style when porting. As commented on one of the patch on github, in C++ you should use inline functions instead of macros, and also const instead of macros. Keep things that belong to your class inside your class, etc.
Created attachment 309030 [details] [review] Redundancy exists in code. I changed the commit message.
Created attachment 309033 [details] [review] Wrong form to write the delete operator Following the review of Nicolas, I eliminated the parentheses of the delete operator.
Review of attachment 309030 [details] [review]: commit 73bc2cf7b05e026bef776e0ccb843e98b008c72d Author: vanechipi <vchipirras6@gmail.com> Date: Mon Aug 10 19:02:10 2015 +0200 facedetect: Redundancy exists in code. Checking the vector is not empty and checking the vector size is greater than zero are the same thing, this is a redundancy in the code. Only checking the vector is not empty is sufficient, therefore removing the other check.
Review of attachment 309028 [details] [review]: commit 28f9ff131276e3e0df5d0b6d6e5c03ce01f5319e Author: vanechipi <vchipirras6@gmail.com> Date: Mon Aug 10 19:13:11 2015 +0200 facedetect: simplify repeated code. Store the value of r.height / 2 instead of repeating the operation line three times.
Review of attachment 309029 [details] [review]: See Nicola's review: https://github.com/vanechipi/gst-plugins-bad/commit/a528dda0e9fa268ab3e9c67631c253a4e66f2c91#commitcomment-12625767
> Author: vanechipi <vchipirras6@gmail.com> Vanessa, please you run git config --global user.name "Vanessa Chipirrás Navalón" so future patches have your full name in the Author field and not just your unix user name. (Also, if there are more fix-ups, feel free to open a new bug :))
Review of attachment 309033 [details] [review]: commit 32942c99d882dec0d6ed0ca4f31f5c83fe2b6b62 Author: Vanessa Chipi <vchipirras6@gmail.com> Date: Mon Aug 10 23:02:12 2015 +0200 facedetect: wrong form to write the delete operator The delete operator is written this way: delete (cascade). This way is misspelled, it is an operator, not a function. Delete the parentheses.
Vanessa, Set git to use your real name as the author of your commits with: git config --global user.name "Vanessa Chipi" I only noticed this after merging the first two, so fixed it in the third one.
Yes, sorry, I forgot it. Already I did it, I will not forget for the next.
Created attachment 309090 [details] [review] Refactor the code Some lines of code are repeated several times, therefore this lines are simplified with a inline function, that this is proper style of C++.
Review of attachment 309090 [details] [review]: ::: ext/opencv/gstfacedetect.cpp @@ +143,3 @@ +inline void +structure_and_message (const vector < Rect > &rectangles, const gchar * name, Here we have structure_and_message() @@ +718,3 @@ "height", G_TYPE_UINT, r.height, NULL); + if (have_nose) + structure (nose, "nose", rnx, rny, filter, s); But we call structure() ? What am I missing ?
Created attachment 309092 [details] [review] Refactor the code Some lines of code are repeated several times, therefore this lines are simplified with a inline function, that this is proper style of C++. I had an oversight before :P. It's fixed.
commit 7a2f5d6b030e414f5d662eb73eb7849e9a5a4a1e Author: Vanessa Chipirrás Navalón <vchipirras6@gmail.com> Date: Wed Aug 12 00:20:26 2015 +0200 facedetect: Refactor the code Some lines of code are repeated several times, therefore this lines are simplified with a inline function, that this is proper style of C++.
Review of attachment 309092 [details] [review]: Thanks Vanessa
:)
Good work :)
Created attachment 309780 [details] [review] Remove unnecessary variable. Memory is reserved for this variable and then released without making any use of it. https://bugzilla.gnome.org/show_bug.cgi?id=748377
Comment on attachment 309780 [details] [review] Remove unnecessary variable. Thanks! commit aa58ff3e46a67796797edf63a66efa9bcbdddc93 Author: Vanessa Chipirrás Navalón <vchipirras6@gmail.com> Date: Fri Aug 21 05:26:25 2015 +0200 facedetect: remove unnecessary variable. Memory is reserved for this variable and then released without making any use of it. https://bugzilla.gnome.org/show_bug.cgi?id=748377
Created attachment 309809 [details] [review] existence of other lines delete operator misspelled. The delete operator is written this way: delete (variable). This way is misspelled, it is an operator, not a function. Then, the parentheses are deleted. https://bugzilla.gnome.org/show_bug.cgi?id=748377
The last correction facedetect.cpp file, I think it's all right now.
Review of attachment 309809 [details] [review]: Normally this patches dirty git blame. But in this case these lines were modified in a recent small commit (0e70f8c94f7fc64774fb8246626d4f11979c99ff). So no harm on cleaning these, in my opinion.