feat: CICP metadata support for PNG#4746
Conversation
|
The failures are interesting -- they pertain to a bit of code I need to refactor which will probably make them go away, but they point to unexpectedly different behavior either in The failures pertain to the ImageSpec::set_cicp() function that accepts a single string_view argument: void
ImageSpec::set_cicp(string_view cicp)
{
// If the string is empty, erase the attribute.
if (cicp.empty()) {
erase_attribute("oiio:CICP");
return;
}
auto vals = Strutil::extract_from_list_string<int>("0,0,0,1", 4, 0);
auto p = find_attribute("oiio:CICP", TypeDesc(TypeDesc::UINT8, 4));
if (p) {
string_view existing_vals = metadata_val(*p);
Strutil::extract_from_list_string<int>(vals, existing_vals);
}
Strutil::extract_from_list_string<int>(vals, cicp);
set_cicp(vals[0], vals[1], vals[2], vals[3]);
}What I'm doing here is a little silly -- if the "oiiio:CICP" attribute already exists in the spec, cast the values to a string, and then extract a vector of integers from said string; and then update that vector with values extracted from the user-provided string argument. I know, the casting-to-a-string bit seems unnecessary. When I'd originally worked this out, I'd observed that libpng apparently incorrectly permits embedding string data to the Anyway, the actual failures we're seeing are due to the Windows and "(Old)" CI runners failing for some reason to extract integers from the string produced by So, in Python, on affected runners: spec = oiio.ImageSpec()
spec.set_cicp("1,2,3,4")
# should set "oiio:CICP" to [1, 9, 3, 4], but instead sets to [0, 9, 0, 1]
spec.set_cicp(",9,,") Long story short, I'm gonna simplify this method, perhaps move some string parsing logic to png_pvt.h directly -- and it's likely gonna clear up these CI failures. But I still think this behavior is worth investigating further... |
zachlewis
force-pushed
the
cicp_png
branch
6 times, most recently
from
bd93744 to
8b639f2
Compare
|
Just talking out loud here. These are discussion prompts, not necessarily prescriptive critiques. I'm a little leery about making it directly a method of ImageSpec. We actually aimed to NOT have a proliferation of methods that set or retrieve each piece of metadata separately, instead just relying on attribute()/getattribute(). I'm not crazy about the precedent adding this would set. There is a freestanding utility function Also, I'm wondering what the interaction between those calls and metadata should be. Should set_colorspace set the cicp flag also if the colorspace is one that is representable as a cicp code (and maybe clear the cicp if it is not)? And vice versa, should reading a cicp code in a file (or calling set_colorspace_cicp) not only set that attribute but also set "oiio:ColorSpace" to the right thing if there is an obvious correspondence? |
|
Let me throw out an alternate design for discussion:
In other words, to apps and OIIO internals, there is only "ColorSpace", though what's stored there can take a few forms (always using a CIF token if we can figure out the right one). It's a detail of individual file format readers and writers to handle translating to and from CICF fields in those file that understand it. |
|
Honestly, I'm not wild overloading the ColorSpace attribute like this. But let me propose a counter proposal:
Given the complexity of all the moving pieces, I think it would be incredibly helpful to treat the CICP attribute as a first class citizen, for the sake of diagnostics, if nothing else:
It's not completely impossible to do most of the above with a single overloaded The way I envisioned the CICP attribute working:
Does this seem reasonable? |
|
n.b. these are about what came before the last three posts here Mostly the variables you use for primaries, transfer function, matrix and video full range flag are Should we be allowing the caller to set CICP components to invalid values? In particular, 0 is an ITU-reserved value for color primaries and will probably never be valid; likewise 0 for transfer function. And non-zero values for a matrix enum are just plain wrong unless the essence is either YCbCr or ITcTp. Are there other places in OpenImageIO where a user passes in what is basically an enum value, but as an int instead of a type-checkable enum, and we look at that int's validity if interpreted as that enum? Oh and there's one point in a comment where it's implied that both YCbCr and ITcTp are always "legal range" (or if you want to be more de jure in your terminology, "narrow range"). But this is not true for ITcTp; BT.2100 defines full-range ITcTp for both 10-bit and 12-bit integer encodings. Now I'll go back and read the last three posts here; but not before thanking you both (Larry and Zach) for considering this stuff so carefully. Oh, and one last thing: is the autocorrect I am now encountering as I type in my comments here an artifact of GitHub, or of my local browser? The only thing worse than trying to type in code in a text buffer that autocorrects, is trying to type German into such an autocorrecting text buffer when the local platform language is English. If it's my browser, I'll try and fix it; if it's GitHub I'll just do the "old man yells at cloud" thing. |
|
I like Zach's counter proposal A LOT but have some questions:
Would you see OCIO then enhanced to let you supply potentially all of the above to it, and it would apply any config-specified resolution protocol or a default resolution protocol if none were explicitly provided? I still want somehow to be able to detect when an image has other metadata that conflicts with its CICP value. That feels like something where one should be able to ask a file format's plugin "what are all the ways you can identify your colorspace" and then pass those to OCIO for sanity checking. Though I hope for a period where we see better support from CICP for certain combinations of primaries and transfer function — say someday support for 16-bit LogC4 in PNG — my feeling is that the amount of time it takes for that to go through ITU standardization will be long compared to OpenColorIO release cycles, and anyway in the meantime people can fall back on filename patterns. Personally I find that sort of thing cringe, but I've been too long out of the production trenches to suggest to others how they should work. |
|
Fantastic feedback, Joe. Thank you.
Good catch -- will fix!
Very good points. Like you say, the So... my possibly-misguided thinking is... we should probably include a means for converting to / from YCbCr / ICtCp, independently of color space conversions. There are a handful of ways we could approach this with OCIO. In a best-case scenario, this is somehow controlled for by the OCIO API itself -- that's something we'll have to discuss further with the OCIO folks. Another solution -- we could add in-memory to the OCIO Config used by ColorConfig a series of NamedTransforms for the relevant conversions (Rec.709-to-YCbCr, Rec.2020-to-YCbCr, Rec.601-to-YCbCr, XYZ-to-ICtCp...), and invoke their inverses either on In practice, there are four image formats that I know of that support CICP:
Great point -- I'll update the comment to better clarify.
It's your browser. If you highlight the offender and right-click, you should see some sort of "Add to dictionary" option.
Sure. There should have been a slash between "active" and "default". The Python OCIO API offers
The OCIO API currently offers a So, the three uses would be:
Maybe. It's all up for discussion.
Now... it should be possible to match / derive ad-hoc color spaces given
That would be interesting. It would be nice if OIIO could optionally reconcile the other attributes with a given CICP attribute on write.
Well, as far as OCIO release cycles go, OCIO doesn't intend to validate CICP data (beyond any heuristics provided by the config author). So, any new additions to H.273 et al. shouldn't break OCIO ABI compatibility. And there's really nothing stopping us from defining an Re: filename patterns -- they can be extremely useful in a lot of situations -- particularly where the person setting the filename doesn't have control over the metadata or the means to interpret it. Right now, it's also the only way we have to indicate whether to apply an inverse display-view transform or a colorimetric color space transform when "linearizing" output-referred encodings. |
|
I just want to refocus the discussion here so we can move things forward... @lgritz -- given that...
...are you any more comfortable with the idea of tracking a separate I feel the benefits of creating the scaffolding for robustly wrangling CICP, among other types of downstream-decoding metadata, outweigh the costs of managing the when, where, and how of OCIO interaction; and I fear that overloading the That said, I definitely understand the allure of using a single attribute to track the internal "image state" of the buffer while it's in motion, and if that's the way you feel we should go for now, I'd be more than happy to oblige. |
|
I don't mind there being a separate "oiio:CICP" attribute. (Aside: if this is a well established standard, we can just call it "CICP". Usually the "oiio:" prefix is for things that only OIIO is expected to understand, and/or internal signalling/hinting of some sort.) I'm less sure whether it's so foundational that we should have a separate ImageSpec::set_CICP() method just for that, versus calling ImageSpec::attribute like we would for anything else. We wanted as much as possible to just use the generalized token/value metadata store, and only have special fields or single-metadata API functions growing with each new item that comes along. |
I meant "only have special... when really necessary" or "NOT have special calls for each new item that comes along". But somehow I combined the two phrasings and managed to completely garble the message. |
|
Ha! I understood your meaning. The main reason why I implemented an Another reason why I implemented an Originally, I was thinking, it would make sense to attach (I was also thinking, it wouldn't hurt to have an overloaded Anyway, we can discuss what to do with the other metadata schemes elsewhere -- I just wanted to give you an idea of where my head was at. All this said, as far as this PR is concerned, I'll happily do whatever you'd prefer. If you'd like to get rid of the Should I proceed with getting rid of |
|
How about this compromise: instead of making set_cicp a method of ImageSpec, let's make it a free-standing function (somewhat like the existing I'm on the fence about whether the string-based API call is necessary, or if it's sufficient to just make that functionality live in oiiotool. Maybe start with that part just in oiiotool, but if we decide later that we really want it directly, it's easy enough to add? But I'm happy to go with your recommendation if you want it in the API now. I don't have a good feel for how often it will come up that people will want to manipulate just one field independently and can't just "get" all 4, modify what they want, and then send the whole package back. Will that idiom of 4 or 5 lines be so commonly repeated in people's code that it's worth having a new API call for us to test and maintain? |
|
Just so you know what's currently swirling in my brain for a place to work toward eventually: You have convinced me that separately maintaining "OCIOColorSpace", "CICP", "Chromaticities", and possibly other attributes is the right approach, if for no other reason, then simply for the sake of being able to accurately report what was really in a file that we read. However, I still feel like it would be a mistake to make everyplace in the code base that needs to check or set or propagate color space information, and every piece of USER code that needs to do so, to have to check a growing list of possible attribute names and different standards for what color space the pixels are, understand them all (plus, the list may grow), and also to know which one to set in order to be supported by the output file type they are using (which is antithetical to OIIO's original core purpose of allowing near total format-agnosticism on the app side). So what I'm thinking is that there is still a use for a get_colorspace() method/function, centrally implemented, that surveys the various attributes and synthesizes a single string descriptor for the color space to put in "internal" attribute "oiio:ColorSpace", and a corresponding set_colorspace() that takes such a token and sets any or all of the individual attributes (and clearing any that contradict, can't be known, or have no corresponding version). This centralizes the logic for translating between the different standards and lets most apps, and most parts of OIIO internals, operate as if it was a single simple attribute, even though there are actually several attributes and standards that get elaborated depending on the file type. So, for example, if you read an openexr file that has a "OCIOColorSpace" attribute, that will be set in the ImageSpec and also "oiio:ColorSpace" will be set to, say, "lin_rec709". If it reads a PNG file with CICF info, the "CICF" attribute will be set, but also "oiio:ColorSpace" will end up with an OCIO name if it corresponds to one, but if not, then "cicf:1,2,3,4". Or if all that's known is chromaticities and they don't seem to correspond to any known CIF space, maybe it will just get "chromaticities:....". Is something like that going to make all the color scientists hate me? |
|
Roger that -- I will...
Let me know if I've forgotten anything.
No, I wouldn't imagine so -- really, the kind of wrangling we're talking about would only happen under pretty unique and specific circumstances. We can make things simpler for user code by treating the CICP attribute as an
Thank you! This is crucial.
Oh, couldn't agree more - ideally, the vast majority of user code should be able to drive everything that needs driving with only the (Do format writers have easy access to the ColorConfig?)
I'm not sure about the particular method details themselves, but I think this is 100% the right idea, and aligns with the approach I imagine OCIO's API will very likely take! Sidebar: It might be useful to contrive an internal
Mmmm. I don't know... maybe I'm misunderstanding, but with a That said, I don't at all mind the idea of string-based shorthand for setting set non- I want to highlight two things:
|
I think we're on the same page about pretty much all of this.
Cool. Needless to say, the specifics I outlined were just off the top of my head and speculative, all depends on how the OCIO stuff shakes out, input from others, etc.
There is one default color config, set by
I think there's an overall question we've always had about which metadata being set should invalidate which other metadata, and what to do about things that seem to contradict. The metadata needs metadata!
It won't. If you call set_colorspace("cicp:x,y,z,w"), the logic would be: Conversely, if the thing passed to set_colorspace appears to be an OCIO or CIF name 'foo', the logic would be: or something like that. Oh, and if it gets as far as an IBA that needs to know something about color space and it's anything other than an OCIO token, it just will treat it as an unknown color space. In effect, the "CICF" values that don't correspond to a known OCIO color space are really only being carried around for the sake of the eventual ImageOutput that writes the file.
We do require OCIO as a dependency now, at a minimum of 2.2, so there is always at least that built-in color config available. I'm sure there are lots of places in the OIIO code that haven't yet caught up to the best logic to apply given that as a minimum set of capabilities, and could thus be improved to give more consistency and better sensible default behavior. |
|
Elaborating a little more... There are a few possible attributes that could be set:
And there are a few operations of note: set_colorspace
ImageInputs:
ImageBufAlgo functions():
ImageOutput:
So the upshot is that most of the OIIO code base and most apps can just deal with oiio:ColorSpace and treat nothing else as special; ImageInputs just call set_colorspace; ImageOutputs need a little format-specific logic to see if the spec has the right kind of color information (or it can be deduced by what's there); ImageBufAlgo lives in an OCIO world. But all along, things like |
|
Ah, I see -- I thought Either way, I still don't understand why we'd want
But why overload In my view:
Does that make sense? It would mean IBAs that alter the output spec's It may be the case that only approximate chromaticities can be derived, or that a "gamma" cannot be ascertained, which is fine. The approximated chromaticities will be good enough for government work, and the "gamma" either won't apply for the color space (cuz it's a log-encoded space, etc.), or, if an output format absolutely requires a "gamma" metadata, we can attempt to derive a value from the output filename, vis-a-vis the OCIO config's FileRules; and failing that, we can require user intervention to override OIIO's existing format-specific defaults. (It would also mean that we'd probably want to start adding an "opt-in" feature to the EXR writer for embedding chromaticity metadata; because, as nice as it may be that we can derive values, CIF et al would like to deprecate the attribute entirely, in favor of |
At one point, so did I. I'm basically just talking out loud here, trying to grope my way to a set of rules that makes for consistent behavior while minimizing what apps using OIIO need to do. The only reason I have them separate now is to disginguish the OIIO operative color space of the image ("oiio:ColorSpace") from "the explicit OCIO name we found in the file, for those formats that support it."
I believe I was thinking that this just gives us a single item to pass all the way through the chain? But maybe I'm wrong and it's not necessary?
Maybe? But like I said, if as we are implementing this, we see that there are some redundancies or inconsistencies, we should definitely eliminate them.
Oh, I didn't know this would be possible except for a very few special cases. An area of OCIO's APIs I have never really dealt with is building configs or augmenting a loaded config with addition CS's. Do all the CIF quads map to something we could in principle add on the fly to the OCIO config, so we really can just concern ourselves with OCIO tokens?
This is the only part you wrote that I disagree with (or maybe just don't understand exactly what you mean). I wouldn't say "oiio:ColorSpace" is ignored by ImageOutput; I would say it's the single source of truth if present, but it's up to the ImageOutput for each format to know which fields and standards are supported by that format and express it the right way in the file.
I think it's simpler than that. I'm suggesting that "CICP", "gamma", "OCIOColorSpace", etc. only express input file metadata. ImageInputs also set "oiio:ColorSpace". IBA functions that don't do color space transformations just pass all metadata through untouched. IBA functions that change color space erase all of them and just set a single "oiio:ColorSpace" to carry all information forward (e.g. the only thing we know coming out of an IBA color transforming function is what we think the new OCIO color space is). ImageOutput set whatever fields they support based on oiio:ColorSpace if it is present, but otherwise may fall back on a format-specific item like "CICP" if present.
I'm in the camp of people who think "Chromaticies" should be considered deprecated for exr files. I have never considered it trustworthy, and I don't think OIIO even reads it from the file. |
|
Should we be moving this wonderful (I mean it) discussion to design the future of passing color space info around within OIIO to a GitHub Discussion rather than on this one PR which presumably will be merged and closed soon? |
|
Small note for @zachlewis : please refer to me in these discussions as "Joseph" rather than "Joe". It's not that I take offense at the diminutive form, but it's that everyone knows me as Joseph, and so if you attribute something to "Joe" they will miss the association, at least without tracing back to my comment and extracting the last name from my GitHub name. Again, no offense taken, I'm just saying this for clarity if anyone comes digging in this PR later. |
|
Catching up (my you've both been energetic in the last 72 hours or so)
|
Sorry, yeah. The right combo of font and eyes and you can see how I keep making that mistake, right? |
I'm not sure what you mean by "deal with", but so far, a core assumption of OIIO is that these are encoding details that live entirely internal to the ImageInput/ImageOutput implementation of a format. The "app side" of the OIIO APIs does not have the concepts of subsampling nor non-RGB data encodings (just like it doesn't know about compressed data streams, or bit depths that aren't 8/16/32). Ideally and whenever possible, those encoding details aren't even handled in the ImageInput/ImageOutput, but are handled completely in the underlying per-format library if at all possible.
100%, that's always been our policy -- ffmpeg for video, libraw for camera raw, libtiff for YCbCr-encoded tiff files, etc.
I have an admittedly sketchy grasp of this part, even the terminology. We want to honor whatever external OCIO config is found at runtime, and in the absence of one, default to modern OCIO's "built-in" config. If there's a way to dynamically augment that at runtime in order to outsource even more of the color handling code to OCIO rather than do any of it on the OIIO side, that's great. But when you call it an "OpenImageIO custom OCIO config" I start to get confused, because that sounds (to me, and I bet to OIIO's downstream users who aren't super OCIO gurus) like it's something we're doing instead of their studio config or OCIO's built-in config. |
|
In re: having @zachlewis's comment was "If we want OIIO to fail over to "lin_rec709" instead of ACES2065-1, like ocio://default specifies, OIIO should ship with a custom OCIO config as its built-in default. (It can easily be a modified version of one of the builtin OCIO configs)." and I created the phrase "OpenImageIO custom OCIO config" to describe that. Sorry if that made things unclear. |
|
A bunch of this may be helped by reference white metadata that is being worked on elsewhere. This could help us interact between floating point scene referred workflows with cicp to a display referred workflow with cicp .
Chris Seeger
Director, Advanced Content Production
Office of the CTO
…________________________________
From: Zach Lewis ***@***.***>
Sent: Tuesday, September 9, 2025 11:19:49 PM
To: AcademySoftwareFoundation/OpenImageIO ***@***.***>
Cc: Seeger, Chris (NBCUniversal) ***@***.***>; Mention ***@***.***>
Subject: [EXTERNAL] Re: [AcademySoftwareFoundation/OpenImageIO] feat: CICP metadata support for PNG (PR #4746)
[https://avatars.githubusercontent.com/u/2228592?s=20&v=4]zachlewis left a comment (AcademySoftwareFoundation/OpenImageIO#4746)<https://urldefense.com/v3/__https://github.com/AcademySoftwareFoundation/OpenImageIO/pull/4746*issuecomment-3273077326__;Iw!!PIZeeW5wscynRQ!uUw_5tYJ2mL1d7awBdGqcbXD6UldXqXtxAR9EWFP8mNQGUb3rQ12UsHMo3BPsdoyW18KPQMUt-zN-HqpWYm1G0m-Dw$>
The only thing I worry about a little is whether OpenEXR will adopt an official CICP attribute soon, and if so, whether the name will match what we're doing.
I'd be very surprised if OpenEXR decides to go in that direction, but it wouldn't hurt to ask. Ironically, CICP and OpenEXR (ideal) use cases are almost mutually exclusive! And in any case, I think the colorInteropID attribute intends to convey what an official OpenEXR CICP attribute would otherwise convey.
That said, maybe we should pepper all things color metadata related with "experimental -- under construction -- subject to change" caveats throughout the 3.1 cycle? Lots of things are in flux. We are the bleeding edge of aspects of this little color metadata management nightmare.
—
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https://github.com/AcademySoftwareFoundation/OpenImageIO/pull/4746*issuecomment-3273077326__;Iw!!PIZeeW5wscynRQ!uUw_5tYJ2mL1d7awBdGqcbXD6UldXqXtxAR9EWFP8mNQGUb3rQ12UsHMo3BPsdoyW18KPQMUt-zN-HqpWYm1G0m-Dw$>, or unsubscribe<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AANIO6FHLFKFZ4WIDNROZ3D3R6KFLAVCNFSM6AAAAAB44ETE7WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTENZTGA3TOMZSGY__;!!PIZeeW5wscynRQ!uUw_5tYJ2mL1d7awBdGqcbXD6UldXqXtxAR9EWFP8mNQGUb3rQ12UsHMo3BPsdoyW18KPQMUt-zN-HqpWYm3TnWF0w$>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
- Also added a test in the test suite to make sure setting the color space erases the attribute
- Replaced usages of "set_colorspace" in the PNG reader with `attribute("oiio:ColorSpace", ...)` calls for the time being, to ensure the CICP attribute isn't removed before it has a chance to persist. In a future PR, we'll address what actually _should_ be done here.
Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Zach Lewis <zachlewis@users.noreply.github.com>
Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Zach Lewis <zachlewis@users.noreply.github.com>
Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Zach Lewis <zachlewis@users.noreply.github.com>
|
Assuming this fully passes CI now, is this ready to merge? |
| /// - `"cicp"` : | ||
| /// Does this format support embedding CICP metadata? | ||
| /// |
There was a problem hiding this comment.
Does a similar comment need to go in ImageInput::supports so that it's documented for both?
There was a problem hiding this comment.
It sure does! On it.
There was a problem hiding this comment.
need to make sure I've appropriately documented in oiiotool.rst and stdmetadata.rst as well.
|
The code looks fine now and tests pass, so just make an unambiguous comment when you've completed all the small docs and other changes and are ready for me to merge. |
Add a little info re: how the CICP attribute is stored and manipulated. Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
|
This PR is ready to merge! Thanks Larry! |
| window that are not overlapping the pixel data window. If not supplied, | ||
| the default is black (0 in all channels). | ||
|
|
||
| .. option:: "CICP" : int8[4] |
Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Zach Lewis <zachlewis@users.noreply.github.com>
Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Zach Lewis <zachlewis@users.noreply.github.com>
|
For follow-up after this is committed: I think the only CI builder we have that uses libpng 1.6.45+ (required for CICP support) is the "all local builds" one. Maybe we should sprinkle around some newer libpng versions for a few other builds (like "Sanitizers" at least?) |
|
Oh, seems like the "latest releases" and "bleeding edge" jobs maybe need to ensure a more recent libpng. |
|
I'll do a pass separately to make sure the "latest releases" tests are all current. |
This PR addresses the primary concern of AcademySoftwareFoundation#4678 -- implement support for reading and writing the `cICP` chunk for PNGs. CICP (Coding Independent Code Points) is a means for using a tuple of integers to communicate characteristics of a variety of, traditionally, video encodings. The tuple is a series of four values that map to integer enumerations (codified in [ITU-T H.273](https://www.itu.int/rec/T-REC-H.273-202407-I/en)) representing a certain ColourPrimaries, TransferCharacteristics, and MatrixCoefficients, as well as a VideoFullRangeFlag. In particular, CICP is used for describing HDR encodings to browsers and image viewers capable of displaying the image as intended. For example, if one were to write P3-D65 PQ-encoded RGB values to a PNG, it will only look "correct" if there's an appropriate `cICP` chunk describing the primaries (14) and transfer function (16); without such metadata, PQ-encoded images will appear significantly darker and lower in contrast. Internally, CICP metadata is stored in a `int[4]` type `CICP` ImageSpec attribute. This PR adds the following: - An oiiotool `--cicp` flag for setting, modifying, or removing CICP metadata for the top image - Methods for reading and writing PNG `cICP` chunk metadata <--> ImageSpec `CICP` - Tests I've included tests. I've also embedded CICP metadata in the existing 16-bit test png in the testsuite. But to see what this is all about with your own eyes, you can quickly convert a scene-linear AP0-encoded EXR to a PQ-encoded P3D65 HDR PNG with the following command: `$ oiiotool -i input_linap0.exr --ociodisplay:from=ACES2065-1 "ST2084-P3-D65 - Display" "ACES 1.1 - HDR Video (1000 nits & P3 lim)" --cicp "12,16,0,1" -o output_p3d65pq.png` If you compare in a capable image viewer on a capable display the image produced with the above command compared to another one that lacks CICP metadata, you should see a pretty significant difference: `$ oiiotool -i output_p3d65pq.png --cicp "" -o output_no_cicp.png` ("Preview" on a ~5 year old Macbook should suffice). --------- Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
This PR addresses the primary concern of AcademySoftwareFoundation#4678 -- implement support for reading and writing the `cICP` chunk for PNGs. CICP (Coding Independent Code Points) is a means for using a tuple of integers to communicate characteristics of a variety of, traditionally, video encodings. The tuple is a series of four values that map to integer enumerations (codified in [ITU-T H.273](https://www.itu.int/rec/T-REC-H.273-202407-I/en)) representing a certain ColourPrimaries, TransferCharacteristics, and MatrixCoefficients, as well as a VideoFullRangeFlag. In particular, CICP is used for describing HDR encodings to browsers and image viewers capable of displaying the image as intended. For example, if one were to write P3-D65 PQ-encoded RGB values to a PNG, it will only look "correct" if there's an appropriate `cICP` chunk describing the primaries (14) and transfer function (16); without such metadata, PQ-encoded images will appear significantly darker and lower in contrast. Internally, CICP metadata is stored in a `int[4]` type `CICP` ImageSpec attribute. This PR adds the following: - An oiiotool `--cicp` flag for setting, modifying, or removing CICP metadata for the top image - Methods for reading and writing PNG `cICP` chunk metadata <--> ImageSpec `CICP` - Tests I've included tests. I've also embedded CICP metadata in the existing 16-bit test png in the testsuite. But to see what this is all about with your own eyes, you can quickly convert a scene-linear AP0-encoded EXR to a PQ-encoded P3D65 HDR PNG with the following command: `$ oiiotool -i input_linap0.exr --ociodisplay:from=ACES2065-1 "ST2084-P3-D65 - Display" "ACES 1.1 - HDR Video (1000 nits & P3 lim)" --cicp "12,16,0,1" -o output_p3d65pq.png` If you compare in a capable image viewer on a capable display the image produced with the above command compared to another one that lacks CICP metadata, you should see a pretty significant difference: `$ oiiotool -i output_p3d65pq.png --cicp "" -o output_no_cicp.png` ("Preview" on a ~5 year old Macbook should suffice). --------- Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
6 participants
Description
This PR addresses the primary concern of #4678 -- implement support for reading and writing the
cICPchunk for PNGs.CICP (Coding Independent Code Points) is a means for using a tuple of integers to communicate characteristics of a variety of, traditionally, video encodings. The tuple is a series of four values that map to integer enumerations (codified in ITU-T H.273) representing a certain ColourPrimaries, TransferCharacteristics, and MatrixCoefficients, as well as a VideoFullRangeFlag.
In particular, CICP is used for describing HDR encodings to browsers and image viewers capable of displaying the image as intended. For example, if one were to write P3-D65 PQ-encoded RGB values to a PNG, it will only look "correct" if there's an appropriate
cICPchunk describing the primaries (14) and transfer function (16); without such metadata, PQ-encoded images will appear significantly darker and lower in contrast.Internally, CICP metadata is stored in a
int[4]typeCICPImageSpec attribute.This PR adds the following:
--cicpflag for setting, modifying, or removing CICP metadata for the top imagecICPchunk metadata <--> ImageSpecCICPTests
I've included tests. I've also embedded CICP metadata in the existing 16-bit test png in the testsuite.
But to see what this is all about with your own eyes, you can quickly convert a scene-linear AP0-encoded EXR to a PQ-encoded P3D65 HDR PNG with the following command:
$ oiiotool -i input_linap0.exr --ociodisplay:from=ACES2065-1 "ST2084-P3-D65 - Display" "ACES 1.1 - HDR Video (1000 nits & P3 lim)" --cicp "12,16,0,1" -o output_p3d65pq.pngIf you compare in a capable image viewer on a capable display the image produced with the above command compared to another one that lacks CICP metadata, you should see a pretty significant difference:
$ oiiotool -i output_p3d65pq.png --cicp "" -o output_no_cicp.png("Preview" on a ~5 year old Macbook should suffice).