[camera_calibration] fix/feat: enable to select range policy for 16 bit images#841
Open
HiroIshida wants to merge 1 commit intoros-perception:noeticfrom
Open
[camera_calibration] fix/feat: enable to select range policy for 16 bit images#841HiroIshida wants to merge 1 commit intoros-perception:noeticfrom
HiroIshida wants to merge 1 commit intoros-perception:noeticfrom
Conversation
20d0709 to
8ef62b7
Compare
Author
|
kindly ping @JWhitleyWork |
8ef62b7 to
fdbb8b1
Compare
Author
|
kindly ping @JWhitleyWork @vrabaud |
fdbb8b1 to
84bb7e3
Compare
Author
|
kindly ping @JWhitleyWork @vrabaud @mikeferguson |
Member
|
I'm not actually a maintainer for the ROS 1 branches - just ROS 2. I'm not sure there are any currently active ROS 1 maintainers... |
Author
|
Understand.. thank you |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feature description
cc. and thanks: @pazeshun, @iory, @tkmtnt7000
This PR enables the selection of a range policy for 16-bit images, offering choices among "upper", "dynamic", and "legacy". Actually, this fixes a bug (or incompatibility with old device) introduced by #334, which is described in background and context in detail.
When "upper" is selected, the upper bit is used (as in the current implementation). If "dynamic" is chosen, a dynamic range utilizing the min/max value is adopted. For "legacy", clamping to 255 is applied.
By the way, for our device (kinect-v1) legacy mode produce most stable for IR calibration.
Dynamic range determination is similar to the one adopted in rviz https://github.com/ros-visualization/rviz/blob/b466cb9d21a78170d9031cbbc7cc5ecc9886af52/src/rviz/image/ros_image_texture.cpp#L115

The below is the screenshot that compares the
cameracalibrator.py's output (left) and rviz visualization of mono16 IR image(right), showing the consistency with our implementation and rviz one.cameracalibration.py (left), rviz (right)
background and context
I suspect that the reason for the "legacy" method clamping the image to a range of 0-255 is due to older cameras (e.g., Kinect v1) producing relatively dark IR images. However, more recent cameras may use brighter IR images, which can result in "white-out" issues, as pointed out in this pull request #334.
However, that PR make it incompatible with less-bright IR cameras. The original motivation of this PR is make this re-compatible with less-bright IR cameras. We still use kinect-v1 and tried to calibrate the IR camera of it following the instruction in https://wiki.ros.org/openni_launch/Tutorials/IntrinsicCalibration but encoutnered black-outed image on the GUI window produced by
rosrun camera_calibration cameracalibrator.py.After this PR, by setting the range policy to either of "dynamic" or "legacy", the calibration node start to work fine with kinect V1.
Implementing dynamic range determination only might be the cleanest approach, but maintaining previous behaviors for backward compatibility is also reasonable. Also, just I dont have enough resource to check the behavior to all the cameras is the part of the reason to leave the previous implementation.
Behavior check with kinect v1 IR image.
with

upper(identical to current implementation and what we got stuck with)with
legacy:with
dynamic:behavior check with kinect v1 RGB image
Although RGB image is not the direct target of this PR, but I tested by running
and confirmed that calibration procedure performed successfully, without any syntax error stuff.
mypy check
Running mypy version 1.7.1 (not for type check but for syntax check) and confirmed that no related error. (only found import-untyped and no-redef)