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

ROI returned by cvGetOptimalNewCameraMatrix has wrong width & height #24831

Open
4 tasks done
devernay opened this issue Jan 8, 2024 · 6 comments · May be fixed by #24927
Open
4 tasks done

ROI returned by cvGetOptimalNewCameraMatrix has wrong width & height #24831

devernay opened this issue Jan 8, 2024 · 6 comments · May be fixed by #24927
Assignees
Labels
bug category: calib3d needs investigation Collect and attach more details (build flags, stacktraces, input dumps, etc)
Milestone

Comments

@devernay
Copy link
Contributor

devernay commented Jan 8, 2024

System Information

OpenCV python version: 4.8.0.76
Operating System / Platform: macOS
Python version: 3.10.6

Detailed description

I noticed here that cvGetOptimalNewCameraMatrix returns (0,0,width-1,height-1) when the distortion parameters are zero.

Looking more closely at the code, I see that the internal function icvGetRectangles finds the rectangles by sampling points in the (0,width-1)x(0,hieght-1) range, and then computes the output ROI sizes as (maxX-minX,maxY-minY), which of course misses one pixel in each dimension!

I think the code in icvGetRectangles should add 1 pixel to both dimensions, because the max value should be included.

Steps to reproduce

Test code:

import cv2
import numpy as np
# zero distortion
distortion_params = np.zeros(8)
width=1920
height=1080
ffactor=1.6
# intrinsics with a centered principal point
K1 = np.array([[width*ffactor, 0, (width-1)*0.5],
              [0,width*ffactor, (height-1)*0.5],
              [0,0,1]])
# intrinsics with a non-centered principal point
K2 = np.array([[width*ffactor, 0, (width-1)*0.5+0.05],
              [0,width*ffactor, (height-1)*0.5-0.04],
              [0,0,1]])
for K in [K1,K2]:
    newK, roi = cv2.getOptimalNewCameraMatrix(K, distortion_params, (width, height), 0)
    print(np.linalg.norm(K-newK))
    print(roi)

Output:

4.547473508864641e-13
(0, 0, 1919, 1079)
4.547473508864641e-13
(0, 0, 1919, 1079)

both rois should be (0, 0, 1920, 1080).

Issue submission checklist

  • I report the issue, it's not a question
  • I checked the problem with documentation, FAQ, open issues, forum.opencv.org, Stack Overflow, etc and have not found any solution
  • I updated to the latest OpenCV version and the issue is still there
  • There is reproducer code and related data files (videos, images, onnx, etc)
@devernay devernay added the bug label Jan 8, 2024
@devernay devernay changed the title ROI returned by cvGetOptimalNewCameraMatrix has wront width & height ROI returned by cvGetOptimalNewCameraMatrix has wrong width & height Jan 8, 2024
@devernay
Copy link
Contributor Author

devernay commented Jan 8, 2024

note: previous icvGetRectangles fix, from 4.8.0: #23305

@asmorkalov asmorkalov added category: calib3d needs investigation Collect and attach more details (build flags, stacktraces, input dumps, etc) labels Jan 9, 2024
@asmorkalov asmorkalov added this to the 4.10.0 milestone Jan 9, 2024
@asmorkalov asmorkalov self-assigned this Jan 9, 2024
@f-dy
Copy link

f-dy commented Jan 9, 2024

bug introduced here:

#23305 changed here the pixel sampling from 0 to width-1, which is good, but then the ROI width should be maxX-minX+1, not maxX-minX

@vrabaud
Copy link
Contributor

vrabaud commented Jan 25, 2024

The error is actually a bit different. Adding +1 is wrong as the rectangle is a Rect_<double> (switching it where you suggest gets the Calib3d_GetOptimalNewCameraMatrixNoDistortion.accuracy test to fail). The problem arises when converting the Rect to ints for the output. I am preparing a PR.

@devernay
Copy link
Contributor Author

Yes it should do floor(x-0.5) on the minimum bound and ceil(x+0.5) on the second

@devernay
Copy link
Contributor Author

Or something like that. To take into account the footprint of a pixel

@gchhablani
Copy link

Are there any updates on this one?
Would be nice to get this fix.
Thanks!

@asmorkalov asmorkalov modified the milestones: 4.10.0, 4.11.0 Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug category: calib3d needs investigation Collect and attach more details (build flags, stacktraces, input dumps, etc)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants