Skip to content
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

improvements on tests of imagecodecs module #25525

Merged
merged 1 commit into from May 17, 2024

Conversation

sturkmen72
Copy link
Contributor

@sturkmen72 sturkmen72 commented May 2, 2024

[WIP] trivial improvements on tests of imagecodecs module

@sturkmen72 sturkmen72 force-pushed the png-spng-test branch 3 times, most recently from cd6cce4 to f925363 Compare May 3, 2024 15:37
@sturkmen72 sturkmen72 changed the title png spng test improvements on tests of imagecodecs module May 3, 2024
@sturkmen72 sturkmen72 force-pushed the png-spng-test branch 2 times, most recently from 9e41d0b to 8fa2fb7 Compare May 3, 2024 17:43
@sturkmen72
Copy link
Contributor Author

sturkmen72 commented May 3, 2024

i was not intending to prepare a PR to merge. but now i thing the proposed changes will be useful.

please review and consider the file changes baboon.ppm and read.png ( let me create a PR to opencv_extra if this PR acceptable)

@asmorkalov
Copy link
Contributor

@sturkmen72 What is your motivation of png -> ppm replacement?

@asmorkalov asmorkalov self-requested a review May 6, 2024 05:43
@asmorkalov asmorkalov self-assigned this May 6, 2024
@asmorkalov asmorkalov added this to the 4.10.0 milestone May 6, 2024
@sturkmen72
Copy link
Contributor Author

@sturkmen72 What is your motivation of png -> ppm replacement?

  1. i replaced baboon.ppm with correct one ( older was an unrelated image and probably not used in tests) using this new image in tests will be meaningful.
  2. "#if defined(HAVE_PNG) || defined(HAVE_SPNG)" alredy exist in code and HAVE_PNG can be OFF in very rare cases.
  3. a bit related "IT’S TIME TO RETIRE LENA FROM COMPUTER SCIENCE"

As a final word, the proposed changes are open to discussion. It may be unimportant. Let me do required changes according devs opinions.

@asmorkalov
Copy link
Contributor

@mshabunin @opencv-alalek what is you opinion?

modules/imgcodecs/test/test_common.cpp Outdated Show resolved Hide resolved
@@ -87,11 +87,17 @@ const string all_images[] =
"readwrite/uint16-mono2.dcm",
"readwrite/uint8-rgb.dcm",
#endif
#if defined(HAVE_PNG) || defined(HAVE_SPNG)
Copy link
Contributor

Choose a reason for hiding this comment

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

We have GDAL library integration which should support all these image types (PNG, JPEG, TIFF). Perhaps it should be accounted too, though I didn't try to test GDAL backend specifically (with no other backends).

modules/imgcodecs/test/test_jpeg.cpp Outdated Show resolved Hide resolved
@sturkmen72 sturkmen72 force-pushed the png-spng-test branch 3 times, most recently from 68f2fad to f3199a0 Compare May 12, 2024 20:14
@vpisarev
Copy link
Contributor

I would choose beautiful Lena over less-than-beautiful baboon. Honestly, I don't see any real justification to make this change.

Copy link
Contributor

@asmorkalov asmorkalov left a comment

Choose a reason for hiding this comment

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

👍

@asmorkalov asmorkalov merged commit 5e78878 into opencv:4.x May 17, 2024
28 checks passed
@sturkmen72 sturkmen72 deleted the png-spng-test branch May 17, 2024 09:14
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.

None yet

4 participants