Skip to content

Cleanup r1 #4

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

Merged
merged 13 commits into from
Aug 6, 2019
Merged

Cleanup r1 #4

merged 13 commits into from
Aug 6, 2019

Conversation

native-api
Copy link

This fixes and cleans up dockerfiles.
Let's get this over with.

@abhiTronix
Copy link
Owner

@native-api Thanks so much for this, but we need some additional changes as you remove libjpeg-turbo too.

@abhiTronix abhiTronix added the enhancement New feature or request label Aug 6, 2019
@native-api
Copy link
Author

native-api commented Aug 6, 2019

No need for changes. The build logic will detect that relevant envvars are no longer present and use the one bundled with OpenCV.
You can delete the corresponding logic from setup.py altogether if you'd like to but it's not strictly necessary atm.

@native-api
Copy link
Author

I deleted libjpeg-turbo from dockerfile because it was failing the build, see commit message.

@abhiTronix
Copy link
Owner

abhiTronix commented Aug 6, 2019

@native-api Change this part here from setup.py which redirects OpenCV to use custom jpeg libs:

if all(v in os.environ for v in ('JPEG_INCLUDE_DIR', 'JPEG_LIBRARY')):
            cmake_args += [
                "-DBUILD_JPEG=OFF",
                "-DJPEG_INCLUDE_DIR=%s" % os.environ['JPEG_INCLUDE_DIR'],
                "-DJPEG_LIBRARY=%s" % os.environ['JPEG_LIBRARY']
            ]

to

 cmake_args += ["-DBUILD_JPEG=ON"]

Or simply just remove it entirely, as "-DBUILD_JPEG=ON" is set by default in OpenCV cmake .

@abhiTronix
Copy link
Owner

@native-api This change is very important as build will fail if no jpeg libs are found.

@native-api
Copy link
Author

if all(v in os.environ for v in ('JPEG_INCLUDE_DIR', 'JPEG_LIBRARY')):

this ensures that a custom JPEG library is only used if JPEG_INCLUDE_DIR and JPEG_LIBRARY envvars are present.

@abhiTronix

This comment has been minimized.

@abhiTronix

This comment has been minimized.

@native-api
Copy link
Author

native-api commented Aug 6, 2019

As it is now, the bundled jpeg-turbo is already used, I checked:

--   Media I/O: 
--     ZLib:                        /lib/libz.so (ver 1.2.3)
--     JPEG:                        libjpeg-turbo (ver 2.0.2-62)
<...>
[  4%] Built target libtiff
[  9%] Built target libjpeg-turbo
[  5%] Built target libpng
<...>

@native-api
Copy link
Author

If you can't understand why there's no problem due to #4 (comment) , just trust me on this.

@abhiTronix
Copy link
Owner

@native-api ooh, I misread that comment. My apologies. I'll merge it.

@abhiTronix abhiTronix merged commit 399baca into abhiTronix:master Aug 6, 2019
@native-api native-api deleted the cleanup_r1 branch August 6, 2019 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants