Merge pull request #24028 from VadimLevin:dev/vlevin/fix-flann-python-bindings

VadimLevin · web-flow · commit e3cb5f80e73c · 2023-07-21T12:44:56.000+03:00
Fix FLANN python bindings #24028 As a side-effect this patch improves reporting errors by FLANN `get_param`. resolves #21642 ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake
diff --git a/modules/flann/include/opencv2/flann/any.h b/modules/flann/include/opencv2/flann/any.h
@@ -19,16 +19,39 @@
 #include <ostream>
 #include <typeinfo>
 
+#include "opencv2/core/cvdef.h"
+#include "opencv2/core/utility.hpp"
+
 namespace cvflann
 {
 
 namespace anyimpl
 {
 
-struct bad_any_cast
+struct bad_any_cast : public std::exception
 {
+    bad_any_cast() = default;
+
+    bad_any_cast(const char* src, const char* dst)
+        : message_(cv::format("cvflann::bad_any_cast(from %s to %s)", src, dst)) {}
+
+
+    const char* what() const noexcept override
+    {
+        return message_.c_str();
+    }
+
+private:
+    std::string message_{"cvflann::bad_any_cast"};
 };
 
+#ifndef CV_THROW_IF_TYPE_MISMATCH
+#define CV_THROW_IF_TYPE_MISMATCH(src_type_info, dst_type_info) \
+    if ((src_type_info) != (dst_type_info)) \
+        throw cvflann::anyimpl::bad_any_cast((src_type_info).name(), \
+                                             (dst_type_info).name())
+#endif
+
 struct empty_any
 {
 };
@@ -271,7 +294,7 @@ struct any
     template<typename T>
     T& cast()
     {
-        if (policy->type() != typeid(T)) throw anyimpl::bad_any_cast();
+        CV_THROW_IF_TYPE_MISMATCH(policy->type(), typeid(T));
         T* r = reinterpret_cast<T*>(policy->get_value(&object));
         return *r;
     }
@@ -280,7 +303,7 @@ struct any
     template<typename T>
     const T& cast() const
     {
-        if (policy->type() != typeid(T)) throw anyimpl::bad_any_cast();
+        CV_THROW_IF_TYPE_MISMATCH(policy->type(), typeid(T));
         const T* r = reinterpret_cast<const T*>(policy->get_value(&object));
         return *r;
     }
diff --git a/modules/flann/include/opencv2/flann/general.h b/modules/flann/include/opencv2/flann/general.h
@@ -31,6 +31,8 @@
 #ifndef OPENCV_FLANN_GENERAL_H_
 #define OPENCV_FLANN_GENERAL_H_
 
+#include "opencv2/core/version.hpp"
+
 #if CV_VERSION_MAJOR <= 4
 
 //! @cond IGNORED
diff --git a/modules/flann/include/opencv2/flann/matrix.h b/modules/flann/include/opencv2/flann/matrix.h
@@ -35,6 +35,9 @@
 
 #include <stdio.h>
 
+#include "opencv2/core/cvdef.h"
+#include "opencv2/flann/defines.h"
+
 namespace cvflann
 {
 
diff --git a/modules/flann/include/opencv2/flann/params.h b/modules/flann/include/opencv2/flann/params.h
@@ -72,23 +72,33 @@ struct SearchParams : public IndexParams
 
 
 template<typename T>
-T get_param(const IndexParams& params, cv::String name, const T& default_value)
+T get_param(const IndexParams& params, const cv::String& name, const T& default_value)
 {
     IndexParams::const_iterator it = params.find(name);
     if (it != params.end()) {
-        return it->second.cast<T>();
+        try {
+            return it->second.cast<T>();
+        } catch (const std::exception& e) {
+            CV_Error_(cv::Error::StsBadArg,
+                      ("FLANN '%s' param type mismatch: %s", name.c_str(), e.what()));
+        }
     }
     else {
         return default_value;
     }
 }
 
 template<typename T>
-T get_param(const IndexParams& params, cv::String name)
+T get_param(const IndexParams& params, const cv::String& name)
 {
     IndexParams::const_iterator it = params.find(name);
     if (it != params.end()) {
-        return it->second.cast<T>();
+        try {
+            return it->second.cast<T>();
+        } catch (const std::exception& e) {
+            CV_Error_(cv::Error::StsBadArg,
+                      ("FLANN '%s' param type mismatch: %s", name.c_str(), e.what()));
+        }
     }
     else {
         FLANN_THROW(cv::Error::StsBadArg, cv::String("Missing parameter '")+name+cv::String("' in the parameters given"));
diff --git a/modules/flann/include/opencv2/flann/result_set.h b/modules/flann/include/opencv2/flann/result_set.h
@@ -40,6 +40,9 @@
 #include <set>
 #include <vector>
 
+#include "opencv2/core/base.hpp"
+#include "opencv2/core/cvdef.h"
+
 namespace cvflann
 {
 
diff --git a/modules/flann/misc/python/pyopencv_flann.hpp b/modules/flann/misc/python/pyopencv_flann.hpp
@@ -17,57 +17,89 @@ PyObject* pyopencv_from(const cvflann_flann_distance_t& value)
 template<>
 bool pyopencv_to(PyObject *o, cv::flann::IndexParams& p, const ArgInfo& info)
 {
-    CV_UNUSED(info);
-    bool ok = true;
-    PyObject* key = NULL;
-    PyObject* item = NULL;
-    Py_ssize_t pos = 0;
-
     if (!o || o == Py_None)
+    {
         return true;
+    }
+
+    if(!PyDict_Check(o))
+    {
+        failmsg("Argument '%s' is not a dictionary", info.name);
+        return false;
+    }
+
+    PyObject* key_obj = NULL;
+    PyObject* value_obj = NULL;
+    Py_ssize_t key_pos = 0;
 
-    if(PyDict_Check(o)) {
-        while(PyDict_Next(o, &pos, &key, &item))
+    while(PyDict_Next(o, &key_pos, &key_obj, &value_obj))
+    {
+        // get key
+        std::string key;
+        if (!getUnicodeString(key_obj, key))
         {
-            // get key
-            std::string k;
-            if (!getUnicodeString(key, k))
+            failmsg("Key at pos %lld is not a string", static_cast<int64_t>(key_pos));
+            return false;
+        }
+        // key_arg_info.name is bound to key lifetime
+        const ArgInfo key_arg_info(key.c_str(), false);
+
+        // get value
+        if (isBool(value_obj))
+        {
+            npy_bool npy_value = NPY_FALSE;
+            if (PyArray_BoolConverter(value_obj, &npy_value) >= 0)
             {
-                ok = false;
-                break;
+                p.setBool(key, npy_value == NPY_TRUE);
+                continue;
             }
-            // get value
-            if( !!PyBool_Check(item) )
+            PyErr_Clear();
+        }
+
+        int int_value = 0;
+        if (pyopencv_to(value_obj, int_value, key_arg_info))
+        {
+            if (key == "algorithm")
             {
-                p.setBool(k, item == Py_True);
+                p.setAlgorithm(int_value);
             }
-            else if( PyInt_Check(item) )
+            else
             {
-                int value = (int)PyInt_AsLong(item);
-                if( strcmp(k.c_str(), "algorithm") == 0 )
-                    p.setAlgorithm(value);
-                else
-                    p.setInt(k, value);
+                p.setInt(key, int_value);
             }
-            else if( PyFloat_Check(item) )
+            continue;
+        }
+        PyErr_Clear();
+
+        double flt_value = 0.0;
+        if (pyopencv_to(value_obj, flt_value, key_arg_info))
+        {
+            if (key == "eps")
             {
-                double value = PyFloat_AsDouble(item);
-                p.setDouble(k, value);
+                p.setFloat(key, static_cast<float>(flt_value));
             }
             else
             {
-                std::string val_str;
-                if (!getUnicodeString(item, val_str))
-                {
-                    ok = false;
-                    break;
-                }
-                p.setString(k, val_str);
+                p.setDouble(key, flt_value);
             }
+            continue;
         }
-    }
+        PyErr_Clear();
 
-    return ok && !PyErr_Occurred();
+        std::string str_value;
+        if (getUnicodeString(value_obj, str_value))
+        {
+            p.setString(key, str_value);
+            continue;
+        }
+        PyErr_Clear();
+        // All conversions are failed
+        failmsg("Failed to parse IndexParam with key '%s'. "
+                "Supported types: [bool, int, float, str]", key.c_str());
+        return false;
+
+    }
+    return true;
 }
 
 template<>
diff --git a/modules/flann/misc/python/test/test_flann_based_matcher.py b/modules/flann/misc/python/test/test_flann_based_matcher.py
@@ -0,0 +1,30 @@
+#!/usr/bin/env python
+# Python 2/3 compatibility
+from __future__ import print_function
+
+import cv2
+import numpy as np
+
+from tests_common import NewOpenCVTests
+
+
+class FlannBasedMatcher(NewOpenCVTests):
+    def test_all_parameters_can_be_passed(self):
+        img1 = self.get_sample("samples/data/right01.jpg")
+        img2 = self.get_sample("samples/data/right02.jpg")
+
+        orb = cv2.ORB.create()
+
+        kp1, des1 = orb.detectAndCompute(img1, None)
+        kp2, des2 = orb.detectAndCompute(img2, None)
+        FLANN_INDEX_KDTREE = 1
+        index_param = dict(algorithm=FLANN_INDEX_KDTREE, trees=4)
+        search_param = dict(checks=32, sorted=True, eps=0.5,
+                            explore_all_trees=False)
+        matcher = cv2.FlannBasedMatcher(index_param, search_param)
+        matches = matcher.knnMatch(np.float32(des1), np.float32(des2), k=2)
+        self.assertGreater(len(matches), 0)
+
+
+if __name__ == '__main__':
+    NewOpenCVTests.bootstrap()

Original file line number	Diff line number	Diff line change
`@@ -35,6 +35,9 @@`
`35`	`35`
`36`	`36`	`#include <stdio.h>`
`37`	`37`
	`38`	`+#include "opencv2/core/cvdef.h"`
	`39`	`+#include "opencv2/flann/defines.h"`
	`40`	`+`
`38`	`41`	`namespace cvflann`
`39`	`42`	`{`
`40`	`43`
Original file line number	Diff line number	Diff line change
`@@ -40,6 +40,9 @@`
`40`	`40`	`#include <set>`
`41`	`41`	`#include <vector>`
`42`	`42`
	`43`	`+#include "opencv2/core/base.hpp"`
	`44`	`+#include "opencv2/core/cvdef.h"`
	`45`	`+`
`43`	`46`	`namespace cvflann`
`44`	`47`	`{`
`45`	`48`