-
-
Notifications
You must be signed in to change notification settings - Fork 55.7k
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
Add HAL implementation hooks to cv::flip() and cv::rotate() functions from core module #24233
Conversation
@vpisarev @opencv-alalek could you take a look? |
Change seems to break videoio/VideoCaptureAPITests.mp4_orientation_meta_auto test. |
videoio/VideoCaptureAPITests.mp4_orientation_meta_auto uses |
In place rotation is used, that may be a case where this change causes problem. |
b701b1a
to
adc2eb4
Compare
Hello, Change has been updated to take care of the in-place Test Thanks |
Previous failure on However CI suggests an abort was detected running I will look at it and update. |
Hello, I had a closer look to the abort issue reported by CI on It is a bit puzzling me because:
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -201,6 +201,9 @@ CV_EXPORTS void cvt32f16f( const float* src, float16_t* dst, int len ); | |||
CV_EXPORTS void addRNGBias32f( float* arr, const float* scaleBiasPairs, int len ); | |||
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 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All functions from this file are defined somewhere in OpenCV codebase.
Added functions are not implemented. So they should not be here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines removed in updated version of the patch
Thanks
@@ -1097,4 +1101,50 @@ void rotate(InputArray _src, OutputArray _dst, int rotateMode) | |||
} | |||
} | |||
|
|||
void rotate(InputArray _src, OutputArray _dst, int rotateMode) | |||
{ | |||
int angle; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing checks of input parameters constraints.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check present in initial version of cv::rotate() is now present in the updated patch.
Thanks
modules/core/src/hal_replacement.hpp
Outdated
@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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rotate
=> rotate90
To avoid confusion with generic rotation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functions renamed accordingly in updated version of the patch:
hal_ni_rotate
=> hal_ni_rotate90
cv_hal_rotate
=> cv_hal_rotate90
Thanks
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);
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);
adc2eb4
to
06bca43
Compare
CI reports test failures in DNN module for multiple platforms. 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. |
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
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
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
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
Patch to opencv_extra has the same branch name.