Support specifying output channels in io.image.read_image by datumbox · Pull Request #2988 · pytorch/vision

datumbox · 2020-11-11T12:05:16Z

I add a channels parameter that allows the users to specify the number of output channels while reading an image. The default value is 0 which leaves the image as-is and ensures the change is BC. The following public API methods of the torchvision.io.image package were updated:

decode_png(input, channels=0)
decode_jpeg(input, channels=0)
decode_image(input, channels=0)
read_image(path, channels=0)

There is a small update on the originally proposed pitch because I added support for grayscale transparency and handling for palette images. Here are the supported values:

channels=0 - leave as original (grayscale, palette, grayscale with alpha, rgb, rgb with alpha, CMYK etc)
channels=1 - Grayscale
channels=2 - Grayscale with Alpha (PNG only, not valid for JPEG)
channels=3 - RGB
channels=4 - RGB with Alpha (PNG only, not valid for JPEG)

The PR adds 3 JPEG assets with total size 7kb. These are used to test the supported conversions. It also removes a 900kb asset file which is no longer needed. The assets were produced using the following snippet:

from PIL import Image

# manually downloaded from https://pytorch.org/assets/images/pytorch-logo.png
original = 'pytorch-logo.png'
with Image.open(original) as img:
    img = img.convert("RGBA")
    img = img.resize((100, 100)) 
    img.convert("L").save("gray_pytorch.jpg")
    img.convert("RGB").save("rgb_pytorch.jpg")
    img.convert("CMYK").save("cmyk_pytorch.jpg")

…e from assets and reduce duplicate code. Moving jpeg assets used by encode and write unit-tests on their separate folders.

…and adding checks for inputs.

datumbox

I left a few comments to explain parts of the implementation.`

test/test_cpp_models.py

test/test_image.py

torchvision/io/image.py

test/test_image.py

torchvision/csrc/cpu/image/readpng_cpu.cpp

torchvision/io/image.py

test/test_image.py

datumbox · 2020-11-12T10:20:06Z

The failing tests on Travis are not related to this PR. I would like to rebase to master once #2985 is merged to ensure all tests still pass.

codecov · 2020-11-12T12:24:18Z

Codecov Report

Merging #2988 (161dbce) into master (80f41f8) will decrease coverage by 0.02%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master    #2988      +/-   ##
==========================================
- Coverage   73.39%   73.37%   -0.03%     
==========================================
  Files          99       99              
  Lines        8825     8825              
  Branches     1391     1391              
==========================================
- Hits         6477     6475       -2     
- Misses       1929     1931       +2     
  Partials      419      419

Impacted Files	Coverage Δ
torchvision/io/image.py	`79.03% <75.00%> (-3.23%)`	⬇️

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 80f41f8...ada56a2. Read the comment docs.

fmassa

Thanks a lot for the PR, it looks great!

I would really prefer if we could unify the name of the arguments across jpeg / png, this way it is clear to the user that they both represent the same thing.

About channels=1 keeping the image as a palette type, although it makes sense I wonder if the expected behavior would be instead to convert it to a grayscale. Thoughts?

I've made a few other comments on the PR, let me know what you think

test/test_cpp_models.py

test/test_image.py

torchvision/csrc/cpu/image/readjpeg_cpu.cpp

torchvision/csrc/cpu/image/readpng_cpu.cpp

test/test_image.py

andfoy · 2020-11-13T14:58:22Z

The usage of pth files is due to the difference between libjpeg and libjpeg-turbo on Windows and Mac, which right now we are not able to use.

datumbox · 2020-11-13T15:25:57Z

@fmassa Thanks for the review. I marked as resolved anything that I either accept your proposal or is already covered discussed. I kept open anything that requires a second look. I'll send now another commit with the changes. I would appreciate to review the last remaining points.

@andfoy Thanks for providing the background story.

…fixing variable name etc.

torchvision/csrc/cpu/image/readpng_cpu.cpp

fmassa

Thanks a lot Vasilis!

* Adding output channels implementation for pngs. * Adding tests for png. * Adding channels in the API and documentation. * Fixing formatting. * Refactoring test_image.py to remove huge grace_hopper_517x606.pth file from assets and reduce duplicate code. Moving jpeg assets used by encode and write unit-tests on their separate folders. * Adding output channels implementation for jpegs. Fix asset locations. * Add tests for JPEG, adding the channels in the API and documentation and adding checks for inputs. * Changing folder for unit-test. * Fixing windows flakiness, removing duplicate test. * Replacing components to channels. * Adding reference for supporting CMYK. * Minor changes: num_components to output_components, adding comments, fixing variable name etc. * Reverting output_components to num_components. * Replacing decoding with generic method on tests. * Palette converted to Gray.

Summary: This image was moved to `test/assets/encode_jpeg` in #2988 but was not removed in this branch for some reason Pull Request resolved: #3139 Reviewed By: datumbox Differential Revision: D25395596 Pulled By: fmassa fbshipit-source-id: a0afdec2d1da41e6743d7d723e71ffde442cf3a7

datumbox added 4 commits November 11, 2020 10:27

Adding output channels implementation for pngs.

d3ea66f

Adding tests for png.

d5871a7

Adding channels in the API and documentation.

cac58ac

Fixing formatting.

c579341

facebook-github-bot added the cla signed label Nov 11, 2020

datumbox added 4 commits November 11, 2020 16:03

Refactoring test_image.py to remove huge grace_hopper_517x606.pth fil…

115338c

…e from assets and reduce duplicate code. Moving jpeg assets used by encode and write unit-tests on their separate folders.

Adding output channels implementation for jpegs. Fix asset locations.

b3d69fa

Add tests for JPEG, adding the channels in the API and documentation …

f44f26e

…and adding checks for inputs.

Changing folder for unit-test.

7ea166d

datumbox commented Nov 11, 2020

View reviewed changes

Fixing windows flakiness, removing duplicate test.

e643a75

datumbox commented Nov 11, 2020

View reviewed changes

test/test_image.py Show resolved Hide resolved

datumbox commented Nov 11, 2020

View reviewed changes

test/test_image.py Show resolved Hide resolved

datumbox changed the title ~~[WIP] Support specifying output channels in io.image.read_image~~ Support specifying output channels in io.image.read_image Nov 12, 2020

datumbox requested a review from fmassa November 12, 2020 10:20

Merge branch 'master' into feature/channels_in_read_image

ed59745

fmassa reviewed Nov 13, 2020

View reviewed changes

Replacing components to channels.

f221cf3

datumbox mentioned this pull request Nov 13, 2020

Refactor tests in test_image.py to avoid writes inside assets #3002

Closed

datumbox added 5 commits November 13, 2020 15:46

Adding reference for supporting CMYK.

110009a

Minor changes: num_components to output_components, adding comments, …

c72f861

…fixing variable name etc.

Reverting output_components to num_components.

161dbce

Replacing decoding with generic method on tests.

c81548c

Palette converted to Gray.

ada56a2

fmassa reviewed Nov 18, 2020

View reviewed changes

torchvision/csrc/cpu/image/readpng_cpu.cpp Show resolved Hide resolved

fmassa approved these changes Nov 18, 2020

View reviewed changes

fmassa merged commit 4d6ba67 into pytorch:master Nov 18, 2020

datumbox deleted the feature/channels_in_read_image branch November 18, 2020 11:50

This was referenced Nov 18, 2020

Remove hardcoded PNG_FOUND define. #3020

Merged

Improved format conversion in io.image.read_image #3021

Closed

datumbox mentioned this pull request Dec 1, 2020

Check num of channels on adjust_* transformations #3069

Merged

fmassa mentioned this pull request Dec 8, 2020

Remove not used image #3139

Closed

datumbox mentioned this pull request Jan 5, 2021

TorchVision Roadmap - 2021 H1 #3221

Closed

13 tasks

kairos03 mentioned this pull request Feb 1, 2021

torchvision.io.read_image return tensor shape is different. #3332

Closed

datumbox mentioned this pull request Feb 8, 2021

Added utility to draw segmentation masks #3330

Merged

3 tasks

Conversation

datumbox commented Nov 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

datumbox left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

datumbox commented Nov 12, 2020

Uh oh!

codecov bot commented Nov 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fmassa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andfoy commented Nov 13, 2020

Uh oh!

datumbox commented Nov 13, 2020

Uh oh!

Uh oh!

fmassa left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

datumbox commented Nov 11, 2020 •

edited

Loading

codecov bot commented Nov 12, 2020 •

edited

Loading