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

Testing cICP #312

Open
svgeesus opened this issue May 8, 2023 · 29 comments · May be fixed by #472
Open

Testing cICP #312

svgeesus opened this issue May 8, 2023 · 29 comments · May be fixed by #472
Milestone

Comments

@svgeesus
Copy link
Contributor

svgeesus commented May 8, 2023

This can be tested in WPT by converting full-range sRGB values to one of the common encodings (such as 2100 PQ, 2100 HLG, Display P3, linear sRGB) and making a PNG with those values and the appropriate cICP chunk. A pass displays the same colors as an sRGB reference image.

For non-browser support, detecting and correctly decoding the chunk demonstrates support.

@svgeesus
Copy link
Contributor Author

svgeesus commented May 8, 2023

TweakPNG may add support for cICP:

@svgeesus
Copy link
Contributor Author

svgeesus commented May 8, 2023

Chromium has cICP support for PNG (and also supports CICP in AVIF):

@ProgramMax
Copy link
Collaborator

What you described is nearly exactly what is already being tested here:
https://wpt.fyi/results/png/cicp-chunk.html?label=master&label=experimental&aligned

There is a cICP chunk which marks the image as Display P3.
The image is painted into a canvas and its color value read and compared to what it would be when converted to sRGB.

Does that mean we can close this?

@svgeesus
Copy link
Contributor Author

We could, although I would prefer to see more than one test; and I want to track non-browser implementations like TweakPNG too. So I prefer to keep it open.

@kmilos
Copy link

kmilos commented Sep 1, 2023

FWIW, darktable implemented support* for cICP chunk some time ago (though only a subset of possible color spaces are actually built in and recognized by the input color profile "colorin" pipeline module).

* for import only, export uses iCCP

@svgeesus
Copy link
Contributor Author

svgeesus commented Sep 1, 2023

Thanks for the heads-up! Yes I see it in the darktable 4.2.0 announcement:

Attempt to obtain the color-space for PNG files from the cICP chunk. This was added in a recent revision of the PNG spec, so we take advantage of it, if present.

@svgeesus
Copy link
Contributor Author

@svgeesus
Copy link
Contributor Author

I raised a Bugzilla issue on Firefox for cICP support.

@svgeesus
Copy link
Contributor Author

svgeesus commented Nov 7, 2023

@svgeesus
Copy link
Contributor Author

svgeesus commented Nov 8, 2023

Mozilla Request for standards position for cICP

@svgeesus svgeesus added this to the 3rd edition milestone Jan 8, 2024
@svgeesus
Copy link
Contributor Author

svgeesus commented Jan 9, 2024

The ImageSharp image library for .NET has added PNG cICP support

@svgeesus
Copy link
Contributor Author

svgeesus commented Jan 9, 2024

At the 8 Jan 2024 PNG WG meeting it was concluded that cICP has now met the W3C minimum support criteria (two independent implementations) so removing that tag.

Leaving the issue open to collect latest implementation status, for the implementation report.

@svgeesus
Copy link
Contributor Author

I have a PR for a test that combines APNG and cICP to check that all frames inherit the colorspace information, not just the static image.

web-platform-tests/wpt#43980

@svgeesus
Copy link
Contributor Author

I have a PR for a test that combines APNG and cICP to check that all frames inherit the colorspace information, not just the static image.

PR is now merged, as hoped Chrome (and Edge) pass while Firefox and Safari do not

@svgeesus
Copy link
Contributor Author

svgeesus commented Jan 22, 2024

From the documentation for oxipng, an optimizer written in rust:

--strip <mode>
Strip metadata chunks, where <mode> is one of:

     safe    =>  Strip all non-critical chunks, except for the following:
                    cICP, iCCP, sRGB, pHYs, acTL, fcTL, fdAT
    all      =>  Strip all non-critical chunks
     <list>  =>  Strip chunks in the comma-separated list, e.g. 'bKGD,cHRM'

source

So clearly, it supports cICP and knows that stripping it will affect the rendered image.

@svgeesus
Copy link
Contributor Author

CSS Color 4, in the section Color Space of Tagged Images, now includes the test results for the PNG cICP tests.

@svgeesus
Copy link
Contributor Author

I added a WPT PR to test that cICP wins over iCCP

@svgeesus
Copy link
Contributor Author

I added a WPT PR to test that cICP wins over iCCP

This is now merged. Chrome 123 on Linux 20, and Edge 123 on Win 10 both pass. Safari TP 188 on macOS 13.6 is reported as failing, because it needs macOS 14 for CICP support. Firefox 125 on Linux 20 fails because they don't implement cICP yet, see bug

@svgeesus
Copy link
Contributor Author

svgeesus commented Mar 5, 2024

libavif has an open issue

@wantehchang
Copy link

wantehchang commented Mar 5, 2024

@svgeesus Chris: The libavif issue AOMediaCodec/libavif#1421 that you mentioned in #312 (comment) covers conversion of AVIF to both PNG and JPEG. The PNG part of that libavif issue is already fixed. So for PNG that issue should be considered closed.

@svgeesus
Copy link
Contributor Author

svgeesus commented Mar 27, 2024

Affinity 2.4 supports cICP in PNG including narrow and full range.

Affinity product page

@digitaltvguy
Copy link
Contributor

digitaltvguy commented Mar 27, 2024 via email

@fintelia
Copy link

fintelia commented Sep 2, 2024

I tested the Video Full Range Flag in both Chrome and Safari. Neither one seems to respect it: pixel values were the same regardless of whether it was set to zero or one

(Firefox was the same... but that's expected since it doesn't have cICP support yet.)

@svgeesus
Copy link
Contributor Author

svgeesus commented Sep 3, 2024

I see several Chrome CICP bugs one of which relates to Full-Range AVIF. Probably best to open a separate issue for PNG Full/Narrow range CICP.

@digitaltvguy
Copy link
Contributor

digitaltvguy commented Sep 3, 2024 via email

@ProgramMax
Copy link
Collaborator

ProgramMax commented Sep 3, 2024

This feels like a tricky one.
For typical computer use cases today, all images will be full range. In fact, I think even an (non-broadcast) expert would need to work very hard to find even 1 real world narrow range image. As a result, I can understand not wanting to spend engineering time solving a non-problem.
Chrome considered it explicitly and closed as won't-fix because there was no interest.

However, that then forces the question "Then should it be part of the spec?"

For legacy broadcast, it does seem important. That's why it is part of H.273 in the first place.
For broadcast experts, I could see PNG test images that use this flag being very useful. Any software along the pipeline which reads CICP data will want it.

To me, the tradeoff is "Almost all users don't need this, so we should drop it..." vs. "...but then we're not using real CICP / H.273."

Perhaps we could still reference H.273 for all CICP values and just drop that field.
(To that end, perhaps we can also drop the one that says the color model is RGB, since we likely will always have a standard RGB PNG available even if we later add extra channels.)
But I feel like the cost of 2 bytes which are unenforced is worth it to keep H.273 in tact. What if later H.273 changes from 4 bytes to 2 bit version flag, 6bits, 12bits, 4 bits, 8 bits? We'll have wished we kept all 32 bits.

So maybe the key is for us to make it a note and reference H.273 but to not require strict enforcement on the values?

@fintelia
Copy link

fintelia commented Sep 4, 2024

The difference between a cICP chunk taking 16 bytes versus 14 bytes (once counting the chunk header and CRC) doesn't really seem like enough justification to diverge from the base specification.

I'd say that the spec should only require that decoders support full-range images, given that that's what major implementations are planning on doing. Leaving the text as-is would likely create pointless engineering work for implementers who don't realize quite how rare narrow-range images are. Though I'm not quite sure how to word it in the spec so that web browsers which ignore the flag as well as legacy broadcast folks' software can both be considered conformant.

@svgeesus
Copy link
Contributor Author

If we say that narrow-range images are poorly supported and should not be created, we effectively require creation/conversion tools to convert narrow-range images to full range, which is lossy due to quantization error.

@svgeesus
Copy link
Contributor Author

The definition of reference image is a full-range image (although it does not explicitly use that term, and probably should): it clearly maps 0, not 16, to black for example.

@digitaltvguy digitaltvguy linked a pull request Sep 30, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants