Skip to content

Fix zlib-ng version parse#25577

Merged
asmorkalov merged 1 commit intoopencv:4.xfrom
FantasqueX:fix-zlib-ng-version-parse-1
May 14, 2024
Merged

Fix zlib-ng version parse#25577
asmorkalov merged 1 commit intoopencv:4.xfrom
FantasqueX:fix-zlib-ng-version-parse-1

Conversation

@FantasqueX
Copy link
Copy Markdown
Contributor

@FantasqueX FantasqueX commented May 13, 2024

Currently, zlib-ng version is 'zlib ver #define ZLIB_VERSION "1.3.0.zlib-ng"'. Because ocv_parse_header_version only accepts dot and numbers and doesn't accepts 1.3.0.zlib-ng. This patch changes ocv_parse_header_version to accept all characters between parentheses.

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

@FantasqueX
Fix zlib-ng version parse
023d805 
Currently, zlib-ng version is 'zlib ver #define ZLIB_VERSION "1.3.0.zlib-ng"'. Because ocv_parse_header_version only accepts dot and numbers and doesn't accepts 1.3.0.zlib-ng. This patch changes ocv_parse_header_version to accept all characters between parentheses.
@sturkmen72
Copy link
Copy Markdown
Contributor

sturkmen72 commented May 13, 2024

-- Media I/O:
-- ZLib-Ng: build (zlib ver #define ZLIB_VERSION "1.3.0.zlib-ng", zlib-ng ver 2.1.6)
-- JPEG: build-libjpeg-turbo (ver 2.1.3-62)
-- SIMD Support Request: YES
-- SIMD Support: YES
-- PNG: build (ver 1.6.37)
-- HDR: YES
-- SUNRASTER: YES
-- PXM: YES
-- PFM: YES

currently cmake output seems like above. is this PR fix this?

@FantasqueX
Copy link
Copy Markdown
Contributor Author

FantasqueX commented May 13, 2024

-- Media I/O: -- ZLib-Ng: build (zlib ver #define ZLIB_VERSION "1.3.0.zlib-ng", zlib-ng ver 2.1.6) -- JPEG: build-libjpeg-turbo (ver 2.1.3-62) -- SIMD Support Request: YES -- SIMD Support: YES -- PNG: build (ver 1.6.37) -- HDR: YES -- SUNRASTER: YES -- PXM: YES -- PFM: YES

currently cmake output seems like above. is this PR fix this?

Yes

@sturkmen72
Copy link
Copy Markdown
Contributor

sturkmen72 commented May 13, 2024

-- Media I/O:
-- ZLib-Ng: build (zlib ver 1.3.0.zlib-ng, zlib-ng ver 2.1.6)
-- JPEG: build-libjpeg-turbo (ver 2.1.3-62)
-- SIMD Support Request: YES
-- SIMD Support: YES
-- PNG: build (ver 1.6.37)
-- TIFF: build (ver 42 - 4.6.0)
-- HDR: YES
-- SUNRASTER: YES
-- PXM: YES
-- PFM: YES

output after patch

@asmorkalov asmorkalov self-requested a review May 13, 2024 12:25
@asmorkalov asmorkalov added this to the 4.10.0 milestone May 13, 2024
@asmorkalov asmorkalov self-assigned this May 13, 2024
@sturkmen72
Copy link
Copy Markdown
Contributor

sturkmen72 commented May 13, 2024

just a note:
#define ZLIB_VERSION "1.3.1"

#define ZLIB_VERNUM 0x1310
#define ZLIB_VER_MAJOR 1
#define ZLIB_VER_MINOR 3
#define ZLIB_VER_REVISION 1
#define ZLIB_VER_SUBREVISION 0

@FantasqueX
Copy link
Copy Markdown
Contributor Author

FantasqueX commented May 13, 2024

-- Media I/O: -- ZLib-Ng: build (zlib ver 1.3.0.zlib-ng, zlib-ng ver 2.1.6) -- JPEG: build-libjpeg-turbo (ver 2.1.3-62) -- SIMD Support Request: YES -- SIMD Support: YES -- PNG: build (ver 1.6.37) -- TIFF: build (ver 42 - 4.6.0) -- HDR: YES -- SUNRASTER: YES -- PXM: YES -- PFM: YES

output after patch

Great, as expected.

asmorkalov
asmorkalov approved these changes May 13, 2024
View reviewed changes
Copy link
Copy Markdown
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
Copy link
Copy Markdown
Contributor

asmorkalov commented May 13, 2024

@FantasqueX please provide cmake options and platform how you got the issue. The patch looks reasonable, but I cannot repoduce the original issue.

@FantasqueX
Copy link
Copy Markdown
Contributor Author

FantasqueX commented May 13, 2024 •
edited
Loading

@FantasqueX please provide cmake options and platform how you got the issue. The patch looks reasonable, but I cannot repoduce the original issue.

cmake -DCMAKE_BUILD_TYPE=Debug -DWITH_ZLIB_NG=ON ..
I noticed this issue on Windows 11. However, I cannot reproduce it on ArchLinux :(
Windows 11: -- ZLib-Ng: build (zlib ver #define ZLIB_VERSION "1.3.0.zlib-ng", zlib-ng ver 2.1.6)
ArchLinux: -- ZLib-Ng: build (zlib ver 1.3.0, zlib-ng ver 2.1.6)

@FantasqueX
Copy link
Copy Markdown
Contributor Author

FantasqueX commented May 14, 2024 •
edited
Loading

I successfully reproduced on ArchLinux. The reason is that other find_package commands after ocv_parse_header_version(ZLIB "${${ZLIB_LIBRARY}_SOURCE_DIR}/zlib.h.in" ZLIB_VERSION) include find_package(ZLIB) which results in ZLIB_VERSION_STRING changed.
Here is the reproducing command:
cmake -DCMAKE_BUILD_TYPE=Debug -DWITH_ZLIB_NG=ON -DWITH_PNG=ON -DBUILD_PNG=ON -DWITH_TIFF=OFF -DWITH_OPENEXR=OFF -DWITH_VTK=OFF ..

@asmorkalov asmorkalov merged commit 02ad8c2 into opencv:4.x May 14, 2024
@mshabunin mshabunin mentioned this pull request Jun 14, 2024
Merged
@FantasqueX FantasqueX deleted the fix-zlib-ng-version-parse-1 branch June 28, 2024 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asmorkalov asmorkalov asmorkalov approved these changes

Assignees

@asmorkalov asmorkalov

Labels

category: build/install

Projects

None yet

Milestone

4.10.0

Development

Successfully merging this pull request may close these issues.

3 participants

@FantasqueX @sturkmen72 @asmorkalov