sharp icon indicating copy to clipboard operation
sharp copied to clipboard

keepIccProfile doesn't retain profile on cmyk image

Open martinj opened this issue 6 months ago • 7 comments

Possible bug

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

  • [x] Running npm install sharp completes without error.
  • [x] 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?

  • [x] 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: 16.30 GB / 96.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.10.0 - ~/.nvm/versions/node/v20.10.0/bin/node
    npm: 10.2.3 - ~/.nvm/versions/node/v20.10.0/bin/npm
  npmPackages:
    sharp: ^0.33.1 => 0.33.1 

What are the steps to reproduce?

Load an cmyk image with embedded icc profile. Try to do anything with it using keepIccProfile and no profile is in the output. What I'm actually want to achieve is to resize an image with cmyk where output is cmyk and icc profile is retained.

What is the expected behaviour?

That it retains the icc profile and output color space is cmyk.

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

const sharp = require('sharp');
sharp('Channel_digital_image_CMYK_color.jpg')
	.keepIccProfile()
        .toColorspace('cmyk')
	.toBuffer()
	.then((data) => sharp(data).metadata())
	.then((metadata) => console.log('ICC', metadata.icc))
	.catch((err) => console.error(err));

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

https://upload.wikimedia.org/wikipedia/en/2/25/Channel_digital_image_CMYK_color.jpg

martinj avatar Dec 19 '23 13:12 martinj

Have you tried setting the pipeline colourspace to ensure the image remains in CMYK throughout?

 .pipelineColourspace('cmyk')

https://sharp.pixelplumbing.com/api-colour#pipelinecolourspace

lovell avatar Dec 19 '23 14:12 lovell

Yes, tried several combinations both with pipelineColorspace and keepMetadata as well.. but it always seems to comeout as srgb and without the icc profile.

martinj avatar Dec 19 '23 14:12 martinj

When using the keepMetadata method, similar situations are also encountered

// test code
const sharp = require("sharp");
(async () => {
  console.log('origin metadata', await sharp("./cmyk.jpg").metadata());
  console.log('new metadata use toColorspace', await sharp(await sharp("./cmyk.jpg").toColorspace("cmyk").keepMetadata().toBuffer()).metadata());
  console.log('new metadata use pipelineColorspace', await sharp(await sharp("./cmyk.jpg").pipelineColorspace("cmyk").keepMetadata().toBuffer()).metadata());
})();

``

```js
origin metadata {
  format: 'jpeg',
  width: 500,
  height: 333,
  space: 'cmyk',
  channels: 4,
  depth: 'uchar',
  density: 180,
  chromaSubsampling: '4:4:4:4',
  isProgressive: false,
  resolutionUnit: 'inch',
  hasProfile: true,
  hasAlpha: false,
  orientation: 1,
  exif: <Buffer 45 78 69 66 00 00 49 49 2a 00 08 00 00 00 0a 00 0f 01 02 00 06 00 00 00 86 00 00 00 10 01 02 00 17 00 00 00 8c 00 00 00 12 01 03 00 01 00 00 00 01 00 ... 9492 more bytes>,
  icc: <Buffer 00 08 80 70 41 44 42 45 02 10 00 00 70 72 74 72 43 4d 59 4b 4c 61 62 20 07 d0 00 07 00 1a 00 05 00 29 00 35 61 63 73 70 41 50 50 4c 00 00 00 00 41 44 ... 557118 more bytes>,
  iptc: <Buffer 50 68 6f 74 6f 73 68 6f 70 20 33 2e 30 00 38 42 49 4d 04 25 00 00 00 00 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 38 42 49 4d 03 ed 00 00 ... 9948 more bytes>,
  xmp: <Buffer 3c 3f 78 70 61 63 6b 65 74 20 62 65 67 69 6e 3d 22 ef bb bf 22 20 69 64 3d 22 57 35 4d 30 4d 70 43 65 68 69 48 7a 72 65 53 7a 4e 54 63 7a 6b 63 39 64 ... 16899 more bytes>
}
new metadata use toColorspace {
  format: 'jpeg',
  size: 93328,
  width: 500,
  height: 333,
  space: 'srgb',
  channels: 3,
  depth: 'uchar',
  density: 180,
  chromaSubsampling: '4:2:0',
  isProgressive: false,
  resolutionUnit: 'inch',
  hasProfile: false,
  hasAlpha: false,
  orientation: 1,
  exif: <Buffer 45 78 69 66 00 00 49 49 2a 00 08 00 00 00 0a 00 0f 01 02 00 06 00 00 00 86 00 00 00 10 01 02 00 17 00 00 00 8c 00 00 00 12 01 03 00 01 00 00 00 01 00 ... 9490 more bytes>,
  iptc: <Buffer 50 68 6f 74 6f 73 68 6f 70 20 33 2e 30 00 38 42 49 4d 04 25 00 00 00 00 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 38 42 49 4d 03 ed 00 00 ... 9948 more bytes>,
  xmp: <Buffer 3c 3f 78 70 61 63 6b 65 74 20 62 65 67 69 6e 3d 22 ef bb bf 22 20 69 64 3d 22 57 35 4d 30 4d 70 43 65 68 69 48 7a 72 65 53 7a 4e 54 63 7a 6b 63 39 64 ... 16899 more bytes>
}
new metadata use pipelineColorspace {
  format: 'jpeg',
  size: 92010,
  width: 500,
  height: 333,
  space: 'srgb',
  channels: 3,
  depth: 'uchar',
  density: 180,
  chromaSubsampling: '4:2:0',
  isProgressive: false,
  resolutionUnit: 'inch',
  hasProfile: false,
  hasAlpha: false,
  orientation: 1,
  exif: <Buffer 45 78 69 66 00 00 49 49 2a 00 08 00 00 00 0a 00 0f 01 02 00 06 00 00 00 86 00 00 00 10 01 02 00 17 00 00 00 8c 00 00 00 12 01 03 00 01 00 00 00 01 00 ... 9490 more bytes>,
  iptc: <Buffer 50 68 6f 74 6f 73 68 6f 70 20 33 2e 30 00 38 42 49 4d 04 25 00 00 00 00 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 38 42 49 4d 03 ed 00 00 ... 9948 more bytes>,
  xmp: <Buffer 3c 3f 78 70 61 63 6b 65 74 20 62 65 67 69 6e 3d 22 ef bb bf 22 20 69 64 3d 22 57 35 4d 30 4d 70 43 65 68 69 48 7a 72 65 53 7a 4e 54 63 7a 6b 63 39 64 ... 16899 more bytes>
}

the space is not cmyk

rambo-panda avatar Dec 20 '23 10:12 rambo-panda

Do we have any update on this. I am also facing kind of similiar issue. Where we have an image which is cmyk and when I am trying to process it using sharp it is changing the color space. And my image is not having the icc profile in it.

aamirsdev avatar Jan 17 '24 14:01 aamirsdev

The test keep existing ICC profile seems to test for a Generic RGB Profile ICC profile in the output file.

Confirmed that the current implementation of keepIccProfile won't work for CMYK profiles with following test:

it('keep existing CMYK ICC profile', async () => {
  const data = await sharp(fixtures.inputJpgWithCmykProfile)
    .keepIccProfile()
    .toBuffer();

  const metadata = await sharp(data).metadata();
  const { description } = icc.parse(metadata.icc);
  assert.strictEqual(description, 'U.S. Web Coated (SWOP) v2');
});

Taking a look at the pipeline, it seems this fails because it converts to sRGB/P3 using the embedded profile: https://github.com/lovell/sharp/blob/26d0b7147d874d497aa5500debce5df2804ffb6a/src/pipeline.cc#L330-L352

Updating that code to the following fixes the issue:

// Convert to sRGB/P3 using embedded profile
try {
  if ((baton->keepMetadata & VIPS_FOREIGN_KEEP_ICC) && baton->withIccProfile.empty()) {
    // New implementation
    if (image.interpretation() == VIPS_INTERPRETATION_CMYK) {
      baton->colourspace = VIPS_INTERPRETATION_CMYK;
    }

    image = image.icc_transform(inputProfile.first, VImage::option()
      ->set("input_profile", inputProfile.first)
      ->set("embedded", TRUE)
      ->set("depth", sharp::Is16Bit(image.interpretation()) ? 16 : 8)
      ->set("intent", VIPS_INTENT_PERCEPTUAL));

  } else {
    // Current implementation
    image = image.icc_transform(processingProfile, VImage::option()
      ->set("embedded", TRUE)
      ->set("depth", sharp::Is16Bit(image.interpretation()) ? 16 : 8)
      ->set("intent", VIPS_INTENT_PERCEPTUAL));
  }
} catch(...) {
  sharp::VipsWarningCallback(nullptr, G_LOG_LEVEL_WARNING, "Invalid embedded profile", nullptr);
}

@lovell All tests, including the new one for preserving a CMYK profile, pass successfully. I am uncertain if this adjustment is located in the most suitable part of the codebase though, as I have a limited understanding of C and the specific workings of ICC tranformations in the pipeline.

I'm willing to work on a preliminary PR if that helps facilitate a more detailed review and discussion on how best to integrate these changes.

adriaanmeuris avatar Feb 05 '24 12:02 adriaanmeuris

The change in commit https://github.com/lovell/sharp/commit/fb70fbb09ff76440124b1fad567c39bd2997cc99 prevents the erroneous sRGB transformation when using CMYK everywhere, plus adds a slightly-modified version of @adriaanmeuris' example test case to help prevent regression. Thanks all for reporting and helping analyse this issue.

lovell avatar Feb 11 '24 20:02 lovell

@lovell thank you so much for the recent update; I noticed that the profile is correcly attached now.

However, it seems this icc transform to sRGB is always happening: https://github.com/lovell/sharp/blob/60f4048d6c33f414979169c660ce3bad69a395a6/src/pipeline.cc#L330-L344

Which results in a color shift. Commenting out the icc_transform solves this. Here's a comparison visual to illustrate the difference:

comparison

Adding this condition to the if statement instead of commenting out the icc_transform also fixed this, but it does result in the test tints cmyk image red to fail.

image.interpretation() != VIPS_INTERPRETATION_CMYK &&

Additionally, I found that to maintain the color profile, it's essential to use .pipelineColourspace('cmyk') and .toColourspace('cmyk'). I noticed that using await sharp(inFile).keepIccProfile().toFile(outFile); still results in a conversion to sRGB, despite the keepIccProfile option being set. Is this the expected behavior?

I'm happy to help out or provide additional information if needed.

adriaanmeuris avatar Feb 12 '24 16:02 adriaanmeuris

@adriaanmeuris Are you able to test to see if the following patch fixes the problem you're seeing?

--- a/src/pipeline.cc
+++ b/src/pipeline.cc
@@ -331,6 +331,7 @@ class PipelineWorker : public Napi::AsyncWorker {
         sharp::HasProfile(image) &&
         image.interpretation() != VIPS_INTERPRETATION_LABS &&
         image.interpretation() != VIPS_INTERPRETATION_GREY16 &&
+        baton->colourspaceInput != VIPS_INTERPRETATION_CMYK &&
         !baton->input->ignoreIcc
       ) {
         // Convert to sRGB/P3 using embedded profile

(I need to refactor some of the variable naming in here as colourspaceInput should really be called something like colourspacePipeline instead.)

lovell avatar Mar 06 '24 14:03 lovell

@lovell Yes, the patch works as expected. The profile is attached, and the output colors now align with the input colors. Looks great!

adriaanmeuris avatar Mar 06 '24 15:03 adriaanmeuris

@adriaanmeuris Great, thanks for confirming, added via commit https://github.com/lovell/sharp/commit/3eeaee71c06c38d0d9e4d2abcb6b04397febe0b8

lovell avatar Mar 06 '24 21:03 lovell

v0.33.3 now available, thanks everyone for your help fixing/testing this.

lovell avatar Mar 23 '24 12:03 lovell