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

Enhancement: silently ignore embedded ICC profile with invalid PCS illuminant, will downgrade "Couldn't link the profiles" from error to warning #3895

Closed
3 tasks done
martinj opened this issue Dec 14, 2023 · 5 comments

Comments

@martinj
Copy link

martinj commented Dec 14, 2023

Possible bug

Is this a possible bug in a feature of sharp, unrelated to installation?

  • Running npm install sharp completes without error.
  • Running node -e "require('sharp')" completes without error.

If you cannot confirm both of these, please open an installation issue instead.

Are you using the latest version of sharp?

  • I am using the latest version of sharp as reported by npm view sharp dist-tags.latest.

If you cannot confirm this, please upgrade to the latest version and try again before opening an issue.

If you are using another package which depends on a version of sharp that is not the latest, please open an issue against that package instead.

What is the output of running npx envinfo --binaries --system --npmPackages=sharp --npmGlobalPackages=sharp?

  System:
    OS: macOS 13.4.1
    CPU: (12) arm64 Apple M2 Max
    Memory: 1.86 GB / 96.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.10.0 - ~/.nvm/versions/node/v20.10.0/bin/node
  npmPackages:
    sharp: ^0.33.0 => 0.33.0

What are the steps to reproduce?

Try to do any conversion using withMetadata() on the image will result in VipsIcc: Couldn't link the profiles error

I've tracked the issue down to 0.32.5, versions below works fine.

What is the expected behaviour?

That it succeeds without error

Please provide a minimal, standalone code sample, without other dependencies, that demonstrates this problem

const sharp = require('sharp');
sharp('1.jpg').withMetadata().toFile('out.jpg').catch(console.error);

Please provide sample image(s) that help explain this problem

1.jpg

@martinj martinj added the triage label Dec 14, 2023
@lovell lovell added question and removed triage labels Dec 14, 2023
@lovell
Copy link
Owner

lovell commented Dec 14, 2023

The ICC profile embedded in this image is invalid as the PCS illuminant does not have the standard/required value of 50. Inspecting the binary data suggests the value might be 0xd32c, which doesn't look right at all. The description is "AppleVision 1710 - 9300", which suggests this profile is probably around 25 years old.

I suspect a recent improvement in the underlying lcms2 is preventing the use of this profile, and you can use the ignoreIcc constructor option to do just that.

sharp('1.jpg', { ignoreIcc: true })...

@martinj
Copy link
Author

martinj commented Dec 14, 2023

Since I discovered this while using a global custom build of vips I initially thought this was related to the version of lcms2 I used, however using same global vips build for sharp 0.32.4 it works, which make me think the change seems to come from sharp.

Did not know of ignoreIcc guess i could use that to work around it. Thank you.

@lovell
Copy link
Owner

lovell commented Dec 14, 2023

Thanks, this probably relates to commit bb7469b

Perhaps sharp should silently ignore the invalid profile, although the output might then be incorrect. Are you able to grant permission for me to add this image to sharp's test corpus if so?

If you're using v0.33.0, you might want to use the new withIccProfile() instead of withMetadata() for more fine-grained control over output profile.

https://sharp.pixelplumbing.com/api-output#withiccprofile

@martinj
Copy link
Author

martinj commented Dec 14, 2023

Perhaps sharp should silently ignore the invalid profile, although the output might then be incorrect

In my case that would be great!

Are you able to grant permission for me to add this image to sharp's test corpus if so?

That specific image i don't own the rights to. However I've produced a similar image with the same issue which you are free to use

broken-icc

If you're using v0.33.0, you might want to use the new withIccProfile() instead of withMetadata() for more fine-grained control over output profile.

Thank you for pointing this out, thats a great addition.

@lovell lovell changed the title VipsIcc: Couldn't link the profiles error in version 0.32.5+ Enhancement: silently ignore embedded ICC profile with invalid PCS illuminant, will downgrade "Couldn't link the profiles" from error to warning Dec 14, 2023
@lovell lovell added this to the v0.33.1 milestone Dec 15, 2023
@lovell
Copy link
Owner

lovell commented Dec 17, 2023

v0.33.1 now available with this enhancement.

@lovell lovell closed this as completed Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants