Skip to content

pyrDown: offset HAL added, IPP removed#25970

Merged
asmorkalov merged 6 commits intoopencv:4.xfrom
savuor:rv/hal_pyrdown
Aug 6, 2024
Merged

pyrDown: offset HAL added, IPP removed#25970
asmorkalov merged 6 commits intoopencv:4.xfrom
savuor:rv/hal_pyrdown

Conversation

@savuor
Copy link
Copy Markdown
Contributor

@savuor savuor commented Jul 30, 2024

Resolves #25976

Changes

  • HAL added for offset support so that border pixels can be fetched from outside of the image ROI (see BORDER_ISOLATED parameter)
  • IPP removed since there is pyrUp instead of pyrDown and there's no easy way to fix this other than rewriting it from scratch
  • replaced old C call by modern cv::pyrDown

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

@savuor savuor changed the title pyrDown: offset HAL added, IPP temporarily disabled pyrDown: offset HAL added, IPP removed Aug 1, 2024
@asmorkalov asmorkalov self-assigned this Aug 2, 2024
@asmorkalov asmorkalov added this to the 4.11.0 milestone Aug 2, 2024
@opencv-alalek opencv-alalek requested a review from vpisarev August 6, 2024 08:42
@param margin_bottom Bottom margins for source image
@param border_type Border type
*/
inline int hal_ni_pyrdown_offset(const uchar* src_data, size_t src_step, int src_width, int src_height,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is our plan to enable such feature in all filters. Starting with the name - shall we use _roi in the name? And pass the whole image size and roi inside?

How to make it python-compatible? (Numpy does not have a notion of roi)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. It's already done in current HAL API for filters.
  2. Python and Numpy is not related here. Unfortunately we do not have solution for it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Numpy supports array slicing (though I am not sure if the slices can be passed outside of the Python)
  • Taking the ROI from source image then applying filter should produce the same result as doing it vice versa (i.e. applying filter then taking the ROI of the result).
    We can recommend Python users to stick to the second option at cost of performance.

@asmorkalov asmorkalov assigned vpisarev and unassigned asmorkalov Aug 6, 2024
@asmorkalov asmorkalov merged commit 1acf722 into opencv:4.x Aug 6, 2024
@savuor savuor deleted the rv/hal_pyrdown branch August 6, 2024 14:56
fengyuentau pushed a commit to fengyuentau/opencv that referenced this pull request Aug 15, 2024
pyrDown: offset HAL added, IPP removed opencv#25970

Resolves opencv#25976

### Changes
* HAL added for offset support so that border pixels can be fetched from outside of the image ROI (see `BORDER_ISOLATED` parameter)
* IPP removed since there is `pyrUp` instead of `pyrDown` and there's no easy way to fix this other than rewriting it from scratch
* replaced old C call by modern `cv::pyrDown`

### 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
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
@asmorkalov asmorkalov mentioned this pull request Aug 27, 2024
savuor added a commit to savuor/opencv that referenced this pull request Nov 1, 2024
pyrDown: offset HAL added, IPP removed opencv#25970

Resolves opencv#25976

### Changes
* HAL added for offset support so that border pixels can be fetched from outside of the image ROI (see `BORDER_ISOLATED` parameter)
* IPP removed since there is `pyrUp` instead of `pyrDown` and there's no easy way to fix this other than rewriting it from scratch
* replaced old C call by modern `cv::pyrDown`

### 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
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
@savuor savuor mentioned this pull request Nov 1, 2024
6 tasks
savuor added a commit to savuor/opencv that referenced this pull request Nov 5, 2024
pyrDown: offset HAL added, IPP removed opencv#25970

Resolves opencv#25976

### Changes
* HAL added for offset support so that border pixels can be fetched from outside of the image ROI (see `BORDER_ISOLATED` parameter)
* IPP removed since there is `pyrUp` instead of `pyrDown` and there's no easy way to fix this other than rewriting it from scratch
* replaced old C call by modern `cv::pyrDown`

### 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
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
savuor added a commit to savuor/opencv that referenced this pull request Nov 8, 2024
pyrDown: offset HAL added, IPP removed opencv#25970

Resolves opencv#25976

### Changes
* HAL added for offset support so that border pixels can be fetched from outside of the image ROI (see `BORDER_ISOLATED` parameter)
* IPP removed since there is `pyrUp` instead of `pyrDown` and there's no easy way to fix this other than rewriting it from scratch
* replaced old C call by modern `cv::pyrDown`

### 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
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
savuor added a commit to savuor/opencv that referenced this pull request Nov 21, 2024
pyrDown: offset HAL added, IPP removed opencv#25970

Resolves opencv#25976

### Changes
* HAL added for offset support so that border pixels can be fetched from outside of the image ROI (see `BORDER_ISOLATED` parameter)
* IPP removed since there is `pyrUp` instead of `pyrDown` and there's no easy way to fix this other than rewriting it from scratch
* replaced old C call by modern `cv::pyrDown`

### 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
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
thewoz pushed a commit to CobbsLab/OPENCV that referenced this pull request Feb 13, 2025
pyrDown: offset HAL added, IPP removed opencv#25970

Resolves opencv#25976

### Changes
* HAL added for offset support so that border pixels can be fetched from outside of the image ROI (see `BORDER_ISOLATED` parameter)
* IPP removed since there is `pyrUp` instead of `pyrDown` and there's no easy way to fix this other than rewriting it from scratch
* replaced old C call by modern `cv::pyrDown`

### 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
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] 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

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

IPP implementation for cv::pyrDown is incorrect

4 participants