nerfstudio
nerfstudio copied to clipboard
nerfstudio-project
[Full images datamanager] fix image indexing with cv.undistort and implement eval indices logic
Two things:
1) fix new undistorted image shape not being the same as input
from my understanding in newK, roi = cv2.getOptimalNewCameraMatrix(K, distortion_params, (image.shape[1], image.shape[0]), 0) the roi that is returned is in pixel notations (px_start, py_start, px_end, py_end)
When slicing an image we should instead do image[y : y + h + 1, x : x + w + 1] to preserve the same shape as the input image. Otherwise we crop the last pixel and output_image.shape != input_image.shape :D
2) add eval indices logic from config
There are a few nice eval options in the config that haven't been implemented. I implemented an easy version for eval_num_images_to_sample_from and eval_image_indices which I think are the most important ones for evaluating.
also added option for mipnerf360 style evaluation in colmap dataparser to hold out every n'th image in train set for eval dataset. Useful for comparing with inria eval scripts
Reverted sampling strategies in colmap dataparser in favour of the new PR #2709 which also implements them. This PR fixes 1) the output image size bug after cv.undistort and 2) implements the logic for "eval_image_indices" and "eval_num_images_to_sample_from" in the datamanager config.
isn't it OK that after the undistortion the shape of the image changes? Maybe the fact that in Mip-NeRF 360 the images are already undistorted, and the second undistortion caused a loss of pixel (prior to PR #2726), lead to this change?
@maturk For the first issue: do you have sources indicating the roi is in (left, upper, right, lower) convention? I think it is in x, y, w, h convention. If you search code on github for sample codes of using cv2.getOptimalNewCameraMatrix(), the current implementation is correct.
The fix should be done upstream: w and h returned by the latest version of icvGetRectangles are off by 1 pixel, see https://github.com/opencv/opencv/issues/24831
@f-dy okay wow this is actually pretty crazy XD Never would I have thought that I would find a bug in OpenCV! Thank you for making the issue and even tracking down the problem in the source code.
I would recommend adding a unit test to test_datamanager.py, so that we're sure to get the thing right in the end.
And btw we should probably not use cv.undistort, which uses bilinear interpolation, and write our own function that calls cv2.remap with INTER_CUBIC instead. bilinear interpolation creates uneven blur in the image, especially for small distortion amounts, i.e with a small k1 you would see rings of blur in the undistorted images. I'll add some code that demonstrates that
Still an issue, even with 4.9. Hopefully will be patched in the future, following the issue that @f-dy opened.
@maturk Let's wait opencv's team to fix this bug, I believe they will fix it very soon. Once they fix the issue, we can bump up opencv version dependency again.
Any suggestions what we should do with this issue with the cropped pixel?
gate the fix you did with if 4.8<=opencv_version<4.10, since the issue is marked for OpenCV 4.10, and add a unit test, so that we can adjust the gate if they don't fix it in 4.10
OpenCV 4.10 is out now, and with that I'd like to add a small request: Can we loosen up the OpenCV dependency ever so slightly? Specifically, can we loosen up at least the 4th digit of the version requirement? The reason being, Conda cannot select package versions down to the 4th digit and if you install COLMAP via Conda/-forge, it will pull in opencv-python automatically. You can restrict the version via Conda, but not down to the 4th digit. Pip, when installing Nerfstudio, cannot modify the opencv-python version installed by Conda, and because of how tight the version check is, it will fail to install. The only way around this right now is to manually modify the pyproject.toml file to loosen the version check, but this of course only works for installations from git, not from pypi. So basically: If we could loosen up the version constraints for opencv-python at least to the point where it doesn't care for the 4th version digit (though ideally even looser), this would reduce conda woes quite a bit and simplify installation requirements.
OpenCV 4.10 is out now, and with that I'd like to add a small request: Can we loosen up the OpenCV dependency ever so slightly? Specifically, can we loosen up at least the 4th digit of the version requirement? The reason being, Conda cannot select package versions down to the 4th digit and if you install COLMAP via Conda/-forge, it will pull in opencv-python automatically. You can restrict the version via Conda, but not down to the 4th digit. Pip, when installing Nerfstudio, cannot modify the opencv-python version installed by Conda, and because of how tight the version check is, it will fail to install. The only way around this right now is to manually modify the pyproject.toml file to loosen the version check, but this of course only works for installations from git, not from pypi. So basically: If we could loosen up the version constraints for opencv-python at least to the point where it doesn't care for the 4th version digit (though ideally even looser), this would reduce conda woes quite a bit and simplify installation requirements.
Yeah sounds good to me @SharkWipf , can you do a PR?