Skip to content

ximgproc: add loose comparison based on pixel count#17691

Closed
tomoaki0705 wants to merge 1 commit intoopencv:3.4from
tomoaki0705:fixFbs
Closed

ximgproc: add loose comparison based on pixel count#17691
tomoaki0705 wants to merge 1 commit intoopencv:3.4from
tomoaki0705:fixFbs

Conversation

@tomoaki0705
Copy link
Copy Markdown
Contributor

realtes opencv/opencv_contrib#2581

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 OpenCV (BSD) 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

void compare(const Mat& src, double s, Mat& dst, int cmpop);

int countExceed(const Mat& src, double value, int cmpop);
int countExceed(const Mat& src1, const Mat& src2, double value, int cmpop);
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.

Functions around the added stuff are reference implementations to test optimizations of OpenCV basic functionality. They do not perform any "checks".

"Check" functions should be created as GoogleTest reporters (with "void" result value).
Also you can see take a look on macros with EXPECT_ prefix from ts/include.

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.

Functions around the added stuff are reference implementations to test optimizations of OpenCV basic functionality. They do not perform any "checks".

Right they don't perform any checks, but this function only "counts" and doesn't "checks".
I expect the check to be done by EXPECT_LE like here

I'm not sure if we are on the same page.
Yes, may be the title of this PR was misleading. If so, I can update the title.
If I'm not understanding your opinion correctly, could you guide me to any implementations I should follow, please ?

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.

This is wrong place for check helper functions.
There are reference implementations to test optimizations of OpenCV basic functionality.
Lets do not mess them.

I should follow

I will add more constructive comment later. Proper infrastructure decision is required here.

Copy link
Copy Markdown
Member

@alalek alalek Jul 2, 2020

Choose a reason for hiding this comment

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

@tomoaki0705 There is no architecture decision yet. Could you please move code of this implementation into related opencv_contrib PR to merge it and to unblock testing?

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 pushed so opencv/opencv_contrib#2581 should be able to build/pass without this PR

@tomoaki0705 tomoaki0705 closed this Jul 2, 2020
@tomoaki0705 tomoaki0705 deleted the fixFbs branch July 2, 2020 08:46
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.

2 participants