-
Couldn't load subscription status.
- Fork 644
feat(openexr): ACES Container hint for exr outputs #4907
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
Changes from all commits
d0f8853
73d1e75
fda980e
e89f01f
2deb446
ca010df
0b3967e
f98e2ce
2571f37
c9b40c0
ee84d2a
0f2feb5
9f23db7
7322ff1
c30d8f8
e94bd5d
5c3fd08
ac1c654
2052c52
1e89fa5
ae19a4c
c177142
25fac3c
d3530e4
21dfe1b
d725544
b61974a
5980084
60fba1b
9216912
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -290,6 +290,198 @@ set_exr_threads() | |
|
|
||
|
|
||
|
|
||
| static constexpr float ACES_AP0_chromaticities[8] = { | ||
| 0.7347f, 0.2653f, // red | ||
| 0.0f, 1.0f, // green | ||
| 0.0001f, -0.077f, // blue | ||
| 0.32168f, 0.33767f // white | ||
| }; | ||
|
|
||
| static const std::string ACES_AP0_colorInteropId = "lin_ap0_scene"; | ||
|
|
||
|
|
||
| bool | ||
| is_spec_aces_container_channels_only(const OIIO::ImageSpec& spec) | ||
| { | ||
| // Note: this is constructing and comparing sets, so that channel order | ||
| // doesn't matter. | ||
|
|
||
| // Allowed channel sets | ||
Glowies marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| static const std::vector<std::set<std::string>> allowed_sets | ||
| = { { "B", "G", "R" }, | ||
| { "A", "B", "G", "R" }, | ||
| { "B", "G", "R", "left.B", "left.G", "left.R" }, | ||
| { "A", "B", "G", "R", "left.A", "left.B", "left.G", "left.R" } }; | ||
|
|
||
| // Gather channel set from spec | ||
| std::set<std::string> channels(spec.channelnames.begin(), | ||
| spec.channelnames.end()); | ||
|
|
||
| // Compare to allowed sets (unordered) | ||
| for (const auto& allowed : allowed_sets) { | ||
| if (channels == allowed) { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
|
|
||
|
|
||
| bool | ||
| is_aces_container_attributes_non_empty(const OIIO::ImageSpec& spec, | ||
| std::string& non_compliant_attr) | ||
| { | ||
| // attributes in this list should NOT be empty if they exist | ||
| static const std::string nonEmptyAttribs[] = { | ||
| "cameraFirmwareVersion", | ||
| "cameraIdentifier", | ||
| "cameraLabel", | ||
| "cameraMake", | ||
| "cameraModel", | ||
| "cameraSerialNumber", | ||
| "comments", | ||
| "creator", | ||
| "lensAttributes", | ||
| "lensFirmwareVersion", | ||
| "lensMake", | ||
| "lensModel", | ||
| "lensSerialNumber", | ||
| "owner", | ||
| "recorderFirmwareVersion", | ||
| "recorderMake", | ||
| "recorderModel", | ||
| "recorderSerialNumber", | ||
| "reelName", | ||
| "storageMediaSerialNumber", | ||
| }; | ||
|
|
||
| for (const auto& label : nonEmptyAttribs) { | ||
| const ParamValue* found = spec.find_attribute(label, | ||
| OIIO::TypeDesc::STRING); | ||
| if (found | ||
| && (found->type() != TypeString || found->get_string(1).empty())) { | ||
| non_compliant_attr = label; | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
|
|
||
|
|
||
| bool | ||
| is_aces_container_compliant(const OIIO::ImageSpec& spec, std::string& reason) | ||
| { | ||
| if (!is_spec_aces_container_channels_only(spec)) { | ||
| reason | ||
| = "Spec channel names do not match those required for an ACES Container."; | ||
| return false; | ||
| } | ||
|
|
||
| // Check data type | ||
| if (spec.format != OIIO::TypeDesc::HALF) { | ||
| reason | ||
| = "EXR data type is not 'HALF' as required for an ACES Container."; | ||
| return false; | ||
| } | ||
|
|
||
| // Check compression | ||
| std::string compression = spec.get_string_attribute("compression", "zip"); | ||
| if (compression != "none") { | ||
| reason = "Compression is not 'none' as required for an ACES Container."; | ||
| return false; | ||
|
Comment on lines
+384
to
+395
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the best policy here? For something like "doesn't have the right channels", the only thing to do is fail. But for "wrong data type" or "wrong compression", we could force compliance by just fixing it. |
||
| } | ||
|
|
||
| // Check non-empty attributes | ||
| std::string non_compliant_attr = ""; | ||
| if (!is_aces_container_attributes_non_empty(spec, non_compliant_attr)) { | ||
| reason = "Spec contains an empty string attribute ("; | ||
| reason += non_compliant_attr; | ||
| reason += ") that is required to be non-empty in an ACES Container."; | ||
| return false; | ||
| } | ||
|
|
||
| // Check attributes with exact values if they exist | ||
| if (spec.get_string_attribute("oiio:ColorSpace", ACES_AP0_colorInteropId) | ||
| != ACES_AP0_colorInteropId | ||
| || spec.get_string_attribute("colorInteropId", ACES_AP0_colorInteropId) | ||
| != ACES_AP0_colorInteropId) { | ||
| reason | ||
| = "Color space is not lin_ap0_scene as required for an ACES Container."; | ||
| return false; | ||
| } | ||
|
Comment on lines
+408
to
+415
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want "no color space known" to pass? Or fail? The way to make "none specified" to fail is to not supply default values to the get_string_attribute calls. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'd like to get @antond-weta's opinions on that. I had assumed that we want the "no color space known" case to pass the compliance check. (ie. If there is no attribute indicating a color space, it passes the ACES Container compliance check, and we just continue processing the image assuming it is in linear AP0 space) |
||
|
|
||
| if (spec.get_int_attribute("acesImageContainerFlag", 1) != 1) { | ||
| reason | ||
| = "acesImageContainerFlag is not set to '1' as required for an ACES Container."; | ||
| return false; | ||
| } | ||
|
|
||
| // Check chromaticities | ||
| float chromaticities[8] = { 0., 0., 0., 0., 0., 0., 0., 0. }; | ||
| bool chroms_found | ||
| = spec.getattribute("chromaticities", | ||
| OIIO::TypeDesc(OIIO::TypeDesc::FLOAT, 8), | ||
| chromaticities); | ||
| bool chroms_equal = std::equal(std::begin(chromaticities), | ||
| std::end(chromaticities), | ||
| std::begin(ACES_AP0_chromaticities)); | ||
|
|
||
| if (chroms_found && !chroms_equal) { | ||
| reason | ||
| = "Chromaticities are not set to AP0 chromaticities as required for an ACES Container."; | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
|
|
||
|
|
||
| void | ||
| set_aces_container_attributes(OIIO::ImageSpec& spec) | ||
| { | ||
| spec.attribute("chromaticities", OIIO::TypeDesc(OIIO::TypeDesc::FLOAT, 8), | ||
| ACES_AP0_chromaticities); | ||
| spec.attribute("colorInteropId", ACES_AP0_colorInteropId); | ||
| spec.attribute("acesImageContainerFlag", 1); | ||
| } | ||
|
|
||
|
|
||
|
|
||
| bool | ||
| process_aces_container(OIIO::ImageSpec& spec, std::string policy, | ||
| int acesImageContainerFlag, | ||
| std::string& non_compliance_reason) | ||
| { | ||
| bool treat_as_aces_container = policy == "strict" | ||
| || acesImageContainerFlag == 1; | ||
| bool is_compliant = is_aces_container_compliant(spec, | ||
| non_compliance_reason); | ||
|
|
||
| if (treat_as_aces_container && !is_compliant) { | ||
| return false; | ||
| } | ||
|
|
||
| set_aces_container_attributes(spec); | ||
|
|
||
| if (policy == "relaxed" && !is_compliant) { | ||
| // When image is not compliant in relaxed mode, we should avoid | ||
| // setting the flag, and we should print a warning | ||
|
|
||
| // TODO: When we have a way to report warnings, report one here | ||
| // to indicate that the given image spec is not compliant | ||
| spec.erase_attribute("acesImageContainerFlag"); | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
|
|
||
|
|
||
| OpenEXROutput::OpenEXROutput() | ||
| { | ||
| pvt::set_exr_threads(); | ||
|
|
@@ -812,6 +1004,26 @@ OpenEXROutput::spec_to_header(ImageSpec& spec, int subimage, | |
| Imf::LevelMode(m_levelmode), | ||
| Imf::LevelRoundingMode(m_roundingmode))); | ||
|
|
||
| // Check ACES Container hint | ||
| int aces_container_flag = spec.get_int_attribute("acesImageContainerFlag", | ||
Glowies marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 0); | ||
| std::string aces_container_policy | ||
| = spec.get_string_attribute("openexr:ACESContainerPolicy", "none"); | ||
|
|
||
| if (aces_container_policy != "none" || aces_container_flag == 1) { | ||
| std::string non_compliance_reason = ""; | ||
| bool should_panic = !process_aces_container(spec, aces_container_policy, | ||
| aces_container_flag, | ||
| non_compliance_reason); | ||
|
|
||
| if (should_panic) { | ||
| errorfmt( | ||
| "Cannot output non-compliant ACES Container in 'strict' mode. REASON: {}", | ||
| non_compliance_reason); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| // Deal with all other params | ||
| for (const auto& p : spec.extra_attribs) | ||
| put_parameter(p.name().string(), p.type(), p.data(), header); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this is not clear about what happens if the spec is passed with a colorInteropID that is set, but is not
lin_ap0_scene. Are you saying that it will be changed tolin_ap0_scene(but pixels stay as they are)? Or is this something that will be rejected as non-compliant? Or does the behavior differ depending on the policy setting?