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

Add support for cICP chunk #218

Open
randy408 opened this issue Jun 15, 2022 · 6 comments
Open

Add support for cICP chunk #218

randy408 opened this issue Jun 15, 2022 · 6 comments
Labels
API Involves additions and/or changes to the API enhancement New feature or request

Comments

@randy408
Copy link
Owner

randy408 commented Jun 15, 2022

Yet another colorspace chunk (this time for HDR), it's not finalized at the moment but Chrome is already adding support for it so it should be considered: https://chromium-review.googlesource.com/c/chromium/src/+/3705739

Proposal: https://github.com/w3c/ColorWeb-CG/blob/master/hdr-in-png-requirements.md#cicp-chunk

@randy408 randy408 added enhancement New feature or request API Involves additions and/or changes to the API labels Jun 15, 2022
@svgeesus
Copy link

svgeesus commented Mar 9, 2023

Now part of the official PNG Third Edition specification

@svgeesus
Copy link

Read support would be great for a PNG checker and write support would be great for tooling such as a utility to add cICP to existing images. Lack of such tooling is blocking adoption by some major users of HDR PNG.

@svgeesus
Copy link

svgeesus commented Dec 7, 2023

@randy408 is there interest in adding cICP support (read/write) to libspng or is it still being evaluated?

If there is interest, is there an estimate of the engineering effort involved and is it the sort of thing that might be suitable for an external contribution?

Asking because we are seeing urgent need to a tool to add or edit this chunk, and currently the state of the art is to crowbar them in with a hex editor.

@randy408
Copy link
Owner Author

randy408 commented Dec 7, 2023

I am planning on adding all HDR chunks at the same time when it's more set it stone, seeing how they're inter-related (e.g. requiring another HDR chunk to be set if another one is also set w3c/png#378).

The unknown chunk API is at least a step above hex editing in terms of convenience:

unsigned char cicp_data[4] =
{
    1, // Colour Primaries
    1, // Transfer Function
    0, // Matrix Coefficients, always 0
    0  // The Video Full Range Flag value MUST be either 0 or 1.
};

struct spng_unknown_chunk cicp =
{
    .type = { 0x63, 0x49, 0x43, 0x50 },
    .length = 4,
    .data = cicp_data,
    .location = SPNG_AFTER_IHDR, // Also means before PLTE
};

spng_set_unknown_chunks(ctx, &cicp, 1);

There is a decode -> encode example in example.c, if you were to adapt that to insert metadata this snippet would go right after spng_set_ihdr() in encode_image(). It's a basic usage example, it needs some modifications to dump the encoded image to a file.

@svgeesus
Copy link

svgeesus commented Dec 7, 2023

Thanks both for the confirmation that the HDR chunks are of interest, and also the practical guidance on how we can use libspng to create test images meanwhile.

Standardization is a constant tension between polishing spec text and demonstrating implementability, and can often get into chicken-and-egg situations.

@svgeesus
Copy link

svgeesus commented Jan 8, 2024

The issue about mDCv requiring cICP was closed, and the spec updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Involves additions and/or changes to the API enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants