Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove normalization in cv::stereo::QuasiDenseStereo getDisparity(). #2869

Merged
merged 2 commits into from
Mar 1, 2021

Conversation

dimitrisPs
Copy link
Contributor

@dimitrisPs dimitrisPs commented Feb 13, 2021

This addresses user comments made in #1941 and #2819. The getDisparity function, of QuasiDenseStereo, originally returned a disparity image normalized to span the full range between 0 and 255, which was improper and understandably confused users.

I've made the following changes:

  • getDisparity now returns proper CV_32F disparity maps with nan values for unknown matches.
  • included an accuracy test.

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 Apache 2 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

…parity maps with nan values for unknown matches.
@dimitrisPs dimitrisPs changed the title Remove normalization in getDisparity(). Remove normalization in cv::stereo::QuasiDenseStereo getDisparity(). Feb 16, 2021
Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contribution!

//
//
// Intel License Agreement
// For Open Source Computer Vision Library
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use short license header: https://github.com/opencv/opencv/wiki/Coding_Style_Guide#file-structure

// This file is part of OpenCV project.
// It is subject to the license terms in the LICENSE file found in the top-level directory
// of this distribution and at http://opencv.org/license.html.


namespace opencv_test { namespace {

class CV_QdsMatchingTest : public cvtest::BaseTest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cvtest::BaseTest

BTW, It makes sence to avoid using of legacy OpenCV testing infrastructure.
Prefer to use GoogleTest directly.

Start from this and then replace legacy calls (like printf => EXPECT_*() / ASSERT_*()):

-class
 ...

-void CV_QdsMatchingTest::run(int)
+TEST(qds_matching_simple_test, accuracy)
 ...

-TEST(qds_matching_simple_test, accuracy) { CV_QdsMatchingTest test; test.safe_run(); }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for that, I used the test files from the other stereo modules as templates, when I was writing qds' test, assuming that those follow the guidelines. I will make changes later this week, thanks for your help.

@dimitrisPs dimitrisPs requested a review from alalek March 1, 2021 11:04
@alalek alalek merged commit 0165d48 into opencv:master Mar 1, 2021
@alalek alalek mentioned this pull request Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants