Skip to content

Update demosaicing.cpp#18590

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
krush11:master
Oct 16, 2020
Merged

Update demosaicing.cpp#18590
opencv-pushbot merged 1 commit intoopencv:3.4from
krush11:master

Conversation

@krush11
Copy link
Copy Markdown
Contributor

@krush11 krush11 commented Oct 14, 2020

fixes #18558

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 other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to 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

@krush11
Copy link
Copy Markdown
Contributor Author

krush11 commented Oct 14, 2020

fixes #18558

Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

As a "bugfix" this patch should go into 3.4 branch first.
We will merge changes from 3.4 into master regularly (weekly/bi-weekly).

D[0] = S[0];
D[1] = (std::abs(S[-1] - S[1]) > std::abs(S[sstep] - S[-sstep]) ? (S[sstep] + S[-sstep] + 1) : (S[-1] + S[1] + 1)) >> 1;
D[2] = (S[-sstep-1] + S[-sstep+1] + S[sstep-1] + S[sstep+1]) >> 2;
D[2] = (S[-sstep-1] + S[-sstep+1] + S[sstep-1] + S[sstep+1] + 2) >> 2;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To make all code consistent please fix these lines too:

  • 1569
  • 1571

Also need to check SIMDInterpolator implementation.

@krush11 krush11 changed the base branch from master to 3.4 October 16, 2020 04:22
D[blue<<1] = (S[-sstep] + S[sstep] + 2) >> 1;
D[1] = S[0];
D[2-(blue<<1)] = (S[-1] + S[1]) >> 1;
D[2-(blue<<1)] = (S[-1] + S[1] + 2) >> 1;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+ 1 must be here

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.

I did not get you as i am not totally familiar with the code base being a new contributor. Can you elaborate on what you mean by +1 ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should follow the rounding rules. https://en.wikipedia.org/wiki/Rounding

>> 1 means * 0.5 (optimized version)
>> 2 means * 0.25 (optimized version)

You are switching from Rounding towards zero to Round half up.

In the second case you adding 0.5 <==> (x + 2) >> 2
In the first case you should add 0.5 too ==> (x + 1) >> 1

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.

oh ok thanks will make the required changes

@krush11
Copy link
Copy Markdown
Contributor Author

krush11 commented Oct 16, 2020

so do i have to make further changes? or will the PR be merged to 3.4 after clearing all checks?

@krush11 krush11 requested a review from alalek October 16, 2020 11:07
@opencv-pushbot opencv-pushbot merged commit e87ba1d into opencv:3.4 Oct 16, 2020
@alalek alalek mentioned this pull request Oct 21, 2020
@alalek alalek mentioned this pull request Nov 27, 2020
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.

3 participants