Skip to content

Added OpenJPEG source code as a part of 3rdparty#18194

Merged
opencv-pushbot merged 2 commits intoopencv:masterfrom
VadimLevin:dev/vlevin/openjpeg-source-intergration
Sep 2, 2020
Merged

Added OpenJPEG source code as a part of 3rdparty#18194
opencv-pushbot merged 2 commits intoopencv:masterfrom
VadimLevin:dev/vlevin/openjpeg-source-intergration

Conversation

@VadimLevin
Copy link
Copy Markdown
Contributor

@VadimLevin VadimLevin commented Aug 26, 2020 •
edited by alalek
Loading

OpenJPEG support for JPEG 2000 was added via #16494 and next step is to add OpenJPEG source code to 3rdparty as it is done for other libraries used by the imgcodecs module.

This patch adds OpenJPEG 2.3.1 to 3rdparty.

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
allow_multiple_commits=1

@mshabunin
Copy link
Copy Markdown
Contributor

mshabunin commented Aug 26, 2020 •
edited
Loading

I hope we would consider using modern methods of adding 3rdparty dependencies:

@alalek
Copy link
Copy Markdown
Member

alalek commented Aug 26, 2020

Current approach is fine for non-huge 3rdparty projects.

Modern methods require additional investigation:

  • ExternalProject can't work with CMake targets
  • FetchContent is just a downloader (and requires latest CMake, Ubuntu 18.04 contains CMake 3.10.x only)
  • install rules?

@VadimLevin Please squash commits onto one, we don't need to keep whole history (at least removed files).

@mshabunin
Copy link
Copy Markdown
Contributor

mshabunin commented Aug 26, 2020

FetchContent is just a downloader

Yes and we have our own downloader ocv_download which is used to download and build TBB library for example.

@VadimLevin VadimLevin force-pushed the dev/vlevin/openjpeg-source-intergration branch 2 times, most recently from 9067da5 to 2edb13c Compare August 26, 2020 18:31
@alalek
Copy link
Copy Markdown
Member

alalek commented Aug 27, 2020

@VadimLevin Please add temporary test commit with enabled option here (will be removed before merge through git rebase)

Please add BUILD_OPENJPEG option few lines below and add handling.

@VadimLevin
Copy link
Copy Markdown
Contributor Author

VadimLevin commented Aug 27, 2020

@VadimLevin Please add temporary test commit with enabled option here (will be removed before merge through git rebase)

Please add BUILD_OPENJPEG option few lines below and add handling.

Done (warnings are not related to OpenJPEG)

Unfortunately, build fails on Windows and it confuses me, because linker string looks correct and all libraries are built.

/OUT:"C:\Users\testuser\Documents\OpenCV\opencv\build\bin\Release\opencv_imgcodecs440.dll" /MANIFEST /NXCOMPAT /PDB:"C:/Users/testuser/Documents/OpenCV/opencv/build/bin/Release/opencv_imgcodecs440.pdb" /DYNAMICBASE "kernel32.lib" "user32.lib" "gdi32.lib" "winspool.lib" "shell32.lib" "ole32.lib" "oleaut32.lib" "uuid.lib" "comdlg32.lib" "advapi32.lib" "..\..\lib\Release\opencv_imgproc440.lib" "..\..\3rdparty\lib\Release\ippiw.lib" "..\..\3rdparty\ippicv\ippicv_win\icv\lib\intel64\ippicvmt.lib" "..\..\3rdparty\lib\Release\zlib.lib" "..\..\3rdparty\lib\Release\libjpeg-turbo.lib" "..\..\3rdparty\lib\Release\libwebp.lib" "..\..\3rdparty\lib\Release\libpng.lib" "..\..\3rdparty\lib\Release\libtiff.lib" "..\..\3rdparty\lib\Release\openjp2.lib" "..\..\3rdparty\lib\Release\IlmImf.lib" "..\..\lib\Release\opencv_core440.lib" /IMPLIB:"C:/Users/testuser/Documents/OpenCV/opencv/build/lib/Release/opencv_imgcodecs440.lib" /DLL /MACHINE:X64 /NODEFAULTLIB:"atlthunk.lib" /NODEFAULTLIB:"atlsd.lib" /NODEFAULTLIB:"libcmt.lib" /INCREMENTAL:NO /PGD:"C:\Users\testuser\Documents\OpenCV\opencv\build\bin\Release\opencv_imgcodecs440.pgd" /SUBSYSTEM:CONSOLE /MANIFESTUAC:"level='asInvoker' uiAccess='false'" /ManifestFile:"opencv_imgcodecs.dir\Release\opencv_imgcodecs440.dll.intermediate.manifest" /ERRORREPORT:PROMPT /NOLOGO /TLBID:1

It fails to find all symbols from openjp2, but it presents in the pointed directory.

@VadimLevin VadimLevin force-pushed the dev/vlevin/openjpeg-source-intergration branch 2 times, most recently from 73b2b3f to 6884dc6 Compare August 28, 2020 21:16
@VadimLevin
Copy link
Copy Markdown
Contributor Author

VadimLevin commented Aug 28, 2020

@alalek I fixed all configuration and build errors related to the OpenJPEG built from sources, can you proceed with review, please?

alalek
alalek reviewed Aug 31, 2020
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.

Please squash all non-test commits into one to keep small patch (we don't need whole history with added/removed files).

@VadimLevin VadimLevin force-pushed the dev/vlevin/openjpeg-source-intergration branch 2 times, most recently from 47119d7 to 5a07119 Compare September 2, 2020 08:34
@VadimLevin
Copy link
Copy Markdown
Contributor Author

VadimLevin commented Sep 2, 2020

@alalek Thank you for review, I've reverted changes in 3rdparty code and suppressed warnings, everything should pass smoothly.

@alalek
Copy link
Copy Markdown
Member

alalek commented Sep 2, 2020

After cleanup of dead files:

patch size opencv: 297 KiB

I believe we can bypass ocv_download() approach here (size of openjpeg-2.3.1.tar.gz is 2Mb)

alalek
alalek approved these changes Sep 2, 2020
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!

@opencv-pushbot opencv-pushbot merged commit cf8322c into opencv:master Sep 2, 2020
@alalek alalek mentioned this pull request Mar 1, 2021
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: build/install category: imgcodecs category: 3rdparty feature

Projects

None yet

Milestone

4.5.0

Development

Successfully merging this pull request may close these issues.

4 participants

@VadimLevin @mshabunin @alalek @opencv-pushbot