Color Calibration Algorithm Implementation Revised #2671

riskiest · 2020-09-17T06:54:31Z

Revise a new PR with branch named 'color-calibration', based on the discussion at: #2653

Color calibration algorithm of OE.30. https://github.com/opencv/opencv/wiki/OE-30.-Color-Calibration
Algorithm struct and other details: https://github.com/riskiest/color_calibration/tree/v4/doc/pdf/English/Algorithm

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

force_builders=linux,Docs,Win64,Mac,Win32,android,ios

parameters

Fix the error whitespace and 'EOF' on the docs.

Implement color distance.

Co-authored-by: Chenqi Shan <[email protected]>

Refer ccm.hpp in entrypoint header and update realted refs in sampe & tutorial

move <iostream> and add getloss() method for class ColorCorrection Model

update sample/color_correction_model.cpp

add getIOs() function for minimize initialization of IO variables

alalek

Please take a look on generated documentation pages:

module.
tutorials

I added commits with changes in public documentation and clang-format for added sources.

alalek · 2020-11-19T20:12:08Z

modules/mcc/include/opencv2/mcc/ccm.hpp

+enum LINEAR_TYPE
+{
+    IDENTITY_,
+    GAMMA,


Enum values are propagated into namespaces.
So it is better to add prefixes to its values, like LINEARIZATION_IDENTITY

P.S. There is enum class in C++ 11, but it is still not supported by OpenCV bindings.

Thanks, I have added prefixes for Enum values.

alalek · 2020-11-19T20:12:27Z

modules/mcc/include/opencv2/mcc/ccm.hpp

+    CMC_1TO1,                   ///< In 1984, the Colour Measurement Committee of the Society of Dyers and Colourists defined a difference measure, also based on the L*C*h color model.
+    CMC_2TO1,
+    RGB,
+    RGBL


Please use prefixes too

alalek · 2020-11-19T20:13:46Z

modules/mcc/include/opencv2/mcc/ccm.hpp

+    Supported list of color cards:
+    - @ref Macbeth (Macbeth ColorChecker)
+    - @ref Vinyl (DKK ColorChecker)
+    - @ref DigitalSG (DigitalSG ColorChecker with 140 squares)


Move "Supported list" out of params or just add reference to enum (use @ref CONST_COLOR for that).

Thanks, I got it.

alalek · 2020-11-19T20:14:29Z

modules/mcc/include/opencv2/mcc/ccm.hpp

+               the color type is RGB not BGR, and the color values are in [0, 1];
+    @param colors the reference color values,the color values are in [0, 1].\n
+    @param ref_cs the corresponding color space
+                NOTICE: For the list of color spaces supported, see the notes above;\n


see the notes above

It is unclear from documentation page which notes are mentioned here

alalek · 2020-11-19T20:17:15Z

modules/mcc/include/opencv2/mcc/ccm.hpp

+    Supported list:
+    - @ref CIE2000
+    - @ref CIE94_GRAPHIC_ARTS
+    - @ref CIE94_TEXTILES
+    - @ref CIE76
+    - @ref CMC_1TO1
+    - @ref CMC_2TO1
+    - @ref RGB (Euclidean distance of rgb color space)
+    - @ref RGBL (Euclidean distance of rgbl color space)


Not sure that it is a good idea to duplicate enum values.
Just reference on enum type is enough.

Additionally it make sense to notice about some exclusions.

Thanks, I think you are right, and I removed enum values from the supported list only if it is necessary.

alalek · 2020-11-19T20:23:04Z

modules/mcc/src/colorspace.hpp

+static std::map<std::tuple<IO, IO, CAM>, Mat> cams;
+static const Mat Von_Kries = (Mat_<double>(3, 3) << 0.40024, 0.7076, -0.08081, -0.2263, 1.16532, 0.0457, 0., 0., 0.91822);
+static const Mat Bradford = (Mat_<double>(3, 3) << 0.8951, 0.2664, -0.1614, -0.7502, 1.7135, 0.0367, 0.0389, -0.0685, 1.0296);
+static const std::map<CAM, std::vector<Mat>> MAs = {
+    { IDENTITY, { Mat::eye(Size(3, 3), CV_64FC1), Mat::eye(Size(3, 3), CV_64FC1) } },
+    { VON_KRIES, { Von_Kries, Von_Kries.inv() } },
+    { BRADFORD, { Bradford, Bradford.inv() } }
+};
+


Reduce initialization of global variables.
Wrap them into functions with "static" variables (functions should return const references).

Done, I moved it into the function which uses these variables.

alalek · 2020-11-19T20:24:24Z

modules/mcc/src/operations.hpp

+    /** @brief run operations to make color conversion
+    */
+    Mat run(Mat abc);
+    static Operations get_IDENTITY_OPS()


return reference

alalek · 2020-11-19T20:27:40Z

modules/mcc/src/color.cpp

+namespace ccm {
+Color::Color()
+    : colors(Mat())
+    , cs(*new ColorSpace())


Please add tests which covers added functionality.

Looks like we have memory leak here.

OK, I have added test files and used share_ptr for Color( ).

modules/mcc/samples/color_correction_model.cpp

alalek · 2020-11-19T20:30:07Z

modules/mcc/include/opencv2/mcc/ccm.hpp

+C_{sl}=0, \qquad C_s=0
+\f]
+
+Because \f$exp(ln(0))\to\infin\f$, the channel whose component is 0 is directly mapped to 0 in the formula above.


Documentation is broken in this place (blames on undefined control sequence \infin)

Thanks, I fixed this issue using \infty.

Ccm pr

update sample --help

alalek

Looks great!
Please apply left cosmetic changes and we ready for merge.

alalek · 2020-11-22T16:12:22Z

modules/mcc/include/opencv2/mcc/ccm.hpp

+    CV_WRAP Mat get_src_rgbl() const;
+    CV_WRAP Mat get_dst_rgbl() const;
+    CV_WRAP Mat get_mask() const;
+    CV_WRAP Mat get_weights() const;


Please follow OpenCV naming convention rules: https://github.com/opencv/opencv/wiki/Coding_Style_Guide#naming-conventions

get_weights() => getWeights()

Also it probably make sense to change return value Mat => const Mat&

Thanks for reviewing the codes. We are encouraged. We renamed functions under OpenCV rules such as getColor() and getMask().

alalek · 2020-11-22T16:14:37Z

modules/mcc/include/opencv2/mcc/ccm.hpp

+Note that the parameter of $ln$ cannot be 0.
+Therefore, we need to delete the channels whose values are 0 from $R_s$ and $R_{dl}$, $G_s$ and $G_{dl}$, $B_s$ and $B_{dl}$.


Please take a look on this part.
It looks broken in documentation.

And several other places, look for $ symbol on documentation page.

Thanks, we fixed the issue and removed $.

alalek · 2020-11-22T16:18:00Z

modules/mcc/include/opencv2/mcc/ccm.hpp

+        @ref LINEARIZATION_COLORPOLYFIT
+        @ref LINEARIZATION_GRAYPOLYFIT
+        @ref LINEARIZATION_COLORLOGPOLYFIT
+        @ref LINEARIZATION_GRAYLOGPOLYFIT


Please extract "valid" list out of @param section (see setColorSpace).

alalek · 2020-11-22T16:19:47Z

modules/mcc/include/opencv2/mcc/ccm.hpp

+    @param upper the upper threshold to determine saturation;
+        NOTICE: it is a tuple of [lower, upper];
+                The colors in the closed interval [lower, upper] are reserved to participate
+                in the calculation of the loss function and initialization parameters\n


Looks like notice is related for both parameters. It make sense to put this description as method body.

alalek

Well done! Thank you for contribution 👍

shanchenqi · 2020-11-24T01:45:01Z

Thanks for your recognition, we will make more contributions.

kevineor

Why removing exposition to colorspace conversions and distances ?
Would be great to have those functions in this module.
OpenCV's cvtcolor can't convert from sRGB to CIELAB D50 2°.

Chenqi Shan and others added 25 commits August 31, 2020 04:04

Add utils, io helpers, the operations for linearization and distance

7a3d082

Add the code for color, colorspace, linearization and ccm computation

51a8d66

Add sample code for color correction

000e0c9

Add the dependency to opencv_imgcodes in CMakeLists.txt

daedbbc

Add the color correction tutorial, introducing build steps and

3987e82

parameters

Add sample code to color correction tutorial

6708cb1

Add color correction algorithms introductions

7d4cc94

Update color_correction_model.markdown

c0ed149

Fix warnings of whitespace, undeclared function, shadow variables.

3131650

Fix the warnings of shadow variables, unused variable in base class.

85fe73b

Fix the error whitespace and 'EOF' on the docs.

Fix the warnnings on win & macos

40b0d1b

Fix bugs & support Vinyl ColorChecker

acb3398

fix shadow variables warning & code style

483400c

update document for sample

d607975

update license

03324c4

fix linearize.hpp

4617f53

Add basic io, utils, operations helpers.

9b82167

Implement color distance.

Implement color, colorspace, linearization and ccm features.

02c1d2a

Add the dependencies to opencv_imgcodecs in CMakeLists.txt

0492014

Add color correction model sample code.

a48a2fd

Co-authored-by: Chenqi Shan <[email protected]>

Add the index markdown of color correction tutorial.

c5246fa

Co-authored-by: Chenqi Shan <[email protected]>

Add the introduction for color correction sample.

8d20535

Split operations into .hpp and .cpp

3696aec

Split mcc, color, colorspace and linearize into .cpp & .hpp

e19da0e

Update test cases

25d0b8a

riskiest mentioned this pull request Sep 17, 2020

Color Calibration Algorithm Implementation #2653

Closed

6 tasks

riskiest force-pushed the color-calibration branch from df58817 to 75f100c Compare September 17, 2020 08:14

Split distance, io and utils into cpp & hpp.

7d133a0

Refer ccm.hpp in entrypoint header and update realted refs in sampe & tutorial

riskiest force-pushed the color-calibration branch from 75f100c to 7d133a0 Compare September 17, 2020 09:10

add static method

0ee295e

riskiest and others added 5 commits November 17, 2020 19:32

Merge pull request #25 from shanchenqi/ccm-pr

5556e24

move <iostream> and add getloss() method for class ColorCorrection Model

update sample/color_correction_model.cpp

a11b115

Merge pull request #26 from shanchenqi/ccm-pr

2aa98ea

update sample/color_correction_model.cpp

add getIOs() function for minimize initialization of IO variables

f60e284

Merge pull request #27 from shanchenqi/ccm-pr

e1f2a8f

add getIOs() function for minimize initialization of IO variables

riskiest requested a review from alalek November 19, 2020 03:04

alalek added 2 commits November 19, 2020 19:54

mcc(ccm): apply clang-format

f97f927

mcc(ccm): fix documentation, code style

60c0be8

alalek reviewed Nov 19, 2020

View reviewed changes

Chenqi Shan and others added 9 commits November 20, 2020 12:43

remove duplicate enum values

592fea1

add prefixes for enum values

4287d41

update codes using cv_Error

5542747

Merge pull request #28 from shanchenqi/ccm-pr

1b35d44

Ccm pr

update test_ccm file

f7aabff

update test_ccm file

ca5c4d5

Merge pull request #30 from shanchenqi/ccm-pr

e5145c8

Ccm pr

update sample --help

92298dd

Merge pull request #31 from shanchenqi/ccm-pr

51274aa

update sample --help

riskiest requested a review from alalek November 22, 2020 04:31

alalek reviewed Nov 22, 2020

View reviewed changes

alalek and others added 5 commits November 22, 2020 17:03

mcc: reduce global initializers

2d96a30

update function naming style

6a18da6

update formulas and note for ccm.hpp

a9f1fb0

add const value

7769198

Merge pull request #32 from shanchenqi/ccm-pr

13e267a

alalek approved these changes Nov 23, 2020

View reviewed changes

alalek merged commit 478cc12 into opencv:master Nov 23, 2020

alalek mentioned this pull request Nov 27, 2020

Merge 4.x #2768

Merged

kevineor reviewed Jan 27, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Color Calibration Algorithm Implementation Revised #2671

Color Calibration Algorithm Implementation Revised #2671

riskiest commented Sep 17, 2020

alalek left a comment

alalek Nov 19, 2020

shanchenqi Nov 20, 2020

alalek Nov 19, 2020

shanchenqi Nov 20, 2020

alalek Nov 19, 2020

shanchenqi Nov 20, 2020

alalek Nov 19, 2020

alalek Nov 19, 2020

shanchenqi Nov 20, 2020

alalek Nov 19, 2020

shanchenqi Nov 20, 2020 •

edited

Loading

alalek Nov 19, 2020

shanchenqi Nov 20, 2020

alalek Nov 19, 2020

shanchenqi Nov 20, 2020

alalek Nov 19, 2020

shanchenqi Nov 20, 2020

alalek left a comment

alalek Nov 22, 2020

shanchenqi Nov 23, 2020 •

edited

Loading

alalek Nov 22, 2020

shanchenqi Nov 23, 2020

alalek Nov 22, 2020

shanchenqi Nov 23, 2020

alalek Nov 22, 2020

shanchenqi Nov 23, 2020

alalek left a comment

shanchenqi commented Nov 24, 2020

kevineor left a comment

		Note that the parameter of $ln$ cannot be 0.
		Therefore, we need to delete the channels whose values are 0 from $R_s$ and $R_{dl}$, $G_s$ and $G_{dl}$, $B_s$ and $B_{dl}$.

Color Calibration Algorithm Implementation Revised #2671

Color Calibration Algorithm Implementation Revised #2671

Conversation

riskiest commented Sep 17, 2020

Pull Request Readiness Checklist

alalek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shanchenqi Nov 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alalek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shanchenqi Nov 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alalek left a comment

Choose a reason for hiding this comment

shanchenqi commented Nov 24, 2020

kevineor left a comment

Choose a reason for hiding this comment

shanchenqi Nov 20, 2020 •

edited

Loading

shanchenqi Nov 23, 2020 •

edited

Loading