ImageSharp icon indicating copy to clipboard operation
ImageSharp copied to clipboard

Support for reading writing ICC profiles in Jpeg

Open JimBobSquarePants opened this issue 8 years ago • 13 comments

Currently we lack support for reading/writing ICC color profiles within Jpeg.

This directly negatively affects the color conversion process that we use for converting YCCK or CMYK jpeg images and prevents us from supporting DCI-P3

Reading in C# should be achievable using code based on Drew Noakes MetaDataExtractor (Apache 2.)

https://github.com/drewnoakes/metadata-extractor-dotnet/blob/5411bd3cad10facfd01deb4b1f6878ee242d98a0/MetadataExtractor/Formats/Icc/IccReader.cs

We'll have to additionally figure out how the conversion process takes place. there's info in WikiPedia but it's limited. @antonfirsov Could you possibly have a dig around libjeg-turbo and see if you can find how it's dealt with there? - My C is poor so I struggle to follow that codebase.

We probably don't need the ability to write a profile, just to store it within the image @dlemstra Perhaps we can work in your plan to store different profiles here?

Please comment below on your thoughts everyone.

JimBobSquarePants avatar Jan 10 '17 21:01 JimBobSquarePants

@JimBobSquarePants

attached is a jpg that has its colors muted when saved back to a new jpg file. i get the same results when i save this out of photoshop into sRGB.

hth

sample-sunset

SepiaGroup avatar Feb 27 '17 14:02 SepiaGroup

http://sampleicc.sourceforge.net/ http://www.sno.phy.queensu.ca/~phil/exiftool/TagNames/ICC_Profile.html https://developer.apple.com/library/content/samplecode/ImageApp/Listings/ICC_h.html

JimBobSquarePants avatar Mar 09 '17 13:03 JimBobSquarePants

I wrote a color library in C# which has ICC reading/writing already built in. If you are interested I'd be willing to add that code to ImageSharp. I'll have to do some refactoring but it is very complete and works well. It does not include conversion though (started it but it's not nearly complete). Here are the relevant files: https://github.com/JBildstein/NCM/tree/master/ColorManager/ICC The rest of the library overlaps a lot with the one from @tompazourek but has a different conversion approach.

JBildstein avatar Mar 21 '17 13:03 JBildstein

Hi @JBildstein,

Thanks so much for getting in touch and helping out! ICC profiles are are complicated beast and your experience is most welcome!

I've made a start in a feature/icc branch by adding a large number of the tags and descriptions from the specification.

https://github.com/JimBobSquarePants/ImageSharp/commit/be197bd5b66136ddcc749614c5303d52f9467843

I've shifted my focus just now though to porting the color conversion code from Colourful.NET into our library replacing the current colorspaces (We can't use your conversion code as a basis as it uses reflection.Emit) I'll be doing a lot of work within that conversion to boost performance. I've made a good start, just not pushed anything yet.

If you could focus on ICC reading/writing that would be great!

Managing the combination of our work is the trickiest part and I think the best way to do this is as follows.

  • I give you temporary write access to the repo.
  • I create a work-in-progress PR from my feature branch.
  • We can continue to push to the feature branch until we are happy with the changes.

Doing the above make handling merges much easier and allows us to move rapidly.

How does that sound?

Cheers

James

JimBobSquarePants avatar Mar 21 '17 22:03 JimBobSquarePants

sure, no problem! I'm happy that my endless reading of the ICC specs helps someone 😄 Your start is great, because I don't have any documentation on my enum members (yet). I'll use the Exif code as a reference point on how to do things if that's alright. Getting write access to the branch works for me, definitely makes things easier.

Using Colorful.NET as a basis for the color conversion is a good idea since, as you said, my conversion is not compatible with .Net Core and his is very likely also easier to read. Still, it might be useful for cross referencing: https://github.com/JBildstein/NCM/tree/master/ColorManager/Conversions Even though the actual conversion is pieced together dynamically, most of the formulas can be found in that folder. You have to be a bit careful though because sometimes things like range checking and whitepoint conversions are done elsewhere.

JBildstein avatar Mar 21 '17 23:03 JBildstein

I've created the PR here. If you work on the feature/icc branch it'll automatically update.

https://github.com/JimBobSquarePants/ImageSharp/pull/144

I'll be having a very close look at your library to combine the best of both worlds.

JimBobSquarePants avatar Mar 22 '17 00:03 JimBobSquarePants

This is almost ready to close. A few conversion bits and we're laughing 👏

JimBobSquarePants avatar May 22 '17 12:05 JimBobSquarePants

Jim's branch had been merged back in May.

Is there anything more to do, or can this ticket be closed?

pkese avatar Sep 14 '17 10:09 pkese

@pkese We still don't have conversion using those classes in our jpeg decoder. Once we have them I'll close this.

JimBobSquarePants avatar Sep 14 '17 10:09 JimBobSquarePants

ICC conversion is tracked here: https://github.com/SixLabors/ImageSharp/pull/273 I have written about half of the unit tests for the calculation bits (not pushed yet) and hope to do the rest soon. Work's keeping me a bit busy at the moment so it's going slower than I'd like.

JBildstein avatar Sep 14 '17 19:09 JBildstein

@JBildstein No worries there, we've got time 😄 . If there's anything I can help with please let me know.

JimBobSquarePants avatar Sep 14 '17 23:09 JimBobSquarePants

@JimBobSquarePants good to know ^^ but I think it should definitely be done by 1.0 thanks, I'll need some help (or some pointers) for integration and I'd love some feedback on the design. I'm not entirely happy about it yet. But I'll give you a ping on gitter when I'm ready.

JBildstein avatar Sep 15 '17 06:09 JBildstein

Good man, no worries

JimBobSquarePants avatar Sep 15 '17 06:09 JimBobSquarePants

I'm closing this. We can read/write Icc Profiles now. Converting between is a different matter covered by #1567

JimBobSquarePants avatar Jan 23 '23 12:01 JimBobSquarePants