[WIP] Stereo Image Proc Disparity Image acceleration #1

dirksavage88 · 2022-03-10T20:14:07Z

Integrate ROS acceleration stereo block match implementation (Vitis Vision library). Goal is baseline implementation of disparity image generation through the Vitis vision stereo block matching function. Next steps are to implement pointcloud and then a stereo image pipeline.

Currently testing in ROS 2 Rolling ubuntu 20.04 focal.

Roadblocks/TODO:

Notes: disparity_fpga utilizes the parameter callbacks found in the original disparity_node.cpp, while I don’t know if this is necessary (since most parameters set at run time should suffice), I left this code untouched.

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

…parity image out fix Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

…ores cv mat output from CV Bridge Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

dirksavage88 · 2022-03-29T13:05:39Z

@vmayoral i’m able to colcon build stereo image proc but don’t see the kernel accelerator in the build-kv260/stereo_image_proc folder. I do get a warning that the .xclbin file was not found (but I also get this for the acceleration examples too). Am I missing something in the cmake file ?

vmayoral · 2022-03-29T13:12:16Z

Hey @dirksavage88, seems you're making good progress 👍!

@vmayoral i’m able to colcon build stereo image proc but don’t see the kernel accelerator in the build-kv260/stereo_image_proc folder. I do get a warning that the .xclbin file was not found (but I also get this for the acceleration examples too). Am I missing something in the cmake file ?

https://github.com/ros-acceleration/image_pipeline/pull/1/files#diff-4b934073461589fd72e15f5e91633e8f58f61af181aafcc776a2b66bfb0e2899R75-R85

Looks good to me from a CMakeLists.txt perspective, so my only thought is that you might be getting errors in the build process. Can you check the build logs in build-kv260/stereo_image_proc? You should have a bunch of logs in there from the Vitis build process, feel free to paste the most relevant in here if you need help navigating around them.

This week I'm overflowed with work, so I'll try and make a complete review of you contribution next week. Ping me if you don't hear from me next week.

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

dirksavage88 · 2022-04-01T02:05:59Z

Hey @dirksavage88, seems you're making good progress +1!

@vmayoral i’m able to colcon build stereo image proc but don’t see the kernel accelerator in the build-kv260/stereo_image_proc folder. I do get a warning that the .xclbin file was not found (but I also get this for the acceleration examples too). Am I missing something in the cmake file ?

https://github.com/ros-acceleration/image_pipeline/pull/1/files#diff-4b934073461589fd72e15f5e91633e8f58f61af181aafcc776a2b66bfb0e2899R75-R85

Looks good to me from a CMakeLists.txt perspective, so my only thought is that you might be getting errors in the build process. Can you check the build logs in build-kv260/stereo_image_proc? You should have a bunch of logs in there from the Vitis build process, feel free to paste the most relevant in here if you need help navigating around them.

This week I'm overflowed with work, so I'll try and make a complete review of you contribution next week. Ping me if you don't hear from me next week.

Looks like it had an issue with the platform in the config file, and in the other log it could not find the *.xo file. I've changed the platform to match what the log suggests.

v++_stereolbm_accel_27105.backup.log
v++_stereolbm_accel.log

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

vmayoral · 2022-04-04T11:27:29Z

@dirksavage88, from the log file:

ERROR: [v++ 60-602] Source file does not exist: /home/papadrew/vitis_ws/build-kv260/stereo_image_proc/stereolbm_accel.xo
INFO: [v++ 60-1662] Stopping dispatch session having empty uuid.
INFO: [v++ 60-1653] Closing dispatch client.

is generally an indicator that your kernel did not synthesize successfully. There's something wrong with your code and it's not compiling a Xilinx Object (.xo) file.

From your other log file (the backup):

...
ERROR: [v++ 60-1258] No valid platform was found that matches 'kv260_custom_platform'. Please make sure that the platform is specified correctly, and the platform has the right version number. The platform repo paths are:
        /home/papadrew/vitis_ws/acceleration/firmware/select/platform
        /home/papadrew/tools/Xilinx/Vitis/2020.2/platforms
The valid platforms found from the above repo paths are:
        /home/papadrew/vitis_ws/acceleration/firmware/select/platform/kv260_base.xpfm

ERROR: [v++ 60-587] Failed to add a platform: specified platform kv260_custom_platform is not found or is not valid
ERROR: [v++ 60-592] Failed to finish compilation
INFO: [v++ 60-1653] Closing dispatch client.

It appears to me like you're mixing things up. Your Vitis setup seems to be 2020.2 but you're using kv260_custom_platform Vitis platform, which is the one that I created for the Vitis 2021.2 release (with KRS beta). That's still an pre-release and unstable but the platform is known to work and has been validated by others so I'd recommend to maybe update your toolset (Vitis, to 2021.2) and re-start building the workspace.

dirksavage88 · 2022-04-11T13:00:37Z

@vmayoral I’ll take a look at it this weekend. Should I be compiling using sw or hw_emu to save time while I work out any issues with the code? Also will I need to move to ROS rolling for Vitis 2021.2?

vmayoral · 2022-04-11T13:14:51Z

Should I be compiling using sw or hw_emu to save time while I work out any issues with the code?

Using those build targets certainly helps iterating, but make sure you built against hw at the end and validate it in hardware.

Also will I need to move to ROS rolling for Vitis 2021.2?

I would recommend you to do so, yes. Most of the other image_pipeline examples are based on Vitis 2021.2.

dirksavage88 · 2022-04-25T15:09:43Z

EDIT: I installed Rolling base from the Debian packages and it worked. I see my issue here: https://github.com/dirksavage88/image_pipeline/blob/f95cae7aabe1bfdcd711dda49935a30196c9f4f8/stereo_image_proc/src/stereo_image_proc/disparity_fpga.cpp#L376

Instead of trying to set the sensor msg image data member to the CV matrix (no header or encoding) I need to use the function overload to convert the entire CVImage to a disparity message and then that gets passed to publish.

…x to image message conversion (cvimage -> ros image msg) Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

…n cmakelist Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

…esis completes without error Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

dirksavage88 · 2022-05-06T00:33:10Z

@vmayoral I believe the kernel passes synthesis:
v++_stereolbm_accel.log

I changed the kv260_custom_platform under vitis 2021.2.

Looking at the vitis analyzer results.

…clean up comments Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

soallak · 2022-05-16T00:28:48Z

Hey,
maybe you could checkout the second PR I opened where I have integrated using Vitis Vision StereoBM. I have already tested on kv260. Speed up is about 5.5x to 8x

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

dirksavage88 · 2022-06-20T15:07:06Z

EDIT: Kernel passes synthesis and linked successfully, I now generated an *.xclbin file. Next is test + benchmark. I have a Jetson nano coming soon so hopefully that is a fair comparison @vmayoral

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

dirksavage88 · 2022-06-30T03:32:17Z

soallak · 2022-07-01T00:14:47Z

stereo_image_proc/CMakeLists.txt

+  target_link_libraries(disparity_fpga ${OpenCL_LIBRARY})
+  target_compile_definitions(disparity_fpga PRIVATE "COMPOSITION_BUILDING_DLL")
+
+  rclcpp_components_register_nodes(disparity_fpga "stereo_image_proc::DisparityNodeFPGA")


Why is this macro call differing from the one above ?
https://github.com/ros2/rclcpp/blob/master/rclcpp_components/cmake/rclcpp_components_register_node.cmake

I see my mistake. And this caused me issues when building, I ended up commenting out parameter callbacks.

Good catch!

soallak · 2022-07-01T00:16:47Z

stereo_image_proc/cfg/Disparity.cfg

@@ -0,0 +1,37 @@
+#! /usr/bin/env python


Do we still need dynamic reconfigure in ros2 ?
The new parameter api support runtime changes to parameter through callbacks afaik

I believe I copied this over from the image_proc acceleration package, I can delete this if it is not needed

soallak · 2022-07-05T08:02:04Z

stereo_image_proc/CMakeLists.txt

+  )
+  # disparity_fpga component (with OpenCL)
+  ament_auto_add_library(disparity_fpga SHARED src/stereo_image_proc/disparity_fpga.cpp)
+  ament_target_dependencies(${PROJECT_NAME} ${lib_depends})


This is not needed. you probably wanted to set dependencies for disparity_fpga ?!

soallak · 2022-07-05T12:19:47Z

stereo_image_proc/src/stereo_image_proc/disparity_fpga.cpp

+  for (auto &param : params)
+  {
+    std::string param_name = param.get_name().c_str();
+    std::string param_val = param.value_to_string().c_str();


this params are doubles !

soallak · 2022-07-05T12:24:52Z

stereo_image_proc/src/stereo_image_proc/disparity_fpga.cpp

+  OCL_CHECK(err, err = krnl_->setArg(4, disp_info_msg->width));
+  OCL_CHECK(err, err = krnl_->setArg(5, disp_info_msg->height));
+
+  OCL_CHECK(err, queue_->enqueueWriteBuffer(imageRToDevice, CL_TRUE, 0, image_r_in_size_bytes, cv_ptr_r->image.data));


I think enqueuing left and right buffers can be done in parallel, so no need to block the first
Simply make sure to flush the queue before enqueue the stereo matching kernel task

soallak · 2022-07-05T12:27:57Z

stereo_image_proc/src/stereo_image_proc/disparity_fpga.cpp

+  cv_bridge::CvImagePtr cv_ptr_l;
+  cv_bridge::CvImagePtr cv_ptr_r;
+  cv::Mat hls_disp16;
+  static const double inv_dpp = 1.0 / NO_OF_DISPARITIES;


I would use constexp double
anyways, vitis stereo_lbm uses 12.4 fixed point, you have to the divide by 16, no ?

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

dirksavage88 added 11 commits February 25, 2022 22:08

Initial stereo processing acceleration committ WIP

24491a2

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

Disparity acceleration kernel added, pass params to kernel args

9741e33

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

Added kernel execution

040bc18

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

Fix to Cmakelist, added stereolbm header

879f190

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

Fixed pathing to vitis common header files

dff42ca

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

CMakelists fixes to paths

28d72b7

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

Fixed header path issue, vitis common/improc header path changes, dis…

460b6e9

…parity image out fix Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

Header guards

03c9154

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

Various fixes, removed unused variable

91a8ee6

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

Fixed shared pointer operand type issue, now disp msg image member st…

eedcf4f

…ores cv mat output from CV Bridge Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

Convert output image from cv bridge to disparity

7c4f271

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

vmayoral self-assigned this Mar 29, 2022

vmayoral self-requested a review March 29, 2022 13:11

Fix compiler warning for disparity fpga

88e191b

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

Added kv260 config, TODO:troubleshoot invalid platform

f95cae7

Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

dirksavage88 added 3 commits April 27, 2022 19:10

Converted HLS CV16 output image to float per disparity image type, fi…

9871ab2

…x to image message conversion (cvimage -> ros image msg) Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Removed bundle control from pragmas, commented out find package XRT i…

c2f6e0c

…n cmakelist Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Removed CLOCK and LINK sections from vitis_acceleration_kernel, synth…

057f09f

…esis completes without error Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Add target export dependencies to CMake to satisfy pipeline linking, …

3091a1a

…clean up comments Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Added kernel link to stereo_image_proc

81e1ce9

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

dirksavage88 added 2 commits June 20, 2022 18:11

Fix to disparity_fpga, cfg files

637f6d6

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Fix to disparity_fpga model parameter in converTo function

4b30478

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

soallak reviewed Jul 1, 2022

View reviewed changes

soallak reviewed Jul 5, 2022

View reviewed changes

Applied patch

9ddbbac

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Stereo Image Proc Disparity Image acceleration #1

[WIP] Stereo Image Proc Disparity Image acceleration #1

dirksavage88 commented Mar 10, 2022 •

edited

Loading

dirksavage88 commented Mar 29, 2022

vmayoral commented Mar 29, 2022

dirksavage88 commented Apr 1, 2022 •

edited

Loading

vmayoral commented Apr 4, 2022

dirksavage88 commented Apr 11, 2022 •

edited

Loading

vmayoral commented Apr 11, 2022

dirksavage88 commented Apr 25, 2022 •

edited

Loading

dirksavage88 commented May 6, 2022 •

edited

Loading

soallak commented May 16, 2022

dirksavage88 commented Jun 20, 2022 •

edited

Loading

dirksavage88 commented Jun 30, 2022

soallak Jul 1, 2022

dirksavage88 Jul 1, 2022 •

edited

Loading

soallak Jul 1, 2022

dirksavage88 Jul 1, 2022

soallak Jul 5, 2022

soallak Jul 5, 2022

soallak Jul 5, 2022

soallak Jul 5, 2022

[WIP] Stereo Image Proc Disparity Image acceleration #1

Are you sure you want to change the base?

[WIP] Stereo Image Proc Disparity Image acceleration #1

Conversation

dirksavage88 commented Mar 10, 2022 • edited Loading

dirksavage88 commented Mar 29, 2022

vmayoral commented Mar 29, 2022

dirksavage88 commented Apr 1, 2022 • edited Loading

vmayoral commented Apr 4, 2022

dirksavage88 commented Apr 11, 2022 • edited Loading

vmayoral commented Apr 11, 2022

dirksavage88 commented Apr 25, 2022 • edited Loading

dirksavage88 commented May 6, 2022 • edited Loading

soallak commented May 16, 2022

dirksavage88 commented Jun 20, 2022 • edited Loading

dirksavage88 commented Jun 30, 2022

soallak Jul 1, 2022

Choose a reason for hiding this comment

dirksavage88 Jul 1, 2022 • edited Loading

Choose a reason for hiding this comment

soallak Jul 1, 2022

Choose a reason for hiding this comment

dirksavage88 Jul 1, 2022

Choose a reason for hiding this comment

soallak Jul 5, 2022

Choose a reason for hiding this comment

soallak Jul 5, 2022

Choose a reason for hiding this comment

soallak Jul 5, 2022

Choose a reason for hiding this comment

soallak Jul 5, 2022

Choose a reason for hiding this comment

dirksavage88 commented Mar 10, 2022 •

edited

Loading

dirksavage88 commented Apr 1, 2022 •

edited

Loading

dirksavage88 commented Apr 11, 2022 •

edited

Loading

dirksavage88 commented Apr 25, 2022 •

edited

Loading

dirksavage88 commented May 6, 2022 •

edited

Loading

dirksavage88 commented Jun 20, 2022 •

edited

Loading

dirksavage88 Jul 1, 2022 •

edited

Loading