Hey Jiri I have a probably silly question:). Can you explain what the ABI problem is when adding the function?

Problem is that I need to add either bool member (in similar way to FeaturesMatcher) is_thread_safe_ or add a virtual fuction for finders to override. Both breaks the ABI.

Problem is that not all finders are thread-safe (SurfFeaturesFinderGpu). Furthermore it does not make any sense to run GPU finders in parallel, but that it probably not that big problem, since the overhead with TBB is very low.

I must admit I don't like this intrusive change too much. I have been reviewing the code and the thing that seems to be thread-unsafe is SurfFeaturesFinderGpu.

Moreover I don't any aparent reason for SurfFeaturesFinderGpu to be thread-unsafe, it just uses member variables instead of local ones.

I'm thinking about different approach to this, something like:

• make SurfFeaturesFinderGpu thread-safe (without breaking ABI)
• run in paralllel for everything

What do you think? Is it acceptable to always run in parallel even for GPU matchers (considering that overhead for TBB is small)?

I am a not GPU expert but it seems nontrivial to make SurfGPU thread-safe according to http://code.opencv.org/issues/3591. However, (I guess) it might be possible to parallel SurfFeaturesFinderGpu if multiple GPU exists but lock the GPU detection to run sequentially for single GPU.

Also if you just want a field to mark thread-safety, do you think sth like this works?

template <typename Finder> class FindFeaturesBody;

template <typename Finder>
class IsThreadSafe { static bool value = false; };
template <> class IsThreadSafe<SurfFeaturesFinderGpu> { static bool value = true; }

I don't think that could solve our problem. We need some runtime mechanism to know in Finder if implementation is thread-safe or not. I.e. if this case IsThreadSafe would always return false because it would be always expanded only for Finder.

I'm thinking about dynamic_cast which would allow us to reuse existing info in class to implement switch-style hack without breaking the ABI. It is a horrible hack, but I don't see any better solution.

I mean template <typename F> class FindFeaturesBody to have templated constructor parameters. So the class know the exact Finder type.

template <typename F> class FindFeaturesBody {
    FindFeaturesBody(F &finder, InputArrayOfArrays images, std::vector<ImageFeatures> &features, const std::vector<std::vector<cv::Rect> > *rois);
}; // You can even remove finder parameter and create a finder member of type F instead

or like:

template <typename F> class FindFeaturesBody {
    FindFeaturesBody(InputArrayOfArrays images, std::vector<ImageFeatures> &features, const std::vector<std::vector<cv::Rect> > *rois);
    F finder;
};
template <> class FindFeaturesBody< SurfFeaturesFinderGpu> {
    // Some warning
};

Yes, I agree that runtime mechanism is better and more consistent with the OpenCV style but looks like dynamic_cast is the only you can do.

I still don't see how this construct could solve our problem. Since the only type info I know compile time is Finder and its implementation is assigned at runtime I don't how template could help us.

In current implementation if I would simply use your templated FindFeaturesBody it would be always only used as FindFeaturesBody<Finder>.

If I would put the implemetation to derived class, then I have everything I need, but then I need some mechanism to call this method in derived class. Adding virtual method would break the ABI, so we have the same problem we are trying to solve.

Am I missing some trick?

this PR will be reopened because of CI issues