fix(oiiotool): be cautious promoting to float due to color space name with --autocc#5192
Conversation
zachlewis
left a comment
There was a problem hiding this comment.
I feel like we should always promote to float whenever converting to a linear space; and if autocc always converts to the scene_linear role, then autocc should always promote to float.
Alternatively, we can use ocio color space "encoding" attributes, if present, as a fast heuristic for determining whether to promote to float ("scene-linear" and "display-linear") versus integer-based encodings... for "log" and "hdr-video", it's probably best to promote to uint16, and for anything else, it's probably best to keep it same-as-source.
(note: this ties into whether or not we should promote video-range to full-range as well. We musn't convert a video-range RGB encoding to a full-range RGB encoding without promoting the image buffer data type; but we can get away with deferring that conversion to the last possible moment, within the OCIO graph)
|
(But I do agree, just because a color space is "float32" doesn't mean it is! And if that's all you want to address in this PR, I'll happily approve) |
In fact, oiiotool does convert nearly everything to float for the internal representation and math precision/speed. (The The problem in particular I'm trying to solve here is a file that comes in as uint8 or uint16, or for that matter as an openexr half, and then written as openexr. OBVIOUSLY all of those cases should be written as half. But a color transform to any of the spaces described in the spec as "float32" were thinking they needed to save as full float. Disk space costs real money!
autocc converts to scene_linear upon input, but then upon output, converts to whatever the filename implies. But again the question here is what the output is saved as, and we don't want things promoted to the widest, most badly compressible data type unless it's explicitly requested. If we were truly starting fresh, I would say that autocc and a color space name in the filename should only imply a color space transform, and the data type should be entirely a function of what the input was, and any BUT... at my place, anyway, the fact that OCIO 1.x had different names per color space and per data type meant that a culture developed of being able to say We have a lot of scripts and knowledge inside people's heads with this expectation. And as OCIO 2.x specs are coming for us soon, I don't want everything to get silently promoted to 32 bit float upon save, ick! |
|
Oooooohhhh... I see, you're parsing the output filename for a color space (and bitdepth) as a means for determining an ultimate color space transform (or, at least, to inform the container bitdepth). That makes sense to me, and I understand what you're doing here. Alrighty, in that case, this looks good to me. FWIW, I think a lot of pipelines use bitdepth token hints similar to SPI's ("f", "h", "8", "10", "12", "16"), cuz all the early OCIO documentation uses those idioms. (At my studio, at some point, someone decided "f" stood for "half-float" and "ff" stood for "full-float", and it drove me crazy!) Anywqy, I approve! |
Almost. I'm (I mean OCIO is) parsing for a color space name. Each color space in the config has a data type associated with it. That's what's being used. OCIO 1.x configs tended to have different (even if mathematically equivalent, in theory) color spaces for different bit depths because it had to, since it was mostly LUT based. OCIO 2.x configs, which are almost always procedural rather than LUT based, has all the interesting/standard color spaces marked as "float". It's only a coincidence that my example used a color space name that had digits in it that corresponded to the bit depth in the config. But I am not parsing for that per se. It's not a necessary ingredient in the unfortunate behavior I'm trying to address here. |
Hahaha. That kind of makes my day. |
|
There's a lot of legacy behavior and expectations in play. The tool that was used here before OIIO took over was 100% oriented around all filenames having color space names embedded (as you say, things like lnf, lnh, vd8, lg10, etc.) and therefore having, when combined with the OCIO (or pre-OCIO thingie) config, all the information it needed from the filename for { file format, color space, data type, bit depth }. When OIIO/oiiotool came around and started taking over, the culture around this was already set, and in fact More historical trivia: That pre-oiio command line tool was called "ginsu", and was really just the CLI interface to all the 2D image processing of Katana (long before it was licensed to Foundry to become that product), which itself started off as our new compositing package, not a lighting tool at all. It was kind of hijacked for lighting because its workflow seemed well suited for that as well and we had outgrown the previous lighting system, a show really needed that. By the time the team was ready to return to finishing the compositing functionality that was the original reason for writing Katana, Nuke existed and was becoming popular, so we just switched to that and never really continued developing Katana as a full compositing tool. |
Background: OCIO 1.x configs were LUT based and so tended to have different color space names for different data types/bit depths. We capitalized on this to let --autocc also select type/bits, but this trick doesn't work so well for OCIO 2.x configs with procedural (non-LUT) transformations that all look like "float". We don't want to promote to float just because of this! Solution: When using color space data type (nearly always float for new configs and certainly the built-in configs) to infer output, DO NOT use "float" as a real request. Treat it as not being a specific type, don't let it override the intended output type for --autocc. (But a color space still marked as a particular integer bit depth is still an --autocc inferred request.) Signed-off-by: Larry Gritz <lg@larrygritz.com>
|
FWIW, I still think it would be pretty useful if OIIO had an "in-house" idiom for describing the color space and the bit depth at the same time, even if not explicitly specified in an OCIO config. I'd need to think about how to do this without making it too contrived or surprising, but having a way to resolve a "color token" to a short, safe color space alias + a bitdepth hint, and vice versa, would be insanely useful for folks that aren't using color spaces with explicit bitdepths and equality-grouped-variants of the same color space allocated (or just tagged) for different bit depths. I'm not sure what the most elegant solution is for OIIO, but I really wouldn't mind some kind of dumb rule like testing a string for a regex match to |
Background: OCIO 1.x configs were LUT based and so tended to have different color space names for different data types/bit depths. We capitalized on this to let
oiiotool --autoccalso select type/bits, but this trick doesn't work so well for OCIO 2.x configs with procedural (non-LUT) transformations that all look like "float". We don't want to promote to float just because of this!Solution: When using color space data type (nearly always float for new configs and certainly the built-in configs) to infer output, DO NOT use "float" as a real request. Treat it as not being a specific type, don't let it override the intended output type for --autocc. (But a color space still marked as a particular integer bit depth is still an --autocc inferred request.)