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

[camera_calibration] fix/feat: enable to select range policy for 16 bit images #841

Open
wants to merge 1 commit into
base: noetic
Choose a base branch
from

Conversation

HiroIshida
Copy link

@HiroIshida HiroIshida commented Dec 6, 2023

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.
Screenshot from 2023-12-06 20-50-59
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.

rosrun camera_calibration cameracalibrator.py image:=/kinect_head/ir/image_raw camera:=/kinect_head/ir --size 5x4 --square 0.0245 -r upper 

with upper (identical to current implementation and what we got stuck with)
Screenshot from 2023-12-06 20-35-12

with legacy:

rosrun camera_calibration cameracalibrator.py image:=/kinect_head/ir/image_raw camera:=/kinect_head/ir --size 5x4 --square 0.0245 -r legacy

Screenshot from 2023-12-06 20-32-53

with dynamic:

rosrun camera_calibration cameracalibrator.py image:=/kinect_head/ir/image_raw camera:=/kinect_head/ir --size 5x4 --square 0.0245 -r dynamic

Screenshot from 2023-12-06 20-34-07

behavior check with kinect v1 RGB image

Although RGB image is not the direct target of this PR, but I tested by running

rosrun camera_calibration cameracalibrator.py --size 6x7 --square 0.108 camera:=/kinect_head/rgb image:=/kinect_head/rgb/image_raw

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)

h-ishida@azarashi:~/planning_ws/src/image_pipeline/camera_calibration$ mypy src/
src/camera_calibration/calibrator.py:37: error: Skipping analyzing "cv_bridge": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/calibrator.py:38: error: Skipping analyzing "image_geometry": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/calibrator.py:43: error: Skipping analyzing "sensor_msgs.msg": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/calibrator.py:43: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
src/camera_calibration/calibrator.py:43: error: Skipping analyzing "sensor_msgs": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/camera_checker.py:36: error: Skipping analyzing "cv_bridge": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/camera_checker.py:38: error: Skipping analyzing "message_filters": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/camera_checker.py:40: error: Skipping analyzing "rospy": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/camera_checker.py:41: error: Skipping analyzing "sensor_msgs.msg": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/camera_checker.py:41: error: Skipping analyzing "sensor_msgs": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/camera_checker.py:42: error: Skipping analyzing "sensor_msgs.srv": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/camera_checker.py:51: error: Cannot find implementation or library stub for module named "Queue"  [import-not-found]
src/camera_calibration/camera_checker.py:51: error: Name "Queue" already defined (possibly by an import)  [no-redef]
src/camera_calibration/camera_calibrator.py:36: error: Skipping analyzing "message_filters": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/camera_calibrator.py:39: error: Skipping analyzing "rospy": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/camera_calibrator.py:40: error: Skipping analyzing "sensor_msgs.msg": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/camera_calibrator.py:40: error: Skipping analyzing "sensor_msgs": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/camera_calibrator.py:41: error: Skipping analyzing "sensor_msgs.srv": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/camera_calibration/camera_calibrator.py:48: error: Cannot find implementation or library stub for module named "Queue"  [import-not-found]
src/camera_calibration/camera_calibrator.py:48: error: Name "Queue" already defined (possibly by an import)  [no-redef]
Found 19 errors in 3 files (checked 4 source files)
h-ishida@azarashi:~/planning_ws/src/image_pipeline/camera_calibration$ mypy --version
mypy 1.7.1 (compiled: yes)

@HiroIshida HiroIshida force-pushed the range_policy branch 5 times, most recently from 20d0709 to 8ef62b7 Compare December 6, 2023 12:15
@HiroIshida HiroIshida marked this pull request as ready for review December 6, 2023 23:33
@HiroIshida HiroIshida changed the title [camera_calibration] enable to select range policy for 16 bit images [camera_calibration] fix/feat: enable to select range policy for 16 bit images Dec 9, 2023
@HiroIshida
Copy link
Author

kindly ping @JWhitleyWork

@HiroIshida
Copy link
Author

kindly ping @JWhitleyWork @vrabaud

@HiroIshida
Copy link
Author

kindly ping @JWhitleyWork @vrabaud @mikeferguson

@mikeferguson
Copy link
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...

@HiroIshida
Copy link
Author

Understand.. thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants