-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
Added QualityMAE with tests #3691
base: 4.x
Are you sure you want to change the base?
Conversation
@opencv-alalek Sorry to bother you. The project seems not to compile due to something in the ARUCO tests, which I did not touched. What is the should I do to makes things moving forward? |
Please revert your aruco changes - they are not related to this PR (problem should be fixed separately after wrong merge of #3401) |
@opencv-alalek Sorry to bother you with that. I would like to add two contributors to my work, how can I do that? |
You can mention them in the commit message, see https://stackoverflow.com/questions/7442112/how-to-attribute-a-single-commit-to-multiple-developers |
protected: | ||
|
||
/** @brief Reference image, converted to internal mat type */ | ||
QualityBase::_mat_type _ref; | ||
|
||
/** @brief What statistics analysis to apply on the absolute error */ | ||
int _flag; | ||
|
||
/** | ||
@brief Constructor | ||
@param ref reference image, converted to internal type | ||
@param statsProc statistical method to apply on the error | ||
*/ | ||
QualityMAE(QualityBase::_mat_type ref, int statsProc) | ||
: _ref(std::move(ref)), | ||
_flag(statsProc) | ||
{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be in impl part. Please exclude it from public headers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part was originally inspired by the QualityMSE
class which also has it public in the header. I did move the implementation into the .cpp
file.
namespace quality_test | ||
{ | ||
|
||
const cv::Scalar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static? to avoid symbols exporting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made it static, but I was thinking perhaps it can go in the test_precomp.hpp
in the same place as MSE_EXPECTED_1
and MSE_EXPECTED_2
?
…n upper cases, and header files updated accordingly
It's been more than a week without notification from you. Is there anything I should do or anything I missed? |
May I have a feedback about what remains to do, to be allowed to do the pull-request? |
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.
Co-authored-by: Julien Fleuret [email protected]
Co-authored-by: Esmaiel Hoja Najafabadi [email protected]
Co-authored-by: Anshul Joshi [email protected]