Marco,

Thanks so much for the code review, really appreciate the level of detail
in your posts.

It will take me some time to digest this information. One question - you
mention that my current PNG reader might
"creates issues if the file have “correct” embedded gAMA of 1/2.2 or
something even better."
Do you mean that the ICC profile may have a better gAMA than 1 ?
In this case, if there is an ICC profile, I suppose I should not set gAMA,
yes?

Side note - one of the applications for my library is in medical imaging,
CT scans etc. where I understand there is no
need for a gamma curve.

Thanks again,
Aaron





On Tue, Feb 21, 2017 at 11:51 AM, Marco Freudenberger <
marco.freudenber...@entrustdatacard.com> wrote:

> Oh, and Aaron:
>
>
>
> I just saw you are setting compression to Z_BEST_COMPRESSION (9). That is
> probably not a good idea either, because it can be REALLY slow with little
> or none advantage in regards to the resulting file size. But that depends
> on your application as well, if you rarely write images and they are
> targeting the web, it MIGHT be the right thing to do. Although – I did
> tests with some images and found that many times lower compression rates
> even give you smaller file sizes. I ended up with using level 3, which is
> ways faster and not really much worse, But that may really depend on the
> images and perhaps on more detailed compression settings like compression
> buffer sizes (of which libpng specification basically says “leave them
> alone unless you know exactly what you are doing” – I didn’t so I left them
> at defaults).
>
>
>
> Marco
>
>
>
>
>
> *Von:* Marco Freudenberger
> *Gesendet:* Dienstag, 21. Februar 2017 10:45
> *An:* 'Aaron Boxer'
> *Cc:* Noel Carboni; lcms-user@lists.sourceforge.net
> *Betreff:* AW: [Lcms-user] Apply profile with PCS = cmsSigXYZData, Color
> Space = cmsSigGrayData
>
>
>
>
>
> Just something „cosmetic“:
>
> 1)      I would set parameter 4 to PNG_COMPRESSION_TYPE_BASE rather than
> 0 (same value, just more obvious what it means).
>
> 2)      I would try to extract the ICC profile name from the profile
> using lcms API (don’t remember how off the top of my head) and supply it as
> parameter 3.
>
> Both are not required, just my love for detail…
>
>
>
> In regards to gAMA: I don’t see anything in the code that is writing the
> image that actually deals with gAMA. However, there are a view things in
> the image READ part regarding gama that make me wonder
>
>
>
> 1)      The code assumes the gamma is 1.0 when there is no gAMA chunk
> present. That is, what some image readers do, but I think this is really
> bad practice, because according to the PNG specification you should use the
> most likely gAMA of the system writing it. That would normally be either
> ~2.2 or 1.8 (the latter if it was written by an old MAC OS).
>
> 2)      Then you use png_set_gamma() to set the SCREEN gamma to 1.0; My
> guess it that it’s extremely unlikely that the screen gamma is 1.0; it is
> more likely ~2.2 (or 1.8 on old MACs), see above. This basically “fixes”
> your assumption of file gamma of 1/1.0, but also creates issues if the file
> have “correct” embedded gAMA of 1/2.2 or something even better. Also, I
> think png_set_gamma() will “correct” the actual RGB or gray or whatever
> values, in 8 bit mode, that means you might lose color precision…
> Note that it’s a total misconception that a gamma value of 1.0 is a good
> “neutral” or “default” value. For example, when reading files like BMP or
> GIF or such without gamma information, you should – if you don’t know
> better – that it has the same gamma as your screen, which is “nominally”
> either sRGB gamma (~2.2) or 2.2.
>
>
>
> There are other things I saw, but it depends on how “serious” you are
> about color management. Unfortunately, PNG is a bit complicated because the
> specification wanted to make it “all right” and some applications do things
> not according to the standard.
>
>
>
> 3)      You seem to ignore the sRGB chunk, which – if present –
> explicitely says an image is in sRGB color space. In that case, gamma =
> sRGB gamma (~2.2)
>
> 4)      You ignore the cHRM chunks. Depending on what level of color
> management you want to achieve, it CAN be important to evaluate them
> (chroma values are a “simplified way” to define a color space). Although,
> they are rarely used. Applications that are serious about color management
> usually embed ICC profiles in the image.
>
> 5)      Depending on what version of libpng you are using – libpng
> internally “explicitely” skips reading one specific sRGB profile
> (identified I think by it’s length and name, don’t remember the details)
> because the original author states that’s a “known bad” profile. I think it
> is actually just a little bit inprecise. Unfortunately, that profile is
> wide spread, and what libpng does – totally skipping the profile, leaving
> you with no color information at all – is highly controversial, because it
> leaves you behind without ANY color space information, although the intent
> is that it should be sRGB. There is a configuration to turn that off:
>
> png_set_option(png, PNG_SKIP_sRGB_CHECK_PROFILE, PNG_OPTION_ON);   //
> turn “ON” skipping the check for an sRGB profile that is “wrong” in the
> libpng author’s opinion -> i.e. allow that profile.
>
>
>
> Having that all said, I don’t really know what your exact use case is,
> maybe all the above is “nitpicking” when it comes to your use case, but if
> you are serious about color management, you should really try to have all
> those things (and some more in regards to PNG) right, otherwise you might
> be contributing to a lot of issues with the PNG format that are already
> “out there”. An ambigious rule in the first version of the PNG
> specification (I don’t remember what it was about) makes it even worse. I
> think it took me ~2 weeks to fully handle all those cases, but my
> application was really a high-precision color management application with
> tons of images from different sources from customers.
>
> Many different applications handle PNG VERY differently unfortunately
> (most of them wrong, even Photoshop, the reference app for color management
> most of the time does some things about gAMA wrong) and the only way to
> avoid any “customer specific maintenance” was to really make sure to go
> 100% by the PNG specification and tell customers that THEIR IMAGES might
> not be according to the standard.
>
>
>
> Marco
>
>
>
>
>
> *Von:* Aaron Boxer [mailto:boxe...@gmail.com <boxe...@gmail.com>]
> *Gesendet:* Dienstag, 21. Februar 2017 07:34
> *An:* Marco Freudenberger
> *Cc:* Noel Carboni; lcms-user@lists.sourceforge.net
>
> *Betreff:* Re: [Lcms-user] Apply profile with PCS = cmsSigXYZData, Color
> Space = cmsSigGrayData
>
>
>
> Hi Marco,
>
> Thanks a lot! Would you be able to take a quick look at my changes ?
>
> Commit is here:
>
> https://github.com/GrokImageCompression/grok/commit/
> 8d62e2252fefd4a15124cac73c7f47a65fde651d
>
> and complete file is here:
>
> https://github.com/GrokImageCompression/grok/blob/master/src/bin/jp2/
> convertpng.cpp
>
> Existing code already had some code for gamma curve, so perhaps I need to
> disable that?
>
> Cheers,
>
> Aaron
>
>
>
>
>
> On Tue, Feb 21, 2017 at 2:28 AM, Marco Freudenberger <Marco.Freudenberger@
> entrustdatacard.com> wrote:
>
> Aaron,
>
>
>
> I might be able to help a little.
>
>
>
> You need to write the iCCP chunk. Also make sure you have no conflicting
> entries in other chunks (sRGB, gAMA, cHRM chunk). PNG is a bit delicate in
> terms of possible ambiguous color space info in header.
>
> You also need to know, that some PNG readers don’t handle the color space
> chucks gracefully (specifically gAMA, cHRM – even Photoshop is clunky in
> that aspect, at least if no iCCP chunk is present).
>
>
>
> Do you use libpng ? If so, you need to do call
>
> png_set_iCCP()
>
> during your file creation to write the ICC profile.
>
>
>
> Marco
>
>
>
> *Von:* Noel Carboni [mailto:ncarb...@prodigitalsoftware.com]
> *Gesendet:* Montag, 20. Februar 2017 13:03
> *An:* 'Aaron Boxer'
> *Cc:* lcms-user@lists.sourceforge.net
> *Betreff:* Re: [Lcms-user] Apply profile with PCS = cmsSigXYZData, Color
> Space = cmsSigGrayData
>
>
>
> >Noel, do you know where I could find sample code to read/write ICC
> profiles for PNG format?
>
> Sorry, I really don't, offhand.  I'd probably rely heavy on Google for
> that.
>
>
>
> -Noel
>
>
>
>
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Lcms-user mailing list
Lcms-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lcms-user

Reply via email to