cshum / imagor

Fast, secure image processing server and Go library, using libvips
Apache License 2.0
3.38k stars 132 forks source link

Automatically not applying SRGB profile on webp conversion #324

Open mevinbabuc opened 1 year ago

mevinbabuc commented 1 year ago

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.

cshum commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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

mevinbabuc commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

I tried adding a colorspace filter but it doesn't seem to restore the color. https://github.com/cshum/imagor/compare/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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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

mevinbabuc commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

cshum commented 1 year ago

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

https://github.com/cshum/imagor/commit/b8a75af515f0bf07bbc49574a98eb1b054fb25b2#diff-e488e36196c25a401a5af66dd4e6010e8b4e2dec77d4eaba2b6c66fe86a3933bR503

cshum commented 1 year ago

@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?