prism icon indicating copy to clipboard operation
prism copied to clipboard

Basic support for writing ICC profiles

Open AttilaTheFun opened this issue 4 years ago • 3 comments

Hi @mandykoh! Thanks again for your help with the Display P3 profiles.

I've got the profile detection working now, but the new issue is that the built in Go image encoding for jpeg / png images doesn't embed ICC profiles or have a way to do this as far as I can tell.

For conversions from Display P3 -> sRGB I suppose this is okay since images without an ICC profile are generally understood to be in the sRGB color space. That said, it would be nice to be able to write images with sRGB, Display P3, Adobe RGB or ProPhoto color profiles after using your library to linearize and convert between color profiles.

I understand it would be a lot of work to add full support for this, but it would be nice to at least be able to tag images with an existing profile once I write them / during the encoding process.

This repository has Creative Commons licensed compact ICC profiles we could use: https://github.com/saucecontrol/Compact-ICC-Profiles

Do you know how much work it would be to add support for at least attaching these profiles to images? And if you're too busy to work on this, I'd be happy to take a crack at it if you could give me a few pointers.

AttilaTheFun avatar Jul 15 '21 23:07 AttilaTheFun

I agree, this is a big missing piece.

The main difficulty is that the image/* packages don’t expose any way to get at or attach image metadata; this was the reason for the meta/autometa packages in Prism, which unfortunately end up causing image data to be parsed twice (once by Prism to extract the metadata, and then again by image/* to decode the image). There’s been a lot of discussion in the various Go fora about how best to support metadata in the existing image packages (see https://github.com/golang/go/issues/33457) but no consensus seems to have emerged.

Without support for a metadata API, the two approaches I can think of are:

  1. Try to intercept the output of image/jpeg and image/png to surgically insert the profiles. This might mean re-parsing the encoded image to overwrite the relevant fields (eg PNG has a header field that indicates if an ICC profile is being used) and inserting the profile data in the right way. This is obviously not going to be that efficient, and may be tricky to keep from becoming fragile (eg in case the output from the encoders changes).
  2. Write new encoders from scratch that do expose a metadata API. This is obviously a lot of work to get correct and efficient, along with designing the metadata API well.

Happy to keep this issue open to track this as an ongoing puzzle.

Edit: Some folks in the Go issue linked above have made attempts at this, so it might be worth hunting around and seeing if anyone got anything worthwhile.

mandykoh avatar Jul 16 '21 10:07 mandykoh

@mandykoh I think maybe it is not quite that bad. An alternative would be to fork the existing decoders and modify them so that they deserialize format-specific metadata into a [string]([]byte) map rather than discarding it. Decoding that metadata can be done separately, and we can make reasonable decisions later about which data is common enough to warrant eager decoding and validation and then figure out how to memoize the result. EXIF, XMP, ICC, and (eventually) ICCMax seem like the most obvious candidates I can think of.

From a quick read, I like a lot of what was proposed in golang/go#33457. As far as I can tell, that entire thread eventually died for lack of interest, but I think it was proceeding on a useful path. One of the reasons it died may be that they tried to do too much by failing to separate attribute deserialize from decode. Extracting ICC or eXif metadata should not require re-reading and re-parsing the input file, but the argument about which kinds of metadata are important enough to decode is one of those things where the lack of consensus can kill a proposal and the solutions can be implemented later.

I'm not convinced that three key/value stores in PNG is really a problem if metadata attribute names are defined as a path. Duplication of values in a key/value store seems like much more of a problem.

I also think that the Format type probably wants to take a function from io.Reader to bool that detects the image format, similar to what you are doing with autometa. To handle multi-image formats, the checker should assume that the io.Reader is positioned at the first byte of the image, but should not assume that this is the first byte of the containing file.

If we are forking, we might want to reconsider the definition of color.Color. A color isn't something that can return an RGBA value in an undefined color space. For advanced color processing, I think it should either be something that can construct a (color model, color space, value) tuple or perhaps something that can construct a CIELABα (alpha) value, which is mostly universal - at least across human perceivers - and probably simpler.

For legacy compatibility, I'd be okay with choosing an RGB color space by fiat (probably sRGB, because that is what most monitors now do) and saying that a color.Color must also be able to construct an "RGBA in sRGB" color value with an unspecified rendering intent (which is almost certainly what is being done today in practice). If the application is smart enough to care about color spaces more broadly, it probably is smart enough to deal with the CIELABα value.

I think we can safely defer alien color perception support until we have at least one working example. :-)

jsshapiro avatar Jan 16 '23 16:01 jsshapiro

Hmm. Please disregard the CIELABα idea. It isn't the Go way of doing things, and I'm realizing as I think about it that the problem is bigger than color.Color.

jsshapiro avatar Jan 16 '23 17:01 jsshapiro