Skip to content
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

Integrate ARM KleidiCV as OpenCV HAL #25443

Merged
merged 7 commits into from May 16, 2024
Merged

Conversation

asmorkalov
Copy link
Contributor

@asmorkalov asmorkalov commented Apr 18, 2024

The library source code with license: https://gitlab.arm.com/kleidi/kleidicv/

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

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
@asmorkalov
Copy link
Contributor Author

@opencv-alalek @mplatings I re-integrated KleidiCV with ocv_download. Current branch allows just use -DWITH_KLEIDICV=ON with default HAL version, substitute own URL or set KLEIDICV_SOURCE_PATH to local KleidiCV instance. Please take a look and provide feedback.

@asmorkalov asmorkalov added the platform: arm ARM boards related issues: RPi, NVIDIA TK/TX, etc label May 8, 2024
@asmorkalov asmorkalov changed the title WIP: Integrate ARM KleidiCV as OpenCV HAL Integrate ARM KleidiCV as OpenCV HAL May 8, 2024
@mplatings
Copy link

mplatings commented May 10, 2024

3rdparty/kleidicv/CMakeLists.txt Outdated Show resolved Hide resolved
3rdparty/kleidicv/CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
@asmorkalov
Copy link
Contributor Author

Fixed review remarks. Works on HAL priorities.

@asmorkalov
Copy link
Contributor Author

@opencv-alalek I introduced commit hash as parameter for KleidiCV download as you proposed.
@opencv-alalek @mplatings I returned back KleidiCV as first HAL. Current implementation (macros in KleidiCV) tries to apply KleidiCV first and then fall back to the next possible implementation. CMake does not control priorities, but CMake status message describes it better, if KleidiCV is the first one.

@opencv-alalek
Copy link
Contributor

CMake does not control priorities

That is not true. CMake scrips do that by defining order of used HAL headers.

The last header has the highest priority because it is used like this (example from carotene):

#undef cv_hal_add8u
#define cv_hal_add8u(src1, sz1, src2, sz2, dst, sz, w, h) TEGRA_BINARYOP(CAROTENE_NS::u8, add, src1, sz1, src2, sz2, dst, sz, w, h)

Any previous definitions from other HALs are undefined unconditionally.

@asmorkalov
Copy link
Contributor Author

Current CMake adds HAL implementation at the beginning of the list. With the CMake chain KleidiCV -> Carotene we have list "carotene;kleidicv". The last one is Kleidi and it defines macros with fallback to previous implementation, Carotene. So current version works as expected.

@asmorkalov asmorkalov merged commit d29ad2f into opencv:4.x May 16, 2024
27 of 28 checks passed
klatism pushed a commit to klatism/opencv that referenced this pull request May 17, 2024
Integrate ARM KleidiCV as OpenCV HAL opencv#25443

The library source code with license: https://gitlab.arm.com/kleidi/kleidicv/

### 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
- [x] 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 added a commit that referenced this pull request May 23, 2024
KleidiCV HAL update to version 0.1.0. #25618

Original integration PR: #25443

Force the library for testing with CI

### 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
- [x] 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants