Add HAL implementation hooks to cv::flip() and cv::rotate() functions from core module by jvuillaumier · Pull Request #24233 · opencv/opencv

jvuillaumier · 2023-09-06T12:59:44Z

Hello,

This change proposes the addition of HAL hooks for cv::flip() and cv::rotate() functions from OpenCV core module.
Flip and rotation are functions commonly available from 2D hardware accelerators. This is convenient provision to enable custom optimized implementation of image flip/rotation on systems embedding such accelerator.

Thank you

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

I agree to contribute to the project under Apache 2 License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
The PR is proposed to the proper branch
There is a reference to the original bug report and related work
There is accuracy test, performance test and test data in opencv_extra repository, if applicable
Patch to opencv_extra has the same branch name.
The feature is well documented and sample code can be built with the project CMake

asmorkalov · 2023-09-06T13:08:26Z

@vpisarev @opencv-alalek could you take a look?

jvuillaumier · 2023-09-06T16:53:47Z

Change seems to break videoio/VideoCaptureAPITests.mp4_orientation_meta_auto test.
My bad - I am looking at it and will update.

asmorkalov · 2023-09-07T06:49:29Z

videoio/VideoCaptureAPITests.mp4_orientation_meta_auto uses cv::rotate internally.

jvuillaumier · 2023-09-07T07:35:50Z

cv::rotate is used internally indeed, when a rotation meta is present in the stream:
https://github.com/opencv/opencv/blob/4.x/modules/videoio/src/cap_interface.hpp#L269

In place rotation is used, that may be a case where this change causes problem.
I will check what the actual issue is and update.

jvuillaumier · 2023-09-11T08:58:50Z

Hello,

Change has been updated to take care of the in-place cv::rotate() case where same Mat is used for source and destination.

Test videoio/VideoCaptureAPITests.mp4_orientation_meta_auto is fixed.

Thanks

jvuillaumier · 2023-09-11T19:26:10Z

Previous failure on videoio/VideoCaptureAPITests.mp4_orientation_meta_auto is confirmed to be fixed (in place case).

However CI suggests an abort was detected running stitchExposureCompMultiFeed_a123.a123/12 test on macOS-x64 - this test is passed on other platforms.

I will look at it and update.

jvuillaumier · 2023-09-12T13:18:50Z

Hello,

I had a closer look to the abort issue reported by CI on stitchExposureCompMultiFeed_a123.a123/12 performance test on macOS-x64.

It is a bit puzzling me because:

Neither this test nor any other test from opencv_perf_stitching test suite is calling directly or indirectly a function changed by this patch (flip/rotate)
Test is passed on other platforms from CI
I could not repro the error locally neither on Yocto-ARM64 nor on Ubuntu-x64
Looking at recent PRs, I have not spotted such error so this test is presumably deemed stable

As I do not have access to a macOS-x64 platform, is it possible to restart CI on same change to confirm the issue and rule out a CI glitch?

Thanks

asmorkalov

👍

opencv-alalek · 2023-09-21T22:25:22Z

modules/core/include/opencv2/core/hal/hal.hpp

 CV_EXPORTS void addRNGBias64f( double* arr, const double* scaleBiasPairs, int len );

+CV_EXPORTS void flip( int src_type, const uchar* src_data, size_t src_step, int src_width, int src_height, uchar* dst_data, size_t dst_step, int flip_mode );
+CV_EXPORTS void rotate( int src_type, const uchar* src_data, size_t src_step, int src_width, int src_height, uchar* dst_data, size_t dst_step, int angle );


All functions from this file are defined somewhere in OpenCV codebase.
Added functions are not implemented. So they should not be here.

Lines removed in updated version of the patch
Thanks

opencv-alalek · 2023-09-21T22:29:37Z

modules/core/src/matrix_transform.cpp


+void rotate(InputArray _src, OutputArray _dst, int rotateMode)
+{
+    int angle;


Missing checks of input parameters constraints.

Check present in initial version of cv::rotate() is now present in the updated patch.
Thanks

opencv-alalek · 2023-09-21T22:33:06Z

modules/core/src/hal_replacement.hpp

+   @param dst_step destination image step
+   @param angle clockwise angle for rotation in degrees from set [90, 180, 270]
+ */
+inline int hal_ni_rotate(int src_type, const uchar* src_data, size_t src_step, int src_width, int src_height,


rotate => rotate90

To avoid confusion with generic rotation.

Functions renamed accordingly in updated version of the patch:
hal_ni_rotate => hal_ni_rotate90
cv_hal_rotate => cv_hal_rotate90

Thanks

@brief

This change is adding a HAL hook for custom cv::flip() implementation. Prototype of the HAL override function is: /** @brief hal_flip @param src_type source and destination image type @param src_data source image data @param src_step source image step @param src_width source and destination image width @param src_height source and destination image height @param dst_data destination image data @param dst_step destination image step @param flip_mode 0 flips around x-axis, positive around y-axis, negative both void flip(int src_type, const uchar* src_data, size_t src_step, int src_width, int src_height, uchar* dst_data, size_t dst_step, int flip_mode);

@brief

This change is adding a HAL hook for custom cv::rotate() implementation. Rotate operation is currently implemented as a composition of cv::transpose() and cv::flip(), which makes some operations a 2-steps process. 2D accelerators are usually capable of applying rotation in a single step. Therefore, HAL function is hooked in the cv::rotate() function rather than in the cv::transpose() and cv::flip() ones. Prototype of the HAL override function is: /** @brief rotate90 @param src_type source and destination image type @param src_data source image data @param src_step source image step @param src_width source image width If angle has value [180] it is also destination image width If angle has values [90, 270] it is also destination image height @param src_height source and destination image height (destination image width for angles [90, 270]) If angle has value [180] it is also destination image height If angle has values [90, 270] it is also destination image width @param dst_data destination image data @param dst_step destination image step @param angle clockwise angle for rotation in degrees from set [90, 180, 270] */ void rotate90( int src_type, const uchar* src_data, size_t src_step, int src_width, int src_height, uchar* dst_data, size_t dst_step, int angle);

jvuillaumier · 2023-09-22T15:36:11Z

CI reports test failures in DNN module for multiple platforms.
Updates in last patch version consist in cleanup, function/macro renaming and additional input parameter check in cv::rotate() - mostly non functional changes.

As CI status for the previous version of the patch was green, I presume present CI test failures are not related to this patch version.

jvuillaumier · 2023-09-25T08:29:28Z

CI errors for the DNN Einsum tests are likely related to #24311

Add HAL implementation hooks to cv::flip() and cv::rotate() functions from core module opencv#24233 Hello, This change proposes the addition of HAL hooks for cv::flip() and cv::rotate() functions from OpenCV core module. Flip and rotation are functions commonly available from 2D hardware accelerators. This is convenient provision to enable custom optimized implementation of image flip/rotation on systems embedding such accelerator. Thank you ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [ ] There is a reference to the original bug report and related work - [ ] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [ ] The feature is well documented and sample code can be built with the project CMake

asmorkalov requested review from opencv-alalek and vpisarev September 6, 2023 13:07

asmorkalov added category: core optimization labels Sep 6, 2023

jvuillaumier force-pushed the rotate_flip_hal_hooks branch from b701b1a to adc2eb4 Compare September 11, 2023 08:54

jvuillaumier changed the title ~~Add HAL implementation hooks cv::flip() and cv::rotate() functions from core module~~ Add HAL implementation hooks to cv::flip() and cv::rotate() functions from core module Sep 14, 2023

asmorkalov approved these changes Sep 21, 2023

View reviewed changes

asmorkalov added this to the 4.9.0 milestone Sep 21, 2023

opencv-alalek reviewed Sep 21, 2023

View reviewed changes

vpisarev approved these changes Sep 22, 2023

View reviewed changes

jvuillaumier added 2 commits September 22, 2023 09:31

jvuillaumier force-pushed the rotate_flip_hal_hooks branch from adc2eb4 to 06bca43 Compare September 22, 2023 09:15

opencv-alalek approved these changes Oct 6, 2023

View reviewed changes

asmorkalov merged commit 24fd395 into opencv:4.x Oct 6, 2023

asmorkalov mentioned this pull request Oct 17, 2023

(5.x) Merge 4.x #24416

Merged

Uh oh!

Conversation

jvuillaumier commented Sep 6, 2023

Pull Request Readiness Checklist

Uh oh!

asmorkalov commented Sep 6, 2023

Uh oh!

jvuillaumier commented Sep 6, 2023

Uh oh!

asmorkalov commented Sep 7, 2023

Uh oh!

jvuillaumier commented Sep 7, 2023

Uh oh!

jvuillaumier commented Sep 11, 2023

Uh oh!

jvuillaumier commented Sep 11, 2023

Uh oh!

jvuillaumier commented Sep 12, 2023

Uh oh!

asmorkalov left a comment

Choose a reason for hiding this comment

Uh oh!

opencv-alalek Sep 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jvuillaumier Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

opencv-alalek Sep 21, 2023

Choose a reason for hiding this comment

Uh oh!

jvuillaumier Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

opencv-alalek Sep 21, 2023

Choose a reason for hiding this comment

Uh oh!

jvuillaumier Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

jvuillaumier commented Sep 22, 2023

Uh oh!

jvuillaumier commented Sep 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

opencv-alalek Sep 21, 2023 •

edited

Loading