Skip to content

test_stereomatching.cpp: validate min disparity affect on valid ROI #9854

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 31, 2017
Merged

test_stereomatching.cpp: validate min disparity affect on valid ROI #9854

merged 1 commit into from
Oct 31, 2017

Conversation

opalmirror
Copy link
Contributor

@opalmirror opalmirror commented Oct 13, 2017 •
edited by alalek
Loading

Merge with extra: opencv/opencv_extra#394

This pull request adds stereomatching tests to the master branch for different settings of StereoBM::minDisparity.

  • An additional parameter, minDisparities, is expected in the StereoBM test cases under opencv_extra/testdata

  • The valid disparity region of interest (ROI) for both StereoBM and StereoSGBM is calculated and validated as well.

It requires a corresponding commit in the opencv_extra repository, commit id b201581 (which will be the subject of another pull request, opencv/opencv_extra#394, same branch name, test_stereo_min_disparity_master).

This pull request is a follow up to pull request #9816 which fixed issue #9783 and was suggested by @alalek (thanks again).

@opalmirror
Copy link
Contributor Author

@alalek I looked at each automated build, and verified that the extra test cases were run and everything passed.

@alalek
Copy link
Member

alalek commented Oct 13, 2017 •
edited
Loading

@opalmirror Well done! Thank you! 👍

@opalmirror opalmirror mentioned this pull request Oct 16, 2017
Merged
@opalmirror
Copy link
Contributor Author

opalmirror commented Oct 16, 2017 •
edited
Loading

@alalek note the optimal merge order here is:

  1. fix StereoBM disparity map right margin truncation when minDisparitie… #9844 add StereoBM fix to 2.4 branch (creating a new 2.4 branch merge commit ID)

  2. test_stereomatching.cpp: validate min disparity affect on valid ROI #9854 (this commit) and stereomatching: add validation of min disparity affect on valid ROI opencv_extra#394 to add testing to master branch of both opencv and extras

  3. A new PR (I need to wait for 1. above to rebase it) which updates the 2.4 branch to pass tests (a backport of the changes in test_stereomatching.cpp: validate min disparity affect on valid ROI #9854).

So, before creating a pull request for 3. I'm waiting for 1. to happen.

@opalmirror
Copy link
Contributor Author

@alalek I am curious when this may be merged. Not frustrated, just wondering if you have an ETM, and sending a gentle ping. Thanks! James

@opalmirror
Copy link
Contributor Author

opalmirror commented Oct 30, 2017 •
edited
Loading

@alalek, thanks for the merge of #9844 to 2.4 backport branch. I've pulled that and am rebasing my backport for test to the 2.4 stable branch. When that's done (an hour or so) I'll pass along the new PR number to commit along with the master branch PRs #9854 and opencv/opencv_extra#394 .

@opalmirror opalmirror mentioned this pull request Oct 30, 2017
Merged
@opalmirror
Copy link
Contributor Author

@alalek, the functional changes to backport this PR to 2.4 is now tested and filed. It is PR #9974. The optimal merge sequence is:

Thanks! I've enjoyed contributing.

@opencv-pushbot opencv-pushbot merged commit 4eb9f17 into opencv:master Oct 31, 2017
opencv-pushbot pushed a commit that referenced this pull request Oct 31, 2017
@vpisarev
Merge pull request #9854 from opalmirror:test_stereo_min_disparity_ma…
87984f2 
…ster
@opalmirror opalmirror deleted the test_stereo_min_disparity_master branch November 1, 2017 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@alalek alalek

Labels
port/backport done Label for maintainers. Authors of PR can ignore this
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@opalmirror @alalek @opencv-pushbot