Skip to content

dstChannels Lab, Luv, YCrCb and XYZ conversion fix#22962

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
stopmosk:20465-dstchannels-does-not-cover-all-color-codes-1
Dec 23, 2022
Merged

dstChannels Lab, Luv, YCrCb and XYZ conversion fix#22962
opencv-pushbot merged 1 commit intoopencv:3.4from
stopmosk:20465-dstchannels-does-not-cover-all-color-codes-1

Conversation

@stopmosk
Copy link
Copy Markdown
Contributor

@stopmosk stopmosk commented Dec 14, 2022 •
edited
Loading

Define channels number for Lab, Luv, YCrCb and XYZ conversions. Resolves performance issue #20465

Fixes #20465

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

@stopmosk
Copy link
Copy Markdown
Contributor Author

stopmosk commented Dec 14, 2022 •
edited
Loading

Perf test results summary before/after patch (last column - x-factor):

image

Due to the performance problem occurs when the dcn parameter is empty (it by default is set into 0), perf tests were run with changing the original lines in perf_color.cpp in the following way:

...
make_tuple(ConversionTypes(COLOR_RGB2Lab), 3, 0),
make_tuple(ConversionTypes(COLOR_Lab2BGR), 3, 0),
make_tuple(ConversionTypes(COLOR_RGB2Luv), 3, 0),
...

This significant difference in performance (especially in conversion of high resolution images) is explained here:

dstChannels doesn't recognize that specific color code so it forwards a dcn arg value of 0. This stops the OpenCL implementation because it fails a precondition check inside oclCvtColorLab2BGR when constructing the OclHelper.

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Dec 15, 2022

/cc @vrabaud

@asmorkalov asmorkalov requested a review from mshabunin December 15, 2022 06:36
@mshabunin
Copy link
Copy Markdown
Contributor

mshabunin commented Dec 15, 2022

@stopmosk , would it make sense to add perf test modifications to the PR too?

@stopmosk
Copy link
Copy Markdown
Contributor Author

stopmosk commented Dec 15, 2022 •
edited
Loading

@asmorkalov is it correct to replace all explicit dcn values to zeros in performance tests?

Before:

OCL_PERF_TEST_P(CvtColorFixture, CvtColor, testing::Combine(
                OCL_TEST_SIZES,
                testing::Values(
                    make_tuple(ConversionTypes(COLOR_RGB2GRAY), 3, 1),
                    make_tuple(ConversionTypes(COLOR_RGB2BGR), 3, 3),
                    make_tuple(ConversionTypes(COLOR_RGB2YUV), 3, 3),
                    make_tuple(ConversionTypes(COLOR_YUV2RGB), 3, 3),
                    make_tuple(ConversionTypes(COLOR_RGB2YCrCb), 3, 3),
                    make_tuple(ConversionTypes(COLOR_YCrCb2RGB), 3, 3),
                    make_tuple(ConversionTypes(COLOR_RGB2XYZ), 3, 3),
                    make_tuple(ConversionTypes(COLOR_XYZ2RGB), 3, 3),
                    make_tuple(ConversionTypes(COLOR_RGB2HSV), 3, 3),
                    make_tuple(ConversionTypes(COLOR_HSV2RGB), 3, 3),
                    make_tuple(ConversionTypes(COLOR_RGB2HLS), 3, 3),
                    make_tuple(ConversionTypes(COLOR_HLS2RGB), 3, 3),
                    make_tuple(ConversionTypes(COLOR_BGR5652BGR), 2, 3),
                    make_tuple(ConversionTypes(COLOR_BGR2BGR565), 3, 2),
                    make_tuple(ConversionTypes(COLOR_RGBA2mRGBA), 4, 4),
                    make_tuple(ConversionTypes(COLOR_mRGBA2RGBA), 4, 4),
                    make_tuple(ConversionTypes(COLOR_RGB2Lab), 3, 3),
                    make_tuple(ConversionTypes(COLOR_Lab2BGR), 3, 4),
                    make_tuple(ConversionTypes(COLOR_RGB2Luv), 3, 3),
                    make_tuple(ConversionTypes(COLOR_Luv2LBGR), 3, 4),
                    make_tuple(ConversionTypes(COLOR_YUV2RGB_NV12), 1, 3),
                    make_tuple(ConversionTypes(COLOR_YUV2RGB_IYUV), 1, 3),
                    make_tuple(ConversionTypes(COLOR_YUV2GRAY_420), 1, 1),
                    make_tuple(ConversionTypes(COLOR_RGB2YUV_IYUV), 3, 1),
                    make_tuple(ConversionTypes(COLOR_YUV2RGB_YUY2), 2, 3),
                    make_tuple(ConversionTypes(COLOR_YUV2GRAY_YUY2), 2, 1)
                )))
{...}

After:

OCL_PERF_TEST_P(CvtColorFixture, CvtColor, testing::Combine(
                OCL_TEST_SIZES,
                testing::Values(
                    make_tuple(ConversionTypes(COLOR_RGB2GRAY), 3, 0),
                    make_tuple(ConversionTypes(COLOR_RGB2BGR), 3, 0),
                    make_tuple(ConversionTypes(COLOR_RGB2YUV), 3, 0),
                    make_tuple(ConversionTypes(COLOR_YUV2RGB), 3, 0),
                    make_tuple(ConversionTypes(COLOR_RGB2YCrCb), 3, 0),
                    make_tuple(ConversionTypes(COLOR_YCrCb2RGB), 3, 0),
                    make_tuple(ConversionTypes(COLOR_RGB2XYZ), 3, 0),
                    make_tuple(ConversionTypes(COLOR_XYZ2RGB), 3, 0),
                    make_tuple(ConversionTypes(COLOR_RGB2HSV), 3, 0),
                    make_tuple(ConversionTypes(COLOR_HSV2RGB), 3, 0),
                    make_tuple(ConversionTypes(COLOR_RGB2HLS), 3, 0),
                    make_tuple(ConversionTypes(COLOR_HLS2RGB), 3, 0),
                    make_tuple(ConversionTypes(COLOR_BGR5652BGR), 2, 0),
                    make_tuple(ConversionTypes(COLOR_BGR2BGR565), 3, 0),
                    make_tuple(ConversionTypes(COLOR_RGBA2mRGBA), 4, 0),
                    make_tuple(ConversionTypes(COLOR_mRGBA2RGBA), 4, 0),
                    make_tuple(ConversionTypes(COLOR_RGB2Lab), 3, 0),
                    make_tuple(ConversionTypes(COLOR_Lab2BGR), 3, 0),
                    make_tuple(ConversionTypes(COLOR_RGB2Luv), 3, 0),
                    make_tuple(ConversionTypes(COLOR_Luv2LBGR), 3, 0),
                    make_tuple(ConversionTypes(COLOR_YUV2RGB_NV12), 1, 0),
                    make_tuple(ConversionTypes(COLOR_YUV2RGB_IYUV), 1, 0),
                    make_tuple(ConversionTypes(COLOR_YUV2GRAY_420), 1, 0),
                    make_tuple(ConversionTypes(COLOR_RGB2YUV_IYUV), 3, 0),
                    make_tuple(ConversionTypes(COLOR_YUV2RGB_YUY2), 2, 0),
                    make_tuple(ConversionTypes(COLOR_YUV2GRAY_YUY2), 2, 0)
                )))
{...}

Or are explicit values important to the test and should be kept, and tests with zero dcn values added additionally?

@alalek
Copy link
Copy Markdown
Member

alalek commented Dec 16, 2022

I propose to add new set of perf tests with ZeroDstChannels mark instead of modification of existed tests (modification of performance tests content should be avoided to keep ability to have comparable reports of different versions):

OCL_PERF_TEST_P(CvtColorZeroDstChannelsFixture, CvtColor, testing::Combine(

@stopmosk stopmosk force-pushed the 20465-dstchannels-does-not-cover-all-color-codes-1 branch from 7fb0e58 to 1339c7f Compare December 23, 2022 10:20
@asmorkalov asmorkalov changed the base branch from 4.x to 3.4 December 23, 2022 10:31
@asmorkalov asmorkalov removed the pr: needs rebase Rebase patch (and squash fixup commits) on the top of target branch label Dec 23, 2022
@opencv-pushbot opencv-pushbot merged commit 139bd30 into opencv:3.4 Dec 23, 2022
@alalek alalek mentioned this pull request Dec 24, 2022
Merged
@alalek alalek mentioned this pull request Jan 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alalek alalek alalek approved these changes

@mshabunin mshabunin Awaiting requested review from mshabunin

Assignees

@alalek alalek

Labels

category: imgproc optimization

Projects

None yet

Milestone

3.4.19

Development

Successfully merging this pull request may close these issues.

dstChannels does not cover all color codes

5 participants

@stopmosk @asmorkalov @mshabunin @alalek @opencv-pushbot