Xilinx / Vitis_Libraries

Vitis Libraries
https://docs.xilinx.com/r/en-US/Vitis_Libraries
Apache License 2.0
853 stars 344 forks source link

remap: breaks OpenCV API (not documented) and doesn't work with common warp maps #100

Closed vmayoral closed 2 years ago

vmayoral commented 2 years ago

I spent the last week trying to make things to work with the Vitis Vision Library but failed. My guess is that the problem is actually two-fold: a) some API changes/break introduced which aren't stated clearly and b) a few (hopefully only due to my lack of familiarity with the Library) undocumented features.

My goal is to compose a pipeline so I started using a simple remap kernel to put together a rectification process Node (that does rectify images using camera calibration). The source code itself is of PL and PS:

Pretty straightforward and derived from the remap examples. The expectation was that given a raw image and its corresponding camera calibration parameters, after invoking in the PS cv::initUndistortRectifyMap to get the warp maps, the results produced by cv::remap and xf::cv::remap should be identical, but they are not. They image from xf::cv::remap comes fully black. Debugging further, the cause seemed related to the warp maps

Issue 1: Figuring out APIs changes ⚠️

I started by comparing both primitives, the one in OpenCV and the one within Vitis Vision Library:

Name Description OpenCV Xilinx Vitis Vision Library
cv::remap :heavy_check_mark:
raw Source image :heavy_check_mark:
rectified Destination image. It has the same size as map1 and the same type as src :heavy_check_mark:
cache_->reduced_map1 The first map of either (x,y) points or just x values having the type CV_16SC2 , CV_32FC1, or CV_32FC2 :heavy_check_mark:
cache_->reduced_map2 The second map of y values having the type CV_16UC1, CV_32FC1, or none (empty map if map1 is (x,y) points), respectively :heavy_check_mark:
interpolation Interpolation method :heavy_check_mark:
(optional) cv::BORDER_CONSTANT Pixel extrapolation method (default is BORDER_CONSTANT) :heavy_check_mark:
(optional) std::numeric_limits<float>::quiet_NaN() Value used in case of a constant border. By default, it is 0. :heavy_check_mark:
xf::cv::remap :heavy_check_mark:
mat_L Input xF Mat :heavy_check_mark:
leftRemappedMat Output xF Mat :heavy_check_mark:
mapxLMat mapX Mat of float type :heavy_check_mark:
mapyLMat mapY Mat of float type :heavy_check_mark:

After reading further, I got that the maps need to be provided as CV_32FC1 (this should be VERY highlighted, with appropriate examples in the documentation on how to transform various types of images, with their corresponding intrinsics/extrinsics into the right maps, otherwise anyone's forced to perform this same research themselves).

The maps that come out of cv::initUndistortRectifyMap are of types CV_16SC2 and CV_16UC1, so time to convert them:

  if (cache_->reduced_map1.type() == CV_32FC2 || cache_->reduced_map1.type() == CV_16SC2) {
    cv::convertMaps(cache_->reduced_map1, cache_->reduced_map2, map_x, map_y, CV_32FC1);
  } else if(cache_->reduced_map1.channels() == 1 && cache_->reduced_map2.channels() == 1) {
    map_x = cache_->reduced_map1;
    map_y = cache_->reduced_map2;
  } else {
    throw Exception("Unexpected amount of channels in warp maps: " + cache_->reduced_map1.channels());
  }

Issue 2: Maps still don't work for xf::cv::remap

After bypassing the first issue, expectation was for things to work but here's the result:

cv::remap xf::cv::remap
Screenshot from 2021-12-15 09-35-18 Screenshot from 2021-12-14 14-20-11

The issue itself has been isolated to the maps themselves. While maps work in cv::remap, why won't in xf::cv::remap (even if converted to CV_32FC1, as described above)[^1].

Questions and suggestions:

[^1]: Refer to the raw image and its corresponding camera calibration parameters to determine the maps. [^2]: I'm well aware that xf::cv::InitUndistortRectifyMapInverse is exemplified in the stereopipeline example but if this is mandatory for monocular perception, it should be VERY highlighted that this is the only possible case and a monocular perception example with both xf::cv::InitUndistortRectifyMapInverse and xf::cv::remap should be provided that guide how to move from OpenCV to Vitis Vision Library.

vmayoral commented 2 years ago

ping @vt-lib-support, @giuliocorradi.

vmayoral commented 2 years ago

Hey folks, just bumping this ticket again.

It seems that 12 days ago there was some movement in the repo. I had a look at the code changes but couldn't locate anything useful for this ticket. Can someone comment please? @vt-lib-support?

Thanks!

vmayoral commented 2 years ago

To facilitate debugging I've generated the warp maps (produced out of cv::initUndistortRectifyMap) with the following code:

  cv::FileStorage file("/tmp/map1.txt", cv::FileStorage::WRITE);
  file << "map1" << cache_->reduced_map1;

  cv::FileStorage file2("/tmp/map2.txt", cv::FileStorage::WRITE);
  file2 << "map2" << cache_->reduced_map2;
  std::cout << "saved maps" << std::endl;

To user this maps, one would need to load them and then convert them as appropriate for digestion by the acceleration kernel (see https://github.com/ros-acceleration/image_pipeline/blob/ros2/image_proc/src/rectify_fpga.cpp#L206-L213).

akashsun commented 2 years ago

Hi Victor, the issue is caused because the XF_WIN_ROWS param set to lesser than what is required. In this application, it is required to set this parameter to '50'. (This is a compile time parameter for hardware code, it indicated the number of line buffers stored for the remap to fetch from). For example, if the param set to 5, it stores only 5 rows of data, 2 rows above, 2 rows below and the current row of input. If the map-y value at (0,2) points to, case i: 4 (4th row in input), the kernel can fetch the data since it is 2 rows below case ii: 5 (5th row) the kernel returns invalid (invalid set to 0), since the data is out of bounds or not avail in the line buffer

This param is set while building the hardware kernel. And this will be constant for a particular cam setup, since the maps are generated only once. So once the maps are generated this param could be estimated as, max of map_y ( |val(x,y)-y| )

vmayoral commented 2 years ago

Thanks for the response @akashsun.

It'll take a few days because I'm in the middle of something, but let me evaluate this and get back to you. While I do so, feel free to add additional bits if you have any. It'd particularly helpful to understand a bit more about XF_WIN_ROWS and where this is documented (50 sounds like a magic number to me).

vmayoral commented 2 years ago

Thanks @akashsun, I can confirm you previous comment did it.

I'd be great to get those documentation details added and the following further clarified:

This param is set while building the hardware kernel. And this will be constant for a particular cam setup, since the maps are generated only once. So once the maps are generated this param could be estimated as, max of map_y ( |val(x,y)-y| )

caosjr commented 5 months ago

Hello @akashsun @vmayoral @vt-lib-support and @giuliocorradi

Could you further explain how you agreed on the 50 value for the XF_WIN_ROWS? I did not understand the max of map_y (|val(x,y)-y|) calculation.

I tried to iterate every pixel from mapy and subtract the column (fabs(mapy[i][j] - j)) and store the highest value, but that did not come out as expected. It is also important to point out that my mapy contains negative values, should they be considered?

akashsun commented 5 months ago

mapy contains the 'target pixel to be picked from the input image along y-axis'. It must not be negative. Mostly you can ignore if it is negative (mostly those will be in borders). secondly, you must nor subract the column, it must be along rows 'i'. (fabs(mapy[i][j] - i)).

Also, '50' is for that particular scenario based on the maps provided by VMayoral.

caosjr commented 5 months ago

Thank you very much @akashsun, It works for me. I ignored the negative values from mapy.

The final result is half of the XF_WIN_ROWS, but I believe this is related to what you have explained earlier. So, the final result should the returned value multiplied by 2, right?

In my case, it returns 44.03, which means the final result is 89 (rounded to the next integer).

akashsun commented 5 months ago

thats right!