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

Metadata/profile not retained when using .sharpen() (0.8.1) #125

Closed
homerjam opened this issue Nov 24, 2014 · 10 comments
Closed

Metadata/profile not retained when using .sharpen() (0.8.1) #125

homerjam opened this issue Nov 24, 2014 · 10 comments
Labels

Comments

@homerjam
Copy link

Since 0.8.1 the ICC profile is no longer retained when the .sharpen() method is used

@homerjam homerjam changed the title Metadata/profile not retained when using .sharpen() Metadata/profile not retained when using .sharpen() (0.8.1) Nov 24, 2014
@lovell lovell added the bug label Nov 24, 2014
@lovell
Copy link
Owner

lovell commented Nov 24, 2014

I'll investigate, thanks for reporting this.

@lovell
Copy link
Owner

lovell commented Nov 24, 2014

The fast parameter-less convolution doesn't seem to be impacted so this problem only occurs when providing parameters to sharpen. The profile may lost be due to the extraction/joining of bands within libvips vips_sharpen method but I need to spend longer investigating this.

Technically the output from this module is designed to always be in the device-independent and web-friendly sRGB colour space so the output profile isn't really required.

Could the real problem here be a JPEG image with a device-dependent RGB profile being reported as being in the device-independent "sRGB" space?

If libjpeg says the colour space is JCS_RGB then libvips currently interprets this as sRGB.

The source for libjpeg says JCS_RGB represents the rather ambiguous "red/green/blue, standard RGB (sRGB)" whilst the libjpeg-turbo fork has removed the reference to "sRGB" and reads "red/green/blue ... " only. I'm confused.

Any ideas here @jcupitt?

lovell added a commit that referenced this issue Nov 24, 2014
At the very least will be useful investigating #125
@jcupitt
Copy link
Contributor

jcupitt commented Nov 24, 2014

Oh argh, you're right, it's a vips bug:

$ vipsheader -a wtc.jpg | grep icc
icc-profile-data: VIPS_TYPE_BLOB, data = 0x16f7270, length = 3144
$ vips sharpen wtc.jpg x.jpg
$ vipsheader -a x.jpg | grep icc
$

Stupid thing. I'll fix it.

@lovell
Copy link
Owner

lovell commented Nov 25, 2014

Thanks John. Any thoughts on the RGB vs sRGB ambiguity that is libjpeg's JCS_RGB?

@jcupitt
Copy link
Contributor

jcupitt commented Nov 25, 2014

None of the jpeg tags really mean anything, they predate ICC profiles and are redundant now. No one uses them, or they do, they are almost certainly setting them incorrectly.

I think the best strategy is what web browsers do: if there's no ICC profile, it's sRGB; if there's an ICC profile, it's in device space and you should use the profile to generate pixels. libvips aims to handle colour like this.

I think there are two sensible things sharp could do re. colour:

  1. Pass colours through unaltered (except for the shrink, of course) and leave the profile attached. You can rely on the web browser to fix up the colour for you.
  2. Just before write (ie. after any shrinking), if there's a profile, transform to sRGB and strip the profile from the image. This is very slightly slower, but you'll save 3kb or more from every image, which can be a useful saving in bandwidth if you are displaying a screen full of thumbnails.

There's a lot of other metadata you can strip for even larger savings, IPCT data can be 40kb, for example. I expect sharp does this already.

jcupitt added a commit to libvips/libvips that referenced this issue Nov 25, 2014
jcupitt added a commit to libvips/libvips that referenced this issue Nov 25, 2014
lovell added a commit that referenced this issue Nov 25, 2014
Perform sRGB conversion at end of pipe only

withMetadata exports profile, should not convert

Convert one fixture to sRGB to help test

Discovered while investigating #125
@lovell
Copy link
Owner

lovell commented Nov 25, 2014

Thanks @jcupitt, that's great advice.

Prior to f57a0e3 this module was not importing the embedded profile for RGB images, which was a bug.

It also now uses the input profile with the output if the withMetadata option is set, otherwise the existing, default web-friendly behaviour is to convert to profile-less sRGB and strip.

@homerjam Are you able to test the latest master of both libvips and sharp to see if your problem is solved?

@jcupitt
Copy link
Contributor

jcupitt commented Nov 25, 2014

Looks good!

If you can use vips_icc_transform() it's a lot faster than separate import and export calls. lcms joins the two profiles together behind the scenes and makes a single transform directly between the device spaces.

@homerjam
Copy link
Author

Yep, its working!

I've trimmed some non-pertinent info, but the profile is there.

It has a strange name in Photoshop though - it says black scaled at the end of the profile name?

Result here

identify -verbose sharp-master-libvips-master.jpg
Image: sharp-master-libvips-master.jpg
  Format: JPEG (Joint Photographic Experts Group JFIF format)
  Mime type: image/jpeg
  Class: DirectClass
  Geometry: 702x900+0+0
...
  Colorspace: sRGB
  Depth: 8-bit
...
  Compression: JPEG
  Quality: 90
...
    icc:name: IEC 61966-2-1 Default RGB Colour Space - sRGB
    jpeg:colorspace: 2
    jpeg:sampling-factor: 2x2,1x1,1x1
    signature: 29159a58a51f090cdb50f1bd417f1dbe51ea7c0c5f76fbc9a38e37790fcb756f
  Profiles:
    Profile-8bim: 7680 bytes
    Profile-exif: 5617 bytes
    Profile-icc: 3048 bytes
...
  Filesize: 180KB
  Number pixels: 632K
  Pixels per second: 31.59MB
  User time: 0.010u
  Elapsed time: 0:01.019
  Version: ImageMagick 6.8.9-7 Q16 x86_64 2014-09-23 http://www.imagemagick.org

@lovell
Copy link
Owner

lovell commented Nov 26, 2014

Thanks for confirming James.

sRGB_IEC61966-2-1_black_scaled is the most recent/recommended v2 sRGB profile published by the ICC according to http://www.color.org/srgbprofiles.xalter#v2

@lovell
Copy link
Owner

lovell commented Nov 26, 2014

v0.8.1 now available via npm, thank you both for helping find and fix this problem.

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

No branches or pull requests

3 participants