Replies: 22 comments 11 replies
-
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. |
Beta Was this translation helpful? Give feedback.
-
@zachlewis said in #4746: 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? |
Beta Was this translation helpful? Give feedback.
-
@JGoldstone said: 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. |
Beta Was this translation helpful? Give feedback.
-
@JGoldstone said: 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. |
Beta Was this translation helpful? Give feedback.
-
@zachlewis said: Fantastic feedback, Joe [sic: Joseph]. 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. |
Beta Was this translation helpful? Give feedback.
-
@zachlewis said: 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. |
Beta Was this translation helpful? Give feedback.
-
@lgritz said: 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. |
Beta Was this translation helpful? Give feedback.
-
@zachlewis said: 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 |
Beta Was this translation helpful? Give feedback.
-
@lgritz said: 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? |
Beta Was this translation helpful? Give feedback.
-
@zachlewis said: 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:
|
Beta Was this translation helpful? Give feedback.
-
@lgritz said:
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 |
Beta Was this translation helpful? Give feedback.
-
@zachlewis said: 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 |
Beta Was this translation helpful? Give feedback.
-
@lgritz said:
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. |
Beta Was this translation helpful? Give feedback.
-
@JGoldstone said: Catching up (my you've both been energetic in the last 72 hours or so)
|
Beta Was this translation helpful? Give feedback.
-
@lgritz said:
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. |
Beta Was this translation helpful? Give feedback.
-
@JGoldstone said: 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. |
Beta Was this translation helpful? Give feedback.
-
@lgritz said:
Oh yes, we already do that. They all have namespaced attributes "formatname:attributename". The ImageInput sometimes sets them to reveal certain settings of the file that was read from, but these do not necessarily describe the data read (for example, maybe "blorgformat:encoding" might be "YCbCr" to indicate that was what was stored in the file, but all the data returned from the ImageInput::read_image is RGB as OIIO requires). An ffmpeg example is here. And ImageOutput format implementations often will recognize several items in this form that aren't metadata of the image per se, but are interpreted as hints to control the writer (tiff example to control 'rowsperstrip' here). Generally, writers are expected to ignore any hints of the form "format:attributename" where the namespace 'format' is the name of a different image format. For examples, the EXR writer will ignore things like "tiff:rowsperstrip" because it knows "tiff" is the name of another format, so it correctly understands this to be a hint for the TIFF writer, and NOT a piece of metadata to add to the OpenEXR file's attributes. EXR's logic for that is here |
Beta Was this translation helpful? Give feedback.
-
@zachlewis said:
Ah, I'm picking up what you're putting down. I was thinking, it would be useful to be able to inspect the various attributes after OCIO-concerned-IBAs -- but if it makes things easier, we can do without.
We won't be able to use "oiio:ColorSpace" to carry all information forward -- "MDCV" metadata will likely only be set by forwards Still, I would imagine that any existing attribute-specific values would take precedence over the "oiio:ColorSpace" value, cuz if those values exist by the time we're writing to disk, it means those values are either correctly being carried forward from the input, or they've been manually set by the user.
That's interesting that CCV also defines the content primaries + white; that seems redundant, if CICP is meant to convey that information. I do know that MaxFALL (Frame Average Light Level) and MaxCLL (Content Light Level) metadata are static -- the values are supposed to be constant throughout all frames of the program. I found this excellent article from the SMPTE Motion Imaging Journal on how to compute the values while rejecting outliers: https://ieeexplore.ieee.org/stamp/stamp.jsp?arnumber=9508136
I assume you mean scene-referred imagery (unless I'm misunderstanding?), but you're not wrong -- CICP is explicitly a convention for display-referred imagery -- isn't equipped to provide downstream imagers or processes with any information regarding how to transform a scene-referred image state for viewing. On the other hand, typically for log-encoded data, I don't think we'd want CICP interfering with how the values are consumed or displayed downstream. (That said, I love that ARRI's log-encoded ProRes videos can contain extractable LUTs in the metadata. I don't know of anyone else who does that, but I wish it were common practice). That's part of the reason why an OCIO config author needs to be able to selectively prioritize when / how / whether to use CICP for identifying an input file's color space (relative to the other means of doing so -- like using a filename!) In practice (at Company 3 / Method, anyway), I think the only part of NCLX metadata considered meaningful for ingesting / converting log-encoded videos is the full range flag (if that). But I'm still with you, Joseph -- it would be helpful if CICP additionally codified the various scene-referred gamuts and transfer functions. In theory, one could define an "in-house" convention with OCIO-2.5 (which is part of the reason why OCIO-2.5 will not validate CICP tuples)
I believe Chris (@digitaltvguy) can clarify. I've been referring to this as
Yes, that's exactly what I had in mind -- in fact, I started putting together a proof of concept doing exactly that. Alternatively, we could define an extremely minimal OCIO config, and just use OCIO-2.5's config merging abilities to merge with one of OCIO's builtin configs, but we'll probably have to hold off on that approach until OIIO raises the minimum OCIO version to 2.5 in three years or whenever.
To be clear, we'll be doing something on top of one of OCIO's built-in configs as a default; and we'll leave user-set OCIO configs alone. I'm gonna bring this up in another issue / discussion, and better explain why I think this is a good idea. The idea is to fail over to something useful that ships with OCIO, but with inoffensive OIIO-specific tweaks to serve OIIO-specific needs (as "ocio://default" doesn't really provide a helpful out-of-the-box experience for a lot of very common use cases -- e.g., "camera log" --> "scene linear" conversions)
In other words, we'd be moving OIIO's hardcoded conventions (like the "Linear" and "sRGB" color space aliases) and preferences (like failing over to "lin_rec709" as a default color space) to OIIO's built-in OCIO config, thereby making OIIO itself behave more reliably, as intended, with user-provided OCIO configs.
This does get to the core of the matter. The third CICP component, the "color matrix", specifically concerns the various non-RGB encodings. If OIIO always converts to and from RGB whenever reading / writing, that make life a lot simpler for us. That means any explicit scaling for luma-chroma encodings will be handled by the plugin at conversion time. |
Beta Was this translation helpful? Give feedback.
-
@lgritz said:
My assumption was just that post OCIO-concerned-IBA execution, most of those attribs would be invalidated anyway, but I am happy to be corrected. I'm hoping to avoid a situation where any function that receives an IB (that may have gone though unknown steps to get there) doesn't need to recreate the full logic of "if this thing is set, it means this, but if not then look for this other thing, and it means that...". My dream was that it could all be one attribute used through the whole chain for the real decisions, with everything else merely being documentation of what was found in the original file (and quickly invalidated any time the meaning of the pixel values could break what the file documented).
You're easily showing just by using terminology that I don't know that I'm way out of my depth here on the color pipeline issues. Take my rambling as only somebody with an eye on whether the APIs and conventions will be easy to document, use, and maintain, but I am relying on others to make the real decisions about a sensible color management workflow for OIIO and you should not hesitate to override me if I'm not accounting for what we need to have a correct and complete solution.
A lot of this dates back to before OCIO was required and before it had built-in configs or any kind of name standardization, just me (not a color scientist) trying to grope for behavior that's likely to be right a lot of the time and notation that's easy for lay users to remember. I recognize that both "Linear" and "sRGB" are imprecise and ambiguous, and am happy for us to change the set of aliases, defaults, and behaviors. Don't consider any of that to be set in stone (though it's a bonus if the things naive users "try" are likely to do more or less what they expect).
So far, our convention has been that the default behavior is always to convert to some RGB-based system upon read, but often there is a back door (using open-with-configuration-hint "oiio:RawColor") to mean "don't do any conversion to RGB, just give me whatever channel encoding was in the file, and I (the caller of OIIO) will be responsible for sorting it out." That is not to say we are averse to providing some freestanding utility functions (or IBA?) that will convert among common choices or understand how they map to CICP (not CICF! I have the contact lenses in today) values. |
Beta Was this translation helpful? Give feedback.
-
(Whew, I did not fully appreciate how much we've been talking and writing about this until I had to paste it message by message!) |
Beta Was this translation helpful? Give feedback.
-
@zachlewis In this discussion, do you think you can pull together the current consensus and your best guess at the way to proceed and express it as a simple checklist of specific changes we can start implementing and checking off one by one on the way to a 3.1 release fairly soon? As an example, the first few items might be things like (these are examples, not necessarily correct):
|
Beta Was this translation helpful? Give feedback.
-
Quick question about autocc / input filepaths... Namely -- OIIO doesn't have an official mechanism for internally storing input image filepaths in an ImageSpec attribute, correct? I think something like an "oiio:InputFilepath" attribute could be useful in other cases, too. Nuke has a similar idiom -- each Read node passes down "input/filename" metadata, which often informs downstream OCIO transform context variables or slate / burnin text values, frame-to-smpte:Timecode translations, etc. The specific reason why I ask is, I think it might be nice to defer "autocc" behavior (i.e., the mechanism by which an OCIO color space is parsed from a filepath) as late as possible, ideally to the same "interpretColorSpaceFromMetadata" function that we'd use just before applying a color-managed (i.e., OCIO-color-space-aware) IBA. One reason for this is, according to the current OpenEXR / CIF proposal, the file path is considered after acesImageContainerFlag and colorInteropId metadata, if they exist in the header. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
This discussion thread will be for discussing and working out color space metadata conventions: OCIO color space names, CICP, chromaticities, and other info. Our goal is to work out a spec describing what metadata names and conventions ImageInputs use, how ImageBufAlgo functions that do color space things should understand and modify the metadata, and how ImageOutputs should act on what is found when it's time to write output.
Unfortunately, a lively discussion on this was in the comments of PR #4746. I will try to cut & paste the important parts of that content below, and then (after I give the all-clear) we can pick up the discussion here and hash through the rest of the details.
Beta Was this translation helpful? Give feedback.
All reactions