Pev fix lsh bad any cast

[pemmanuelviel]

closes #17628

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

• I agree to contribute to the project under OpenCV (BSD) License.
• To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
• The PR is proposed to proper branch
• There is reference to original bug report and related work
• There is accuracy test, performance test and test data in opencv_extra repository, if applicable
  Patch to opencv_extra has the same branch name.
• The feature is well documented and sample code can be built with the project CMake

allow_multiple_commits=1

[alalek]

Thank you for the contribution!

Could you please add simple test case for this? (which triggers this exception)

TEST(Flann_LshTable, regression_bad_any_cast)
{
    LshIndexParams params(...);
    ...
    EXPECT_NO_THROW(... check expression ...);
}

Test is useful to avoid similar regressions in the future.

[alalek]

Lets do not change constructor parameters (preserve source and ABI compatibility).

I believe we can fix that in this way (without changing of struct fields / constructors):

-(*this)["table_number"] = table_number;
+(*this)["table_number"] = (int)table_number;

• change type for "key_size", "multi_probe_level".

[pemmanuelviel]

Good catch, thanks.
I've made the change to see if this passes the tests now.
Adding the test will be my next task.

[alalek]

Please revert these changes too

[pemmanuelviel]

I've squashed all these commits in a new branch "pev--fix-bad-any-cast-in-lsh" based on 3.4. It includes a test.
Finally we don't change any unsigned int for int type for table_number_, key_size_ and multi_probe_level_.
However, as

• in all the others types of IndexParams the types are int and not unsigned int
• and the version of these three parameters in LshIndexParams of miniflann is int and not unsigned int,
  I would suggest for consistency to switch to int for master, what is actually done by the "pev-fix-lsh-bad-any-cast" (one dash and not two after pev to easily identify between master based branched and 3.4 based ones)

[pemmanuelviel]

@alalek your forced push let the members declared as int instead of their original unsigned int type (the revert mentioned earlier concerning the few lines after 378).
I've pushed another commit (b0315a7) to change that on the branch, but it doesn't seem to appear in the PR.

[alalek]

@pemmanuelviel According to statement from #17684:

in all the others types of IndexParams the types are int an not unsigned int

lets change (keep from original patch) "int" type for fields.

I believe we are ready to merge this PR in the current state.

[pemmanuelviel]

@alalek I was not sure you wanted to keep the change.
So yes, we can merge this PR without my latest commit b0315a7.

Talking about consistency between uint and int, what do you think about my suggestion to merge PR 17628 on master to change also the LshIndexParams arguments type so they become int

• like for others xxxIndexParams,
• and are identical to the ones of the LshIndexParams in miniflann.hpp (l.121) and miniflann.cpp (l.276)?

[alalek]

PR on master (#17628) will be closed after merging this PR and after regular merge of fixes from 3.4 to master branch.

Which type is currently used in upstream flann project for smilar fields in xxxIndexParams?

[pemmanuelviel]

@alalek
All of them, except LshIndexParams, are using signed integers in their respective header files:

• CompositeIndexParams(int trees = 4, int branching = 32, int iterations = 11,
• HierarchicalClusteringIndexParams(int branching = 32,
• KDTreeIndexParams(int trees = 4)
• KDTreeSingleIndexParams(int leaf_max_size = 10, bool reorder = true, int dim = -1)
• KMeansIndexParams(int branching = 32, int iterations = 11,

And for their mapping in miniflann, even LshIndexParams use signed integers.

[alalek]

@pemmanuelviel Thank you for information! Actually my question was about int table_number_ fields from LshIndex.

[alalek]

Thank you for contribution!

[pemmanuelviel]

@alalek Each type of tree has its own set of parameters, with some equivalences for some of them.
Thus "table_number" is the equivalent for LSH of "trees" for KDTree and "branching" for KMeans and HierachicalClustering.

[alalek]

Third commit is omitted due to GitHub todays incident.
PR is merged with commit cdac7c7

https://github.com/opencv/opencv/pull/17640/commits/b0315a74a6a91b1446ee56a47b67fe6e675b95ec

This commit does not belong to any branch on this repository.

Still broken.

PR is not marked as merged automatically due GitHub malfunction. Closing...