Skip to content
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

Fixed setup.py when missing libjpeg #7840

Merged
merged 3 commits into from
Aug 17, 2023
Merged

Fixed setup.py when missing libjpeg #7840

merged 3 commits into from
Aug 17, 2023

Conversation

vfdev-5
Copy link
Collaborator

@vfdev-5 vfdev-5 commented Aug 16, 2023

This PR fixes the error when installing torchvision from source on a system without libjpeg (e.g. GH Codespaces)

/workspaces/vision/torchvision/csrc/io/image/cpu/decode_jpeg.cpp:159:10: error: 'JPEG_LIB_VERSION' was not declared in this scope; did you mean 'PSTL_VERSION'?
  159 |   return JPEG_LIB_VERSION;
      |          ^~~~~~~~~~~~~~~~
      |          PSTL_VERSION
error: command '/usr/bin/gcc' failed with exit code 1

@pytorch-bot
Copy link

pytorch-bot bot commented Aug 16, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/7840

Note: Links to docs will display an error until the docs builds have been completed.

❌ 13 New Failures, 1 Unrelated Failure

As of commit f5ca062 with merge base 69220e0 (image):

NEW FAILURES - The following jobs have failed:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @vfdev-5 , LGTM if green with a few comments. Maybe we should just merge it and completely re-write that part of the readme in another PR. We should remove mentions of accimage and also be clear that jpeg-turbo is preferred now.

`TORCHVISION_INCLUDE` and `TORCHVISION_LIBRARY`, respectively.
**Notes:** `libpng` and `libjpeg` are optional dependencies. If any of them is available on the system,
torchvision will provide encoding/decoding image functionalities from `torchvision.io.image`.
When building torchvision from source, `libpng` and `libjpeg` can be found on the standard library locations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should say that. It depends on whether the user installed them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you detail what is incorrect here?

Copy link
Member

@NicolasHug NicolasHug Aug 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't know whether those libraries can be found on the standard library locations. If users didn't install them, then they won't be found there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I think I need to rephrase the sentence I wrote: "libpng and libjpeg are first searched on the standard library locations"

**Notes:** `libpng` and `libjpeg` must be available at compilation time in order to be available. Make sure that it is
available on the standard library locations, otherwise, add the include and library paths in the environment variables
`TORCHVISION_INCLUDE` and `TORCHVISION_LIBRARY`, respectively.
**Notes:** `libpng` and `libjpeg` are optional dependencies. If any of them is available on the system,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good to clarify that libjpeg and libpng are optional, but I don't think we need to remove that sentence:

libpng and libjpeg must be available at compilation time in order to be available.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I do not understand what you ask. Libs are optional and this sentence "libpng and libjpeg must be available at compilation time in order to be available." does not make sense (without speaking that it is unclear what it is exactly "in order be available")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the sentence means

libjpeg and libpng need to be available at compile time when you build from source.

We don't need to remove it

@NicolasHug NicolasHug merged commit 4025fc5 into main Aug 17, 2023
@github-actions
Copy link

Hey @NicolasHug!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

@NicolasHug
Copy link
Member

Thanks for the fix @vfdev-5 , I'll submit a PR soon to rework our README a bit, as other parts of it also need an update

@vfdev-5 vfdev-5 deleted the fix-setup-py-libjpeg branch August 17, 2023 12:33
facebook-github-bot pushed a commit that referenced this pull request Aug 25, 2023
Summary: Co-authored-by: Nicolas Hug <nh.nicolas.hug@gmail.com>

Reviewed By: matteobettini

Differential Revision: D48642325

fbshipit-source-id: 2eac112d0e7387bf9b7a10624a397aae09e17e61
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants