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

Automatically not applying SRGB profile on webp conversion #324

Open
mevinbabuc opened this issue Feb 18, 2023 · 14 comments
Open

Automatically not applying SRGB profile on webp conversion #324

mevinbabuc opened this issue Feb 18, 2023 · 14 comments

Comments

@mevinbabuc
Copy link

mevinbabuc commented Feb 18, 2023

Few images that don't have a color profile attached with it, comes out as dull images. This is happening only with WEBP outputs. JPEG is fine.
We have temporarily switched to sharp, which is able to handle this.

I tried this with your latest release.

This is the image
https://s3-ap-southeast-1.amazonaws.com/asia-compressed-image-store/aJu2ijdgnZXpBPds-0UXUxiXcJUOqaUVh-JHUAwEiznuzXHxyy-b'QVJKMDQ5MDQtRWRpdC5qcGc='.jpg

Converting the above image to webp using imagor gives a dull output.

settings that I have:

IMAGOR_AUTO_WEBP: 1
S3_SAFE_CHARS: "'{}()="
S3_FORCE_PATH_STYLE: 1

@cshum Any pointers in the right direction would be helpful.

@mevinbabuc mevinbabuc changed the title Automatically not applying SRGB profile Automatically not applying SRGB profile on webp conversion Feb 18, 2023
@cshum
Copy link
Owner

cshum commented Feb 18, 2023

@mevinbabuc would you test the docker build under master branch?
https://github.com/cshum/imagor/pkgs/container/imagor/71546111?tag=master

ghcr.io/cshum/imagor@sha256:8c5f0ae25eea2f8fa20031721f74c8a29baabb399c48220458b4043dbde99ea1

@mevinbabuc
Copy link
Author

I see that the WebP issue is resolved. Awesome

but when explicitly setting to JPEG I face the same issue
http://localhost:9000/unsafe/filters:strip_icc():format(jpeg)/aJu2ijdgnZXpBPds-0UXUxiXcJUOqaUVh-JHUAwEiznuzXHxyy-b'QVJKMDQ5MDQtRWRpdC5qcGc='.jpg

Is it expected?
We strip ICC on production to reduce the file size

@cshum
Copy link
Owner

cshum commented Feb 18, 2023

Setting JPEG explicitly should be no problem.
Stripping ICC that causes it because it loses the colour profile afterwards, which is expected.

@mevinbabuc
Copy link
Author

We have been stripping ICC profiles in production.
You can see that this one works.

http://localhost:9000/unsafe/filters:filters:strip_icc():format(webp)/aJu2ijdgnZXpBPds-0UXUxiXcJUOqaUVh-JHUAwEiznuzXHxyy-b'QVJKMDQ5MDQtRWRpdC5qcGc='.jpg

Above, we are converting the same image to Webp, by stripping ICC

But when switching the format explicitly to JPEG, when IMAGOR_AUTO_WEBP is true
http://localhost:9000/unsafe/filters:strip_icc():format(jpeg)/aJu2ijdgnZXpBPds-0UXUxiXcJUOqaUVh-JHUAwEiznuzXHxyy-b'QVJKMDQ5MDQtRWRpdC5qcGc='.jpg
results in a dull output.

Maybe in webp, color profiles are actually not getting stripped?
Or is there a conversion between color profiles happening? (Don't know if that is even possible. )

@mevinbabuc
Copy link
Author

I just tested it and strip_icc is not working for webp.

I also found that our sharp setup was doing an explicit colorspace conversion and hence we didn't have this issue.
image

Is there a way to add similar functionality, maybe as a filter in imagor similar to this?
In this way, we can strip ICC profile at the end and reduce the download size further. This should shed 4Kb per file for us.

@cshum
Copy link
Owner

cshum commented Feb 21, 2023

I tried adding a colorspace filter but it doesn't seem to restore the color.
master...colorspace
I presume the photo is already srgb so converting to srgb means no effect.

Does you code base added some other processing under the sharp pipeline?

@mevinbabuc
Copy link
Author

mevinbabuc commented Mar 8, 2023

@cshum, this is how our sharp pipeline looks like

_sharpObject
    .rotate() // for auto-orientation
    .sharpen() // default sharpening
    .toColorspace('srgb') // default option, added for dev clarity
    .toFormat(format, formatOptions[format]) // takes in format along with custom options
    .toBuffer() // by default all metadata will be stripped here

Hope this is helpful.

@mevinbabuc
Copy link
Author

@cshum Upon looking into your commit, I'm not able to find the mapping for ProPhoto RGB color profile. The image is in ProPhoto RGB color profile and I guess that's the reason it did not get converted. The photo does get converted in sharp using the above code.

It would really help us out if you could take a second look at this, as few photographers have started to switch more into using ProPhoto RGB.

@cshum
Copy link
Owner

cshum commented May 2, 2023

Other libraries may have been swapping a more lightweight ICC profile instead of stripping it completely. Will need a more detailed look.

@mevinbabuc
Copy link
Author

Maybe @lovell who created the sharp library can shed some light on how they handle color profile conversions. As this is geared towards web usage, we would like ProRGB or AdobeRGB or any other color profiles to be explicitly converted to sRGB for web use.

@lovell
Copy link

lovell commented May 12, 2023

Use vips_icc_transform() to convert from a linear RGB input with a profile to a non-linear sRGB output (with or without a profile).

https://www.libvips.org/API/current/libvips-colour.html#vips-icc-transform

https://github.com/lovell/sharp/blob/e873978e53fd7667b44b92076d4617f96614179b/src/pipeline.cc#L308-L332

I suspect you'll also want to test with CMYK images, both with and without profiles.

@mevinbabuc
Copy link
Author

@cshum Hi, do you have the bandwidth to take a look at the suggestion by lovell ?

@cshum
Copy link
Owner

cshum commented Jul 29, 2023

@mevinbabuc I was trying to apply similar logic some time ago, but no luck so far with the said image

b8a75af#diff-e488e36196c25a401a5af66dd4e6010e8b4e2dec77d4eaba2b6c66fe86a3933bR503

@cshum
Copy link
Owner

cshum commented Jul 29, 2023

@cshum, this is how our sharp pipeline looks like

_sharpObject
    .rotate() // for auto-orientation
    .sharpen() // default sharpening
    .toColorspace('srgb') // default option, added for dev clarity
    .toFormat(format, formatOptions[format]) // takes in format along with custom options
    .toBuffer() // by default all metadata will be stripped here

Hope this is helpful.

@mevinbabuc can you share the image generated?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants