Skip to content

new module: intensity_transform #2399

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

Merged

Conversation

elim-dk
Copy link
Contributor

@elim-dk elim-dk commented Dec 31, 2019

The new module, intensity_transform, implements the following intensity transformation algorithms: gamma correction, log transformation, autoscaling, and contrast stretching.

This pullrequest changes

Adds a new module called intensity_transform.

@elim-dk elim-dk force-pushed the algorithm/intensity-transformations branch from 5f74f07 to bf25db3 Compare December 31, 2019 18:12
Copy link
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.

Thank you for the contribution!

Please take a look on comments below.

@@ -0,0 +1,2 @@
set(the_description "Intensity transformations")
ocv_define_module(intensity_transform opencv_core opencv_imgcodecs WRAP python)
Copy link
Member

Choose a reason for hiding this comment

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

imgcodecs

Looks like this dependency is not used in module itself ("samples" has own extra dependencies list).
Please remove this from here and precomp.hpp

* @param input input bgr or grayscale image.
* @param output resulting image of log transformations.
*/
CV_EXPORTS_W void logTransform(const cv::Mat input, cv::Mat& output);
Copy link
Member

Choose a reason for hiding this comment

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

[code style] Please avoid using of "cv::" in OpenCV files for Mat.


void contrastStretching(const Mat input, Mat& output, const int r1, const int s1, const int r2, const int s2)
{
vector<int> table(256);
Copy link
Member

Choose a reason for hiding this comment

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

std::array ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your comment. Since cv::LUT method takes in cv::_InputArray as the argument, std::array didn't work here.

Copy link
Member

Choose a reason for hiding this comment

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

It should work, related code is here.

BTW, Use std::array<uchar, 256> table; and remove final .convertTo().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the document! But, when I use std::array<uchar, 256> table;, I get the following error:
candidate function not viable: no known conversion from 'std::array<uchar, 256>' (aka 'array<unsigned char, 256>') to 'const cv::_InputArray' for 2nd argument CV_EXPORTS_W void LUT(InputArray src, InputArray lut, OutputArray dst);
Any idea why?

Copy link
Member

Choose a reason for hiding this comment

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

Which compiler is used?
Which version of code from main "opencv" repository is used?

Could you try LUT(..., InputArray(table), ...)?

Copy link
Member

Choose a reason for hiding this comment

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

This commit works well (pass CI checks, see #2366): b05b328

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great, thank you! Made the change.


TEST(intensity_transform_logTransform, accuracy)
{
cv::Mat const image = (cv::Mat_<uchar>(10, 10) <<
Copy link
Member

Choose a reason for hiding this comment

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

Could you please avoid using of this initializer in OpenCV? Consider using RAW array instead.
see opencv/opencv#15959 (comment)

{
for(int j = 0; j < res.rows; ++j)
{
EXPECT_EQ((int)res.at<uint8_t>(i,j), (int)expectedRes.at<uint8_t>(i,j));
Copy link
Member

Choose a reason for hiding this comment

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

loops =>

EXPECT_EQ(0, cvtest::norm(res, expectedRes, NORM_INF));

Please note that this check is very strong (and should be avoided due non bit-exact floating-point implementation).
It is better to relax conditions:

EXPECT_LE(cvtest::norm(res, expectedRes, NORM_INF), 1);
EXPECT_LE(cvtest::norm(res, expectedRes, NORM_L1), 10);

@elim-dk elim-dk force-pushed the algorithm/intensity-transformations branch 2 times, most recently from 9383248 to 88faaa7 Compare January 12, 2020 15:48
Comment on lines 19 to 31
if (input.channels() == 1)
{
output = input + 1;
}
else
{
output = input + Scalar(1, 1, 1);
}

output.convertTo(output, CV_64F);
cv::log(output, output);
output *= c;
output.convertTo(output, CV_8UC3);
Copy link
Member

Choose a reason for hiding this comment

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

Mat::convertTo() with alpha/beta parameters can reduce number of operations:

Mat in_plus_1_64f;
input.convertTo(in_plus_1_64f, CV_64F, 1, 1.0f);  // input + 1
Mat log_64f;
log(in_plus_1_64f, log_64f);
log_64f.convertTo(output, CV_8UC3, c, 0.0f);  // log_64f * c

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's useful! Thank you for the feedback!


void contrastStretching(const Mat input, Mat& output, const int r1, const int s1, const int r2, const int s2)
{
vector<int> table(256);
Copy link
Member

Choose a reason for hiding this comment

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

It should work, related code is here.

BTW, Use std::array<uchar, 256> table; and remove final .convertTo().

…fomration, autoscaling, contrast stretching)

build error fix: remove trailing whitespaces, casting types

minor edits based on alalek's feedback

removing trailing whitespace

using std::array for LUT argument
@elim-dk elim-dk force-pushed the algorithm/intensity-transformations branch from 88faaa7 to 9aa74df Compare January 27, 2020 21:44
Copy link
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.

Well done! Thank you 👍

@opencv-pushbot opencv-pushbot merged commit 9aa74df into opencv:master Jan 29, 2020
@elim-dk
Copy link
Contributor Author

elim-dk commented Jan 29, 2020

Well done! Thank you 👍

Thank you for all your constructive comments and help!

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