How to contribute

Version 3 (Kirill Kornyakov, 2014-07-14 06:01 pm) → Version 4/52 (Alexander Shishkov, 2014-07-14 06:01 pm)

h1. How to contribute

*Under construction*

You can:
# Say "thanks":http://sourceforge.net/projects/opencvlibrary/reviews/.
# Help others on "forum":http://tech.groups.yahoo.com/group/OpenCV.
# Provide feedback: report bugs, request features.
# Submit a code: create a ticket with the _Patch_ tracker (issue type).
# -Donate- You should wait for the OpenCV Foundation.

*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.*

Hello!

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:

# 0. A prerequisite: is the algorithm patented? If yes, then we may not be that interested in putting it in.
#


1.
License - the current license is fine, it's compatible with OpenCV.
#


2.
Interface. Here some work is needed to make this code a seamless part of OpenCV.
** - @M_PI -> CV_PI@
** You should - 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.
** Do not use k prefix in constant names. Put enumerations inside FreakDescriptorExtractor class.
- "typedef unsigned char uint8" should be removed. it's duplication of "uchar" OpenCV type.
- structures PatternPoint, DescriptionPair, OrientationPair, PairStat, sortMean should be removed from the header
-
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.
** - "CV_INLINE" in "uchar meanIntensity(...)" declaration should be dropped.
-
"m_" in member names should be dropped, we do not this convention in OpenCV.

- m_patternLookup, m_descriptionPairs, m_orientationPairs can be turned into Mat's or be put (probably, with some other members) into hidden structure:

<pre>
class CV_EXPORTS FreakDescriptorExtractor : public DescriptorExtractor
{
...
protected:
struct FreakDescriptorExtractorImpl* impl;
};

# Implementation.
**
</pre>

- 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.

- 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.

3. implementation.

-
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".
**


-
Avoiding duplication of existing OpenCV functionality.
**
There is already more or less efficient normHamming() in OpenCV. Please, use it instead of hammingseg.h

-
Writing to cerr should be replaced with CV_Error() calls.
# Documentation.
** Documentation


4. documentation.

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.
# You should provide


5.
sample code. code - you have it already.

it needs to be renamed (e.g. to freak_demo.cpp)

** help() function should be added, check opencv/samples/cpp.
# Regression the commented out section should probably be uncommented and executed when user passes some optional parameter.
no USE_SSE is allowed in samples, they should be written in completely platform-abstract way whenever possible
kResPath should be made command line parameter (maybe optional)

6. regression
tests - the code should include some regression tests.
** Please,


please,
take a look at opencv/modules/features2d/test/test_features2d.cpp. 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 your code.
# Integration
**
Freak.

7. add the Algorithm registration.
Take a look at features2d_init.cpp.

8. probe
the integration. After you've done 1-7, you may do a test integration of the descriptor.

Take the
latest snapshot of the SVN trunk
** Add your code the class FreakDescriptorExtractor to features2d.hpp
Add
the snaphot algorithm registration to features2d_init.cpp
** Put freak.cpp file to features2d/src/, put #include "precomp.hpp" in the beginning
Insert documentation and add
Add
the test to the corresponding module test_features2d.cpp.
** 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.
** Create patch file and either send it to me by e-mail or submit it as patch at code.opencv.org.

--
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.

Do not hesitate to ask any questions (now and/or during the process, if you decide to go ahead with integration)

Regards,
Vadim