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

support color and depth/ir formats #2840

Conversation

SamerKhshiboun
Copy link
Collaborator

@SamerKhshiboun SamerKhshiboun commented Aug 10, 2023

Instead of supporting ROS Images of RGB, Y8 and Z16, now we support all ROS Images with these encodings: (depends on stream/profile chosen format):
RGB8, BGR8, RGBA8, BGRA8, Z16, Y8, Y16, RAW8, RAW10, RAW16, YUYV (YUV442)

Some tests on D455:

Color Y16: ros2 launch realsense2_camera rs_launch.py rgb_camera.color_format:=Y16
image

Color BGRA8: ros2 launch realsense2_camera rs_launch.py rgb_camera.color_format:=BGRA8
image

Color YUYV: ros2 launch realsense2_camera rs_launch.py rgb_camera.color_format:=YUYV
image

@SamerKhshiboun SamerKhshiboun force-pushed the support_opencv_and_ros_formats branch 3 times, most recently from af89554 to 5455b9d Compare August 12, 2023 21:02
@SamerKhshiboun SamerKhshiboun marked this pull request as ready for review August 12, 2023 21:02
@SamerKhshiboun SamerKhshiboun requested review from Nir-Az and removed request for Nir-Az August 12, 2023 21:02
@SamerKhshiboun SamerKhshiboun marked this pull request as draft August 12, 2023 21:36
@SamerKhshiboun SamerKhshiboun force-pushed the support_opencv_and_ros_formats branch 3 times, most recently from 583dc19 to c0be4b9 Compare August 14, 2023 06:04
@SamerKhshiboun SamerKhshiboun mentioned this pull request Aug 14, 2023
@SamerKhshiboun SamerKhshiboun marked this pull request as ready for review August 14, 2023 07:30
std::map<stream_index_pair, rclcpp::Publisher<sensor_msgs::msg::CameraInfo>::SharedPtr> _info_publishers;
std::map<stream_index_pair, rclcpp::Publisher<realsense2_camera_msgs::msg::Metadata>::SharedPtr> _metadata_publishers;
std::map<stream_index_pair, rclcpp::Publisher<IMUInfo>::SharedPtr> _imu_info_publishers;
std::map<stream_index_pair, rclcpp::Publisher<Extrinsics>::SharedPtr> _extrinsics_publishers;
rclcpp::Publisher<realsense2_camera_msgs::msg::RGBD>::SharedPtr _rgbd_publisher;
std::map<stream_index_pair, cv::Mat> _images;
std::map<unsigned int, std::string> _encoding;
std::map<rs2_format, std::string> rs_format_to_ros_format;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use _ for members

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

rs_format_to_cv_format[RS2_FORMAT_BGR8] = CV_8UC3;
rs_format_to_cv_format[RS2_FORMAT_RGBA8] = CV_8UC4;
rs_format_to_cv_format[RS2_FORMAT_BGRA8] = CV_8UC4;
rs_format_to_cv_format[RS2_FORMAT_YUYV] = CV_8UC2;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about UYVY?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also need to add format of Y10BPack, will test this tomorrow with D415.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After investigating, Y10BPack is not supported anymore.

// Convert the CV::Mat into a ROS image message (1 copy is done here)
cv_bridge::CvImage(std_msgs::msg::Header(), _encoding.at(bpp), cv_matrix_image).toImageMsg(*img_msg_ptr);
cv_bridge::CvImage(std_msgs::msg::Header(), rs_format_to_ros_format[stream_format], cv_matrix_image).toImageMsg(*img_msg_ptr);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For unsupported format this will create a new entry, probably initialized with RS2_FORMAT_ANY. Is this OK?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsupported formats will be stopped several steps before, and will switch the stream format to the default one (for example, for color, it will be RGB as default)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that you check above, but as I understand ROS_ERROR_STREAM does not throw (otherwise this won't work) and you don't return.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed functions to return status, and checked status after
(did this for two functions: when filling CV matrix image, and when filling ROS image msg)

// we try to reduce image creation as much we can, so we check if the same image structure
// was already created before, and we fill this image next with the frame data
// image.create() should be called once per <stream>_<profile>_<format>
if (image.size() != cv::Size(width, height) || CV_MAKETYPE(image.depth(), image.channels()) != rs_format_to_cv_format[stream_format])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed like above

if (f.is<rs2::video_frame>())
{
auto timage = f.as<rs2::video_frame>();
width = timage.get_width();
height = timage.get_height();
bpp = timage.get_bytes_per_pixel();
stream_format = timage.get_profile().format();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why will this produce different results than line 933?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I tested this with some flow, will verify this again tomorrow.

Copy link
Collaborator Author

@SamerKhshiboun SamerKhshiboun Aug 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, and printed an error if check fail.

@SamerKhshiboun SamerKhshiboun force-pushed the support_opencv_and_ros_formats branch 2 times, most recently from 9586de8 to 83ed9eb Compare August 15, 2023 07:56
Copy link

@OhadMeir OhadMeir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SamerKhshiboun SamerKhshiboun merged commit 40ac69f into IntelRealSense:ros2-development Aug 16, 2023
6 checks passed
@SamerKhshiboun SamerKhshiboun deleted the support_opencv_and_ros_formats branch October 10, 2023 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants