Pitfalls of OdometryFrame's dependence on UMat

[JoeHowse]

System Information

OpenCV Python version: 5.0.0-pre (commit d976272)

Detailed description

Internally, OdometryFrame relies on UMat in all cases (regardless of whether the user originally provided frame data in UMat format or not). The use of UMat here was introduced in #22598, replacing an earlier templated solution.

Generally, OpenCV functions do not (and should not) have the side effect of converting Mat parameters to UMat. On many systems, Mat operations perform better than UMat operations. Moreover, Mat operations have fewer compatibility pitfalls (as they never attempt to use OpenCL features). For example, it is difficult to find any case on ARM Linux where UMat is a compatible and performant choice.

[Edit]
Here is a specific example of a compatibility problem that I encountered. On one of my Linux systems, when I run cv::OdometryFrame::compute, the program outputs the following OpenCL compilation error and then the whole system locks up:

OpenCL program build log: core/minmaxloc
Status -11: CL_BUILD_PROGRAM_FAILURE
-D DEPTH_0 -D srcT1=uchar -D WGS=256 -D srcT=uchar4 -D WGS2_ALIGNED=128 -D DOUBLE_SUPPORT -D HAEV_SRC_CONT -D HAVE_MASK_CONT -D kercn=4 -D NEED_MAXVAL -D dstT1=uchar -D dstT=uchar4 -D convertToDT=noconvert -D wdepth=0 -D convertFromU=noconvert -D MINMAX_STRUCT_ALIGNMENT=8 -D AMD_DEVICE input.cl:8:1: error: OpenCL C version 1.1 does not support the 'static' storage class specifier

This is a case where the system in general has poor OpenCL support. Nonetheless, most OpenCV functions work (with Mat input) and this particular function does not.
[/Edit]

I would suggest revising the OdometryFrame implementation to avoid Mat-->UMat conversion. Alternatively, I would suggest documenting this class's dependence on UMat because users may need to consider the performance/compatibility pitfalls.

Steps to reproduce

The issue (internal use of UMat in all cases) is apparent from code review, as well as testing.

Issue submission checklist

• I report the issue, it's not a question
• I checked the problem with documentation, FAQ, open issues, forum.opencv.org, Stack Overflow, etc and have not found any solution
• I updated to the latest OpenCV version and the issue is still there
• There is reproducer code and related data files (videos, images, onnx, etc)

[savuor]

@JoeHowse Thanks for your considerations!

1. What are the compatibility UMat issues that are specific for Odometry? If they are, they should be properly described in the issues here, on Github.
2. I agree that Mat <-> UMat conversions are unwanted but that solution is much easier to maintain than previous TMat-based approach.
3. As for performance, in some cases the odometry calculation should be done on CPU (small image sizes, robust sigma estimation for residuals), in other cases it should be done on GPU. Probably the decision what device to use should be done somewhere deep inside the Odometry class.

Since we want to support both Mats and UMats as input data, maybe a compromise solution would be to keep a pair of Mat and UMat instead of just UMats? They would store the same data, a mechanism that was intended by getMat()/getUMat() methods but is still problematic.

[JoeHowse]

1. What are the compatibility UMat issues that are specific for Odometry? If they are, they should be properly described in the issues here, on Github.

I have added a specific case in the issue description, above.

3. As for performance, in some cases the odometry calculation should be done on CPU (small image sizes, robust sigma estimation for residuals), in other cases it should be done on GPU. Probably the decision what device to use should be done somewhere deep inside the Odometry class.

This sounds reasonable, yes.

Since we want to support both Mats and UMats as input data, maybe a compromise solution would be to keep a pair of Mat and UMat instead of just UMats? They would store the same data, a mechanism that was intended by getMat()/getUMat() methods but is still problematic.

Provided that one of these may be unused (i.e. empty), this sounds reasonable.

[savuor]

The issue you've described is related to the minMaxLoc function which (as a workaround, but anyway) is used by Odometry.
This bug should have its own issue here on Github. If you don't create an issue, I'll do that.

As for Mat-UMat pair, I'll discuss this solution with other OpenCV team if they know some better solution.
If not, we'll implement it someday - or if you have resources, fell free to propose your own implementation, I'll review it.

[JoeHowse]

For the minMaxLoc compatibility issue, I have opened #22860.