Skip to content

Correct Bayer2Gray u8 SIMD#25968

Merged
asmorkalov merged 3 commits intoopencv:4.xfrom
FantasqueX:correct-bayer2gray-simd-1
Aug 21, 2024
Merged

Correct Bayer2Gray u8 SIMD#25968
asmorkalov merged 3 commits intoopencv:4.xfrom
FantasqueX:correct-bayer2gray-simd-1

Conversation

@FantasqueX
Copy link
Copy Markdown
Contributor

@FantasqueX FantasqueX commented Jul 30, 2024

SIMD version of CV_DESCALE is not correct. It should be implemented using v_dotprod.

What's more, the stop condition of vector operation should be bayer < bayer_end - 14 because we just need to make sure result is safely stored into dst.

Closes: #25823

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

@opencv-alalek opencv-alalek added pr: needs test New functionality requires minimal tests set category: imgproc bug labels Jul 31, 2024
@FantasqueX
Copy link
Copy Markdown
Contributor Author

Hi @opencv-alalek May I ask why you add "pr: needs test" label? Imgproc_ColorBayer.regression can cover the function I change. Do I still need to add a new test?

@asmorkalov
Copy link
Copy Markdown
Contributor

The test passed before and is green now. It means that it's not representative. Could you add something that highlights the issue?

@asmorkalov asmorkalov added this to the 4.11.0 milestone Aug 5, 2024
SIMD version of CV_DESCALE is not correct. It should be implemented
using v_dotprod.

What's more, the stop condition of vector operation should be `bayer <
bayer_end - 14` because we just need to make sure result is safely
stored into `dst`.
@FantasqueX FantasqueX force-pushed the correct-bayer2gray-simd-1 branch from a475a55 to b60703b Compare August 14, 2024 15:01
@FantasqueX
Copy link
Copy Markdown
Contributor Author

@asmorkalov Hi, I have added a regression test which fails without my patch.

Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov left a comment

Choose a reason for hiding this comment

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

Looks good for me in general

@asmorkalov asmorkalov requested a review from mshabunin August 19, 2024 12:00
@asmorkalov
Copy link
Copy Markdown
Contributor

@mshabunin could you take a look too?

@asmorkalov asmorkalov removed the pr: needs test New functionality requires minimal tests set label Aug 19, 2024
@FantasqueX FantasqueX force-pushed the correct-bayer2gray-simd-1 branch from 3a7f07b to 202ea92 Compare August 20, 2024 02:03
@asmorkalov asmorkalov self-assigned this Aug 20, 2024
@asmorkalov asmorkalov merged commit 7cf075c into opencv:4.x Aug 21, 2024
@FantasqueX FantasqueX deleted the correct-bayer2gray-simd-1 branch August 21, 2024 08:35
@asmorkalov asmorkalov mentioned this pull request Aug 27, 2024
thewoz pushed a commit to CobbsLab/OPENCV that referenced this pull request Feb 13, 2025
…md-1

Correct Bayer2Gray u8 SIMD opencv#25968

SIMD version of CV_DESCALE is not correct. It should be implemented using v_dotprod.

What's more, the stop condition of vector operation should be `bayer < bayer_end - 14` because we just need to make sure result is safely stored into `dst`.

Closes: opencv#25823

### 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
- [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.
- [ ] 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

None yet

Development

Successfully merging this pull request may close these issues.

Bayer2Gray SIMD version and non-SIMD version should be equal

3 participants