PR: Enable libPNG support #2379

andfoy · 2020-07-01T16:59:43Z

This PR fixes the regressions introduced in #2301

packaging/torchvision/conda_build_config.yaml

fmassa · 2020-07-01T17:20:41Z

setup.py


    ext_modules = [
        extension(
            'torchvision._C',
            sources,
            include_dirs=include_dirs,
+            library_dirs=library_dirs,


nit: this is not needed anymore

maybe it is, given that we changed include_dirs, good to double-check

I think it might be good to include if future functions require other libraries?

…sion

codecov · 2020-07-01T18:09:52Z

Codecov Report

Merging #2379 into master will decrease coverage by 0.07%.
The diff coverage is 51.61%.

@@            Coverage Diff             @@
##           master    #2379      +/-   ##
==========================================
- Coverage   70.73%   70.66%   -0.08%     
==========================================
  Files          93       94       +1     
  Lines        7768     7799      +31     
  Branches     1199     1204       +5     
==========================================
+ Hits         5495     5511      +16     
- Misses       1910     1925      +15     
  Partials      363      363

Impacted Files	Coverage Δ
torchvision/io/__init__.py	`100.00% <ø> (ø)`
torchvision/io/image.py	`51.61% <51.61%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa6af6d...84dad44. Read the comment docs.

fmassa · 2020-07-02T09:11:56Z

.circleci/unittest/linux/scripts/environment.yml

@@ -6,6 +6,7 @@ dependencies:
  - pytest-cov
  - codecov
  - pip
+  - libpng


I think the same is needed for Windows in https://github.com/pytorch/vision/blob/master/.circleci/unittest/windows/scripts/environment.yml ? Not sure why tests are not failing there

fmassa

Tests seem to be passing, although I'm not sure why we don't need to add libpng to https://github.com/pytorch/vision/blob/master/.circleci/unittest/windows/scripts/environment.yml as well.

Let's merge and see if all the builds pass on master.

This reverts commit c1a99b7.

peterjc123 · 2020-07-02T14:47:44Z

setup.py

+    image_link_flags = []
+
+    # Locating libPNG
+    libpng = distutils.spawn.find_executable('libpng-config')


I tried this locally on a Windows box after doing conda install libpng and it returns None.

I suggest changing this to pngfix. as it is more robust and is listed in the official page. http://www.linuxfromscratch.org/blfs/view/svn/general/libpng.html

Suggested change

libpng = distutils.spawn.find_executable('libpng-config')

libpng = distutils.spawn.find_executable('libpng-config')

pngfix = distutils.spawn.find_executable('pngfix')

peterjc123 · 2020-07-02T14:59:37Z

setup.py

+        png_version = parse_version(png_version)
+        if png_version >= parse_version("1.6.0"):
+            print('Building torchvision with PNG image support')
+            png_lib = subprocess.run([libpng, '--libdir'],


Ok, I saw this. Unluckily, on Windows, libpng-config doesn't exist. But there is a CMake configuration. Reading that file, it seems that you'll only need to link against libpng.lib and set the corresponding include directory include\libpng16.

peterjc123 · 2020-07-02T15:06:40Z

setup.py

+    png_found = libpng is not None
+    image_macros += [('PNG_FOUND', str(int(png_found)))]
+    print('PNG found: {0}'.format(png_found))
+    if png_found:


Suggested change

if png_found:

if png_found:

if libpng is not None:

# Unix / Mac

else:

# Windows

png_lib = os.path.join(os.path.dirname(os.path.dirname(pngfix)), 'lib')

png_include = os.path.join(os.path.dirname(os.path.dirname(pngfix)), 'include', 'libpng16')

peterjc123 · 2020-07-02T15:07:47Z

setup.py

+
+    # Locating libPNG
+    libpng = distutils.spawn.find_executable('libpng-config')
+    png_found = libpng is not None


Suggested change

png_found = libpng is not None

png_found = libpng is not None or pngfix is not None

peterjc123 · 2020-07-02T15:12:21Z

torchvision/csrc/cpu/image/image.cpp

+// If we are in a Windows environment, we need to define
+// initialization functions for the _custom_ops extension
+#ifdef _WIN32
+#if PY_MAJOR_VERSION < 3


nit: It is not no longer needed as PyTorch has dropped Python 2 support.

peterjc123 · 2020-07-02T15:15:46Z

setup.py

@@ -243,7 +359,9 @@ def run(self):

    # Package info
    packages=find_packages(exclude=('test',)),
-
+    package_data={
+        package_name: ['*.lib', '*.dylib', '*.so']


and .dll I think.

Suggested change

package_name: ['*.lib', '*.dylib', '*.so']

package_name: ['*.dll', '*.dylib', '*.so']

peterjc123 · 2020-07-02T15:16:02Z

test/test_image.py

+
+class ImageTester(unittest.TestCase):
+    def test_read_png(self):
+        for img_path in get_images(IMAGE_DIR, "png"):


peterjc123 · 2020-07-02T15:16:09Z

test/test_image.py

+            self.assertEqual(img_lpng, img_pil)
+
+    def test_decode_png(self):
+        for img_path in get_images(IMAGE_DIR, "png"):


peterjc123 · 2020-07-02T15:17:29Z

@andfoy I made a few suggestions in this PR. Maybe you could take a look.

peterjc123 · 2020-07-02T15:28:27Z

packaging/build_wheel.sh

+        cp "$env_path/lib/libpng16.dylib" torchvision
+    else
+        # Include libPNG
+        cp "$bin_path/Library/lib/libpng.lib" torchvision


Suggested change

cp "$bin_path/Library/lib/libpng.lib" torchvision

cp "$bin_path/Library/bin/libpng16.dll" torchvision

* Add libpng requirement into conda recipe * Try to install libjpeg-turbo * Add PNG reading capabilities * Remove newline * Add image extension to compilation instructions * Include png functions as part of the main library * Update CMakeLists * Detect if building on conda-build * Debug * More debug messages * Print globbed libreries * Print globbed libreries * Point to correct PNG path * Remove libJPEG preventively * Debug extension loading * Link libpng explicitly * Link with PNG * Add PNG reading capabilities * Add libpng requirement into conda recipe * Try to install libjpeg-turbo * Remove newline * Add image extension to compilation instructions * Include png functions as part of the main library * Update CMakeLists * Detect if building on conda-build * Debug * More debug messages * Print globbed libreries * Print globbed libreries * Point to correct PNG path * Remove libJPEG preventively * Debug extension loading * Link libpng explicitly * Link with PNG * Install libpng on conda-based wheel distributions * Add -y flag * Add -y flag to yum * Locate LibPNG on windows conda * Remove empty else * Copy libpng16.so * Copy dylib on Mac * Improve check on Windows * Try to install ninja using conda on windows * Use libpng on Windows * Package lib on windows wheel * Point library to the correct place * Include binaries as part of wheel * Copy libpng.so on linux * Look for png.h on Windows when using conda-build * Do not skip png tests on Mac/Win * Restore libjpeg-turbo * Install jpeg-turbo on wheel distributions * Install libjpeg-turbo from conda-forge on wheel distributions * Do not pull av on conda-build * Add pillow disclaimer * Vendors libjpeg-turbo 2.0.4 * Merge JPEG work * Remove submodules * Regenerate circle config * Fix style issues * Fix C++ style issues * More style corrections * Add JPEG-turbo to linking libraries * More style corrections * More style corrections * More style corrections * Install libjpeg-turbo-devel * Install libturbo-jpeg on typing pipeline * Update Circle template * Windows and Unix turbojpeg have the same linking name * Install turbojpeg-devel instead of libjpeg-turbo * Copy TurboJPEG binaries to wheel * Move test image * Move back test image * Update JPEG test path * Remove dot from extension * Move image functions to extension * Use stdout arg in subprocess * Disable image extension if libpng or turbojpeg are not found * Append libpng stdout * Prevent list appending on lists * Minor path correction * Minor error correction * Add linking flags * Style issues correction * Address minor review corrections * Refactor library search * Restore access index * Fix JPEG tests * Update libpng version in Travis * Add -y flag * Remove dot * Update libpng using apt * Check libpng version * Change libturbojpeg binary * Update import * Change call * Restore av in conda recipe * Minor error correction * Remove unused comment in travis.yml * Update README * Fix missing links * Remove fixes for 16.04 * Remove JPEG-related code * Remove installation references to turbojpeg * Remove further references to turbojpeg * Fix c++ style issues * Fix c++ style issues * Fix libpng-config include flag parsing * Remove conda-forge * Remove include dirs from main extension * Do not pass extra include and library paths to main torchvision extension * Add libpng to environment.yml * Remove inexistent imports * Add instructions regarding environment variables to README Co-authored-by: Ryad ZENINE <r.zenine@gmail.com>

This reverts commit c1a99b7.

andfoy and others added 30 commits June 8, 2020 15:31

Add libpng requirement into conda recipe

2b78943

Try to install libjpeg-turbo

23255aa

Add PNG reading capabilities

006ab0c

Remove newline

7b9ec24

Add image extension to compilation instructions

f97a9f0

Include png functions as part of the main library

ac6d26e

Update CMakeLists

b14912e

Detect if building on conda-build

770cea5

Debug

0861b80

More debug messages

a42a029

Print globbed libreries

b7a19ea

Print globbed libreries

1afde4d

Point to correct PNG path

386fd5b

Remove libJPEG preventively

2b5c469

Debug extension loading

0341aa5

Link libpng explicitly

721e5e3

Link with PNG

2186d68

Add libpng requirement into conda recipe

b80fb08

Try to install libjpeg-turbo

3d153f0

Add PNG reading capabilities

36b0a8f

Remove newline

9d14d9e

Add image extension to compilation instructions

852a289

Include png functions as part of the main library

3e86f49

Update CMakeLists

021e767

Detect if building on conda-build

e734175

Debug

58c6524

More debug messages

b9295c1

Print globbed libreries

02fa9d9

Print globbed libreries

6c757d4

Point to correct PNG path

c207eab

Fix c++ style issues

0fc3135

fmassa reviewed Jul 1, 2020

View reviewed changes

packaging/torchvision/conda_build_config.yaml Outdated Show resolved Hide resolved

andfoy added 2 commits July 1, 2020 12:18

Fix libpng-config include flag parsing

ce92995

Remove conda-forge

2614a9c

fmassa reviewed Jul 1, 2020

View reviewed changes

andfoy added 4 commits July 1, 2020 12:22

Remove include dirs from main extension

bf41911

Do not pass extra include and library paths to main torchvision exten…

eb23830

…sion

Add libpng to environment.yml

c41800c

Remove inexistent imports

b1ff783

Add instructions regarding environment variables to README

84dad44

fmassa reviewed Jul 2, 2020

View reviewed changes

fmassa approved these changes Jul 2, 2020

View reviewed changes

fmassa merged commit c1a99b7 into pytorch:master Jul 2, 2020

fmassa mentioned this pull request Jul 2, 2020

Add PNG reading support #2382

Merged

fmassa added a commit that referenced this pull request Jul 2, 2020

Revert "PR: Enable libPNG support (#2379)"

44e762c

This reverts commit c1a99b7.

fmassa mentioned this pull request Jul 2, 2020

Revert "PR: Enable libPNG support" #2383

Merged

fmassa added a commit that referenced this pull request Jul 2, 2020

Revert "PR: Enable libPNG support (#2379)" (#2383)

e4b9823

This reverts commit c1a99b7.

peterjc123 reviewed Jul 2, 2020

View reviewed changes

de-vri-es pushed a commit to fizyr-forks/torchvision that referenced this pull request Aug 4, 2020

Revert "PR: Enable libPNG support (pytorch#2379)" (pytorch#2383)

e1a9c3f

This reverts commit c1a99b7.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR: Enable libPNG support #2379

PR: Enable libPNG support #2379

andfoy commented Jul 1, 2020 •

edited

Loading

fmassa Jul 1, 2020

fmassa Jul 1, 2020

andfoy Jul 1, 2020

codecov bot commented Jul 1, 2020 •

edited

Loading

fmassa Jul 2, 2020

fmassa left a comment

peterjc123 Jul 2, 2020

peterjc123 Jul 2, 2020

peterjc123 Jul 2, 2020 •

edited

Loading

peterjc123 Jul 2, 2020 •

edited

Loading

peterjc123 Jul 2, 2020 •

edited

Loading

peterjc123 Jul 2, 2020 •

edited

Loading

peterjc123 Jul 2, 2020

peterjc123 Jul 2, 2020

peterjc123 Jul 2, 2020

peterjc123 Jul 2, 2020

peterjc123 Jul 2, 2020

peterjc123 commented Jul 2, 2020

peterjc123 Jul 2, 2020 •

edited

Loading

	libpng = distutils.spawn.find_executable('libpng-config')
	libpng = distutils.spawn.find_executable('libpng-config')
	pngfix = distutils.spawn.find_executable('pngfix')

-    if png_found:
+if png_found:
+    if libpng is not None:
+        # Unix / Mac
+    else:
+        # Windows
+        png_lib = os.path.join(os.path.dirname(os.path.dirname(pngfix)), 'lib')
+        png_include = os.path.join(os.path.dirname(os.path.dirname(pngfix)), 'include', 'libpng16')

	png_found = libpng is not None
	png_found = libpng is not None or pngfix is not None

	package_name: ['.lib', '.dylib', '*.so']
	package_name: ['.dll', '.dylib', '*.so']

	cp "$bin_path/Library/lib/libpng.lib" torchvision
	cp "$bin_path/Library/bin/libpng16.dll" torchvision

PR: Enable libPNG support #2379

PR: Enable libPNG support #2379

Conversation

andfoy commented Jul 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 1, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

fmassa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterjc123 Jul 2, 2020 • edited Loading

Choose a reason for hiding this comment

peterjc123 Jul 2, 2020 • edited Loading

Choose a reason for hiding this comment

peterjc123 Jul 2, 2020 • edited Loading

Choose a reason for hiding this comment

peterjc123 Jul 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterjc123 commented Jul 2, 2020

peterjc123 Jul 2, 2020 • edited Loading

Choose a reason for hiding this comment

andfoy commented Jul 1, 2020 •

edited

Loading

codecov bot commented Jul 1, 2020 •

edited

Loading

peterjc123 Jul 2, 2020 •

edited

Loading

peterjc123 Jul 2, 2020 •

edited

Loading

peterjc123 Jul 2, 2020 •

edited

Loading

peterjc123 Jul 2, 2020 •

edited

Loading

peterjc123 Jul 2, 2020 •

edited

Loading