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

Resized Display P3 image includes Display P3 color profile but it is missing color information #2862

Closed
AttilaTheFun opened this issue Aug 27, 2021 · 12 comments

Comments

@AttilaTheFun
Copy link

AttilaTheFun commented Aug 27, 2021

I'm working on adding wide color support to my application.

I've got my pipeline from device -> server -> Amazon S3 working and I can successfully upload images with the Display P3 color profile from device to S3 without a loss of color information, retaining the color profile. I use libvips on my Go server as well, in order to do some initial processing.

Once the images are stored in S3, I have Sharp running in a Lambda function for Cloudfront origin requests to the S3 bucket. I have verified that a vanilla Cloudfront distribution is able to serve the wide color images correctly, but when they pass through Sharp resizing they lose color information.

I'm on the latest version, v0.29, and I'm using the attached canary image to test (it is an image that shows up in Display P3 but disappears in sRGB).

Original Canary Broken by Sharp

If you download and inspect the images, you can see that the resulting image has the Display P3 profile but it has lost the color information.

I'm using the pipelineColorspace("rgb16") operator, and specifying the output icc profile. Is there something else I'm doing wrong here? Thanks!

Relevant code:

  S3.getObject({ Bucket: bucket, Key: originalKey })
    .promise()
    .then((data) =>
      Sharp(data.Body)
        .pipelineColorspace("rgb16")
        .resize(width, height)
        .withMetadata({ icc: iccProfilePath })
        .toColorspace("srgb") // I have tried with and without this line to the same result.
        .toFormat(format)
        .toBuffer()
    )
    .then((buffer) =>
      S3.putObject({
        Body: buffer,
        Bucket: bucket,
        ContentType: contentType,
        Key: newKey,
      }).promise()
    )
    .then(function () {
      console.log("Redirecting to resized at: " + `/${newKey}`);
      const response = {
        status: "301",
        statusDescription: "Moved Permanently",
        headers: {
          location: [
            {
              key: "Location",
              value: `/${newKey}`,
            },
          ],
        },
      };
      callback(null, response);
    })
    .catch((err) => callback(err));
@lovell
Copy link
Owner

lovell commented Aug 27, 2021

Hi, it looks like GitHub has removed the ICC profiles from these images. Are you able to upload/provide the images via another service?

Since libvips/libvips#2190 libvips v8.11.3 (and therefore sharp v0.29.0) allows p3 as a built-in, named profile, so you should be able to use the following:

  .pipelineColorspace("rgb16")
  .withMetadata({ icc: "p3" })

@AttilaTheFun
Copy link
Author

That's weird - when I download the attached images from GitHub I still see the color profiles:
Screen Shot 2021-08-27 at 2 44 40 PM
Screen Shot 2021-08-27 at 2 45 19 PM

Either way, I'll try using the built-in p3 profile. Just to be clear, is that the "Display P3" profile, or the "DCI-P3" profile? My understanding is that they have different white points and I'm trying to use the former for wide color images.

@AttilaTheFun
Copy link
Author

Hi @lovell, I tried your suggestion of using the built-in p3 profile and still got the same result. Here is the image with the sP3C profile attached, uploaded to imgur: https://i.imgur.com/LYriNZI.png

The relevant code is:

      Sharp(data.Body)
        .pipelineColorspace("rgb16")
        .resize(width, height)
        .withMetadata({ icc: iccProfile })
        .toFormat(format)
        .toBuffer()

Where iccProfile is "p3".

For reference, this is the source image loaded from S3 into data.Body in the Display P3 color space, also uploaded to imgur https://i.imgur.com/Fa97cRx.png.

@lovell
Copy link
Owner

lovell commented Aug 28, 2021

Thanks, and sorry you were correct, the P3 profile is present in the GitHub image (I was looking at a previously downloaded image from a different issue with a very similar filename, d'oh).

I can reproduce the problem locally, and will need to spend a little time to debug exactly what's happening.

In terms of the built-in "p3" profile in libvips, this is DisplayP3, which is hopefully the more useful/common flavour.

@AttilaTheFun
Copy link
Author

Sounds good, thanks for taking a look!

I know libvips is capable of applying these profiles correctly because, on upload, I use libvips from Go to apply the Display P3 and sRGB profiles to the uploaded images before storing them in S3.

I'm using the vips_icc_transform API to go from their embedded icc profiles to one of the profiles I supply. (I normalize wide-color images from Adobe RGB, ProPhoto etc to Display P3, and RGB images to sRGB IEC61966-2.1 to ensure maximum compatibility.)

@lovell
Copy link
Owner

lovell commented Aug 30, 2021

Commit 104464c fixes this and adds a test that would previously have failed. This will be in v0.29.1 - thanks for reporting!

@AttilaTheFun
Copy link
Author

Awesome! Thanks so much! I'll test out the fix when I have some time.

@AttilaTheFun
Copy link
Author

Hi @lovell - I finally had a chance to pull this change and try it out.
Unfortunately, now I'm getting a different, weird error for both sRGB and P3 images:

"errorType": "Error",
    "errorMessage": "A number was expected",
    "stack": [
        "Error: A number was expected",
        "    at /var/task/node_modules/sharp/lib/output.js:1008:15",
        "    at new Promise (<anonymous>)",
        "    at Sharp._pipeline (/var/task/node_modules/sharp/lib/output.js:1007:14)",
        "    at Sharp.toBuffer (/var/task/node_modules/sharp/lib/output.js:136:15)",
        "    at /var/task/index.js:238:10",
        "    at processTicksAndRejections (internal/process/task_queues.js:95:5)"
    ]

It's possible this is due to how I installed it - I ran:
npm install https://github.com/lovell/sharp#e044788f63b6cd1147717b181369d596b606942d
because the v0.29.1 release has not been pushed to npm yet.

Is there another way I should be installing it? Or could you push like a pre-release version of it?

The relevant code from my lambda function is:

  // Get the original object, resize it, and save it back to s3:
  S3.getObject({ Bucket: bucket, Key: originalKey })
    .promise()
    .then((data) =>
      Sharp(data.Body)
        .pipelineColorspace("rgb16")
        .resize(width, height)
        .withMetadata({ icc: iccProfile })
        .toFormat(format)
        .toBuffer()
    )
    .then((buffer) =>
      S3.putObject({
        Body: buffer,
        Bucket: bucket,
        ContentType: contentType,
        Key: newKey,
      }).promise()
    )
    .then(function () {
      console.log("Redirecting to resized at: " + `/${newKey}`);
      const response = {
        status: "301",
        statusDescription: "Moved Permanently",
        headers: {
          location: [
            {
              key: "Location",
              value: `/${newKey}`,
            },
          ],
        },
      };
      callback(null, response);
    })
    .catch((err) => callback(err));

Where iccProfile is "p3".

@lovell
Copy link
Owner

lovell commented Sep 3, 2021

Please can you try adding the --build-from-source flag:

npm install --build-from-source https://github.com/lovell/sharp#e044788f63b6cd1147717b181369d596b606942d

@AttilaTheFun
Copy link
Author

Awesome! Everything seems to be working now. Thanks for all your help!

@lovell
Copy link
Owner

lovell commented Sep 4, 2021

Thanks for checking, I'll re-open this until v0.29.1 is released with the fix.

@lovell lovell reopened this Sep 4, 2021
@lovell
Copy link
Owner

lovell commented Sep 7, 2021

v0.29.1 is now available - thank you for reporting this.

@lovell lovell closed this as completed Sep 7, 2021
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