How to contribute

Version 3 (Daniil Osokin, 2014-07-14 06:01 pm)

1 1
h1. How to contribute
2 1
3 1
*Under construction*
4 1
5 1
You can:
6 1
# Say "thanks":http://sourceforge.net/projects/opencvlibrary/reviews/.
7 1
# Help others on "forum":http://tech.groups.yahoo.com/group/OpenCV.
8 1
# Provide feedback: report bugs, request features.
9 1
# Submit a code: create a ticket with the _Patch_ tracker (issue type).
10 2 Daniil Osokin
# -Donate- You should wait for the OpenCV Foundation.
11 3 Daniil Osokin
12 3 Daniil Osokin
*Below are recommendations for some contributors. They are raw, but can be used as a starting point for anybody who wants to see his code added to the library.*
13 3 Daniil Osokin
14 3 Daniil Osokin
Hello!
15 3 Daniil Osokin
16 3 Daniil Osokin
We would gladly accept such a contribution, but because of little resources available on our side, I would ask you to do all the preparations. Here is the checklist, including both generic items and the specific things related to your code that I found after brief inspection:
17 3 Daniil Osokin
18 3 Daniil Osokin
0. A prerequisite: is the algorithm patented? If yes, then we may not be that interested in putting it in.
19 3 Daniil Osokin
20 3 Daniil Osokin
1. License - the current license is fine, it's compatible with OpenCV.
21 3 Daniil Osokin
22 3 Daniil Osokin
2. Interface. Here some work is needed to make this code a seamless part of OpenCV.
23 3 Daniil Osokin
- @M_PI -> CV_PI@
24 3 Daniil Osokin
- remove all the static constants from the header. it's not portable and generally a bad practice. Use defines (which should have prefix CV_FREAK_ or something) and enumerations.
25 3 Daniil Osokin
Do not use k prefix in constant names. Put enumerations inside FreakDescriptorExtractor class.
26 3 Daniil Osokin
- "typedef unsigned char uint8" should be removed. it's duplication of "uchar" OpenCV type.
27 3 Daniil Osokin
- structures PatternPoint, DescriptionPair, OrientationPair, PairStat, sortMean should be removed from the header
28 3 Daniil Osokin
- cv:: and std:: should not be used in the header, since all the stuff is already inside cv namespace and we already have "using std::vector", "using std::string" directives.
29 3 Daniil Osokin
- "CV_INLINE" in "uchar meanIntensity(...)" declaration should be dropped.
30 3 Daniil Osokin
- "m_" in member names should be dropped, we do not this convention in OpenCV.
31 3 Daniil Osokin
32 3 Daniil Osokin
- m_patternLookup, m_descriptionPairs, m_orientationPairs can be turned into Mat's or be put (probably, with some other members) into hidden structure:
33 3 Daniil Osokin
34 3 Daniil Osokin
<pre>
35 3 Daniil Osokin
        class CV_EXPORTS FreakDescriptorExtractor : public DescriptorExtractor
36 3 Daniil Osokin
        {
37 3 Daniil Osokin
        ...
38 3 Daniil Osokin
        protected:
39 3 Daniil Osokin
        struct FreakDescriptorExtractorImpl* impl;
40 3 Daniil Osokin
        };
41 3 Daniil Osokin
</pre>
42 3 Daniil Osokin
43 3 Daniil Osokin
- reading/writing point pairs from a file looks not very good. First of all, in mobile world reading data from external files may be inefficient and inconvenient. It's also inconvenient for redistribution of final applications that may use this functionality. Why not just drop all the I/O and just pass optional const vector<int>& to the constructor? If the final application is written in Python, Java, C#, Objective-C etc., it's super-easy to read those array from application resources, xml file, a database etc.
44 3 Daniil Osokin
45 3 Daniil Osokin
- drawPattern is purely debugging method, I suggest to remove it from the external interface (make it a method of FreakDescriptorExtractorImpl?) and comment it off in the implementation, so that it can be restored if needed.
46 3 Daniil Osokin
47 3 Daniil Osokin
3. implementation.
48 3 Daniil Osokin
49 3 Daniil Osokin
- The code should be portable, it should compile fine on ARM too, for example. Therefore, you should not use SSE2 intrinsics outside of conditional compilation. Instead of "#ifdef USE_SSE" you should use "#if CV_SSE2".
50 3 Daniil Osokin
51 3 Daniil Osokin
- Avoiding duplication of existing OpenCV functionality. There is already more or less efficient normHamming() in OpenCV. Please, use it instead of hammingseg.h
52 3 Daniil Osokin
53 3 Daniil Osokin
- Writing to cerr should be replaced with CV_Error() calls.
54 3 Daniil Osokin
55 3 Daniil Osokin
4. documentation.
56 3 Daniil Osokin
57 3 Daniil Osokin
documentation in RST format should be provided for the functionality. Check http://docs.opencv.org/modules/features2d/doc/feature_detection_and_description.html#orb as example.
58 3 Daniil Osokin
59 3 Daniil Osokin
5. sample code - you have it already.
60 3 Daniil Osokin
61 3 Daniil Osokin
it needs to be renamed (e.g. to freak_demo.cpp)
62 3 Daniil Osokin
help() function should be added, check opencv/samples/cpp.
63 3 Daniil Osokin
the commented out section should probably be uncommented and executed when user passes some optional parameter.
64 3 Daniil Osokin
no USE_SSE is allowed in samples, they should be written in completely platform-abstract way whenever possible
65 3 Daniil Osokin
kResPath should be made command line parameter (maybe optional)
66 3 Daniil Osokin
67 3 Daniil Osokin
6. regression tests - the code should include some regression tests.
68 3 Daniil Osokin
69 3 Daniil Osokin
please, take a look at opencv/modules/features2d/test/test_features2d.cpp, Features2d_DescriptorExtractor_BRIEF or _ORB in particular (since they also use hamming distance). You will need to add a similar test for Freak.
70 3 Daniil Osokin
71 3 Daniil Osokin
7. add the Algorithm registration. Take a look at features2d_init.cpp.
72 3 Daniil Osokin
73 3 Daniil Osokin
8. probe the integration. After you've done 1-7, you may do a test integration of the descriptor.
74 3 Daniil Osokin
75 3 Daniil Osokin
Take the latest snapshot of the SVN trunk
76 3 Daniil Osokin
Add the class FreakDescriptorExtractor to features2d.hpp
77 3 Daniil Osokin
Add the algorithm registration to features2d_init.cpp
78 3 Daniil Osokin
Put freak.cpp file to features2d/src/, put #include "precomp.hpp" in the beginning
79 3 Daniil Osokin
Insert documentation and
80 3 Daniil Osokin
Add the test to test_features2d.cpp.
81 3 Daniil Osokin
Build OpenCV, run tests. If you have access to both Linux and Windows - great! Sometimes GCC-approved code does not built with MSVC and vice versa.
82 3 Daniil Osokin
Create patch file and either send it to me by e-mail or submit it as patch at code.opencv.org.
83 3 Daniil Osokin
84 3 Daniil Osokin
--
85 3 Daniil Osokin
It may look like a lot of work to do. And it is. Earlier we did such refactoring and cleanups many times by ourselves. Some contributions were just put as-is, without tests, documentation, samples etc. and now we have no idea what to do with it - either remove completely such immature functionality or put it to legacy and let it die slowly. Not to let this happen again, we decided to raise the bar and accept only well-prepared code into the library. We also have to ask contributors to do this work.
86 3 Daniil Osokin
87 3 Daniil Osokin
Do not hesitate to ask any questions (now and/or during the process, if you decide to go ahead with integration)
88 3 Daniil Osokin
89 3 Daniil Osokin
Regards,
90 3 Daniil Osokin
Vadim