Skip to content

imgcodecs: jpeg: add IMWRITE_JPEG_SAMPLING_FACTOR parameter#22064

Merged
alalek merged 5 commits intoopencv:3.4from
Kumataro:3.4-fix22052
Jun 20, 2022
Merged

imgcodecs: jpeg: add IMWRITE_JPEG_SAMPLING_FACTOR parameter#22064
alalek merged 5 commits intoopencv:3.4from
Kumataro:3.4-fix22052

Conversation

@Kumataro
Copy link
Copy Markdown
Contributor

@Kumataro Kumataro commented Jun 3, 2022 •
edited by alalek
Loading

fix #22052

This merge request contains sample and test program.

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
force_builders=Custom
build_image:Custom=centos:7
buildworker:Custom=linux-f1

alalek
alalek reviewed Jun 3, 2022
View reviewed changes
Copy link
Copy Markdown
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 contribution!

Public API changes are good.

modules/imgcodecs/src/grfmt_jpeg.cpp Outdated
Comment on lines +707 to +708
// Unknown parameter is set.
sampling_factor = 0;
Copy link
Copy Markdown
Member

@alalek alalek Jun 3, 2022

Choose a reason for hiding this comment

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

It makes sense to emit warning here through CV_LOG_WARNING(NULL, "Unknown value for IMWRITE_JPEG_SAMPLING_FACTOR: " << sampling_factor);

Copy link
Copy Markdown
Contributor Author

@Kumataro Kumataro Jun 3, 2022

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, i fixed it.

samples/cpp/imgcodecs_jpeg.cpp Outdated
Comment on lines +16 to +18
Mat framebuffer = Mat( 160 * 2, 160 * 5, CV_8UC3, cv::Scalar::all(255) );

Mat img = Mat( 160, 160, CV_8UC3, cv::Scalar::all(255) );
Copy link
Copy Markdown
Member

@alalek alalek Jun 3, 2022

Choose a reason for hiding this comment

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

Mat framebuffer = Mat(...);

No need to copy Mat. Just initialize in ctor: Mat framebuffer(...);

@Kumataro Kumataro mentioned this pull request Jun 3, 2022
Closed
4 tasks
@Kumataro
Copy link
Copy Markdown
Contributor Author

Kumataro commented Jun 4, 2022

Thank you for build and test ! I checked compile log.
For compiling test_npr.cpp, W10/BuildContrib is OK, but W10/BuildAndTest was NG,

I think W10/BuildAndTest will become OK if it run again.

[OK] W10 / BuildContrib

[996/2735] Building CXX object modules\ml\CMakeFiles\opencv_test_ml.dir\test\test_mltests.cpp.obj
[997/2735] Building CXX object modules\photo\CMakeFiles\opencv_test_photo.dir\test\test_denoising.cuda.cpp.obj
[998/2735] Building CXX object modules\photo\CMakeFiles\opencv_test_photo.dir\test\test_npr.cpp.obj
[999/2735] Building CXX object modules\photo\CMakeFiles\opencv_test_photo.dir\test\test_denoising.cpp.obj
[1000/2735] Building CXX object modules\photo\CMakeFiles\opencv_test_photo.dir\test\test_decolor.cpp.obj

[NG] W10 / BuildAndTest

[1031/1914] Building CXX object modules\photo\CMakeFiles\opencv_test_photo.dir\test\test_npr.cpp.obj
FAILED: modules/photo/CMakeFiles/opencv_test_photo.dir/test/test_npr.cpp.obj

@Kumataro
Copy link
Copy Markdown
Contributor Author

Kumataro commented Jun 4, 2022

Thank you for retrying to test. The result of W10/BuildAndTest was success !

alalek
alalek approved these changes Jun 7, 2022
View reviewed changes
Copy link
Copy Markdown
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.

LGTM 👍

samples/cpp/imgcodecs_jpeg.cpp Outdated
Comment on lines +1 to +4
// This file is part of OpenCV project.
// It is subject to the license terms in the LICENSE file found in the top-level directory
// of this distribution and at http://opencv.org/license.html

Copy link
Copy Markdown
Member

@alalek alalek Jun 5, 2022

Choose a reason for hiding this comment

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

We don't have license headers in "samples". Please remove this.

Copy link
Copy Markdown
Contributor Author

@Kumataro Kumataro Jun 20, 2022

Choose a reason for hiding this comment

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

Thank you for review!! I'm very sorry for the delay in responding. I created a commit to remove those lines.

@alalek alalek merged commit dd7b900 into opencv:3.4 Jun 20, 2022
@alalek alalek mentioned this pull request Jun 26, 2022
Merged
@alalek alalek mentioned this pull request Aug 21, 2022
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

Assignees

@alalek alalek

Labels

category: imgcodecs feature

Projects

None yet

Milestone

3.4.19

Development

Successfully merging this pull request may close these issues.

[request] New parameter to imwrite JPEG to disable chroma subsampling

3 participants

@Kumataro @alalek @asmorkalov