Native C++ AMF reader (Config::CreateFromAMF) — ACES 1.x + ACES 2, rebased from AJA, validated bit-exact#1
Open
giardiello wants to merge 10 commits into
Open
Native C++ AMF reader (Config::CreateFromAMF) — ACES 1.x + ACES 2, rebased from AJA, validated bit-exact#1giardiello wants to merge 10 commits into
giardiello wants to merge 10 commits into
Conversation
Import the AMFParser C++ implementation, public API, and unit tests from the AJA amf_to_config branch: https://github.com/chandyn/OpenColorIO/tree/amf_to_config This squashes 38 commits from AJA into a single baseline that applies cleanly onto current main. It adds: - CreateFromAMF(): builds an in-memory OCIO Config from an AMF file - An expat-based AMFParser (fileformats/amf/AMFParser.{h,cpp}) - The public AMFInfo struct (OpenColorTypes.h) - Unit tests and AMF test data Design guidance on the original work came from Doug Walker and Bill Bowen. Tracks issue AcademySoftwareFoundation#1927 (Add a C++ implementation for ACES Metadata File support). Co-authored-by: Chandy Navaratan <chandy@raje.sh> Co-authored-by: Bill Bowen <bilbo@aja.com> Signed-off-by: giardiello <giardiello@me.com>
Phase 0 (baseline hardening) + Phase 1 (config-agnostic ID resolution): - Fix unused-variable warning in HandleLookTransformStartElement. - Fix latent workingLocation bug: m_numLooksBeforeWorkingLocation was a size_t with a -1 sentinel, so the "< 0" checks never fired. Changed to a signed type, which also clears the sign-compare warnings. - Add ElementMatchesTransformId() which matches an ACES Transform ID against the OCIO 2.5+ "amf_transform_ids" interchange attribute, falling back to description-substring matching for older configs. searchColorSpaces, searchViewTransforms and searchLookTransforms now use it. - m_useAmfIds is set from the reference config profile version (>= 2.5). Signed-off-by: giardiello <giardiello@me.com>
Phase 2 (remove hardcoded config/version assumptions): - The reference config is now chosen after parsing: if the AMF references any ACES v2.0 transform IDs, the ACES 2 studio builtin config is used; otherwise the ACES 1.3 studio config. An explicit config path still overrides this. - initAMFConfig resolves the standard color spaces (scene_linear, color_timing, cie_xyz_d65_interchange, data) through roles so it works with configs that name them differently (e.g. the ACES 2 CIE-XYZ-D65 space). - The generated config version now matches the reference config version instead of a hardcoded 2.3, so ACES 2 color spaces validate. - Clear interop IDs on color spaces copied from the reference config (their interop targets are not present in the minimal generated config), re-pointing AMFInfo names afterwards. Adds an ACES 2 end-to-end unit test (S-Log3 -> RGC look + CDL -> P3-D65 SDR). Signed-off-by: giardiello <giardiello@me.com>
Phase 4 (idiomatic public API): add a static Config::CreateFromAMF() factory mirroring CreateFromFile/CreateFromBuiltinConfig, so callers can build an AMF config the same way as any other config. The existing free CreateFromAMF() function is retained. The ACES 2 unit test now exercises the new factory. Signed-off-by: giardiello <giardiello@me.com>
Phase 5 (Python bindings): - Bind the AMFInfo struct as a read-only type exposing clipIdentifier, clipColorSpaceName, inputColorSpaceName, numLooksApplied, displayName and viewName. - Add Config.CreateFromAMF(amfFilePath, configFilePath="") which returns a (Config, AMFInfo) tuple (Python has no out-parameters). Verified from Python: an ACES 2 AMF auto-selects the ACES 2 studio config and produces a working processor, while an ACES 1 AMF selects the ACES 1.3 config. Signed-off-by: giardiello <giardiello@me.com>
Phase 6 (tests + robustness): - Add AMFTest.py covering the ACES 2 path, the ACES 1 path and the missing file error, registered in the Python test suite. - Throw when the AMF file cannot be opened, instead of silently producing a degenerate config (matches Config::CreateFromFile behaviour). - Resolve the ACES2065-1 color space directly by name when the AMF has no input transform (it cannot be found via amf_transform_ids). Signed-off-by: giardiello <giardiello@me.com>
Replace the remaining hard tabs (inherited from the original AJA branch) with spaces in AMFParser.cpp and AMFParser_tests.cpp, per the OpenColorIO coding style guide. Signed-off-by: giardiello <giardiello@me.com>
The Academy is developing an official ACES AMF library (Python today, with a C++ version planned). Add a file-level map plus inline NOTE(aces-amf) anchors marking exactly which parts of the reader could later be delegated to that library (XML parsing, the intermediate model, and schema/semantic validation) and which parts are OCIO-specific and would stay (the AMF -> Config mapping and transform-ID resolution against the OCIO config). No behavior change. Signed-off-by: giardiello <giardiello@me.com>
…pliant Prototyped the documented integration boundary end-to-end against the official Python aces-amf-lib v0.1.0: parse -> object model -> this reader's mapping logic -> OCIO processor. Confirmed the library exposes every field process() needs as typed values, and recorded the accessor shape plus a key tradeoff (the library is AMF v2.0-only and strictly validates required fields, whereas this expat reader is more permissive and also reads v1.0). The validation revealed the aces2 test fixture was missing v2.0-required fields (amfInfo/pipelineInfo/clipId uuid and dateTime); add them so the fixture is spec-compliant and parses under the official library's schema validation. Signed-off-by: giardiello <giardiello@me.com>
The slogtopq_1.clf / slogtopq_wlook_1.clf fixtures inherited from the AJA baseline are ~7.7 MB baked 3D LUTs (274k lines each) that dominated the diff. Untrack them and add a .gitignore entry; they remain on disk for the local slogtopq* tests. Small, self-contained fixtures should replace them before any upstream submission. Signed-off-by: giardiello <giardiello@me.com>
|
@giardiello - Notes
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch turns the ACES Metadata File (AMF) reader into a first-class, in-process OCIO primitive:
Config::CreateFromAMF()(plus a freeCreateFromAMF()and Python bindings) that reads an AMF and returns a live OCIOConfigimplementing the AMF viewing pipeline, along with anAMFInfodescribing how the pipeline maps onto the config.It starts from the AJA prototype on
chandyn/OpenColorIO@amf_to_config(tracked by upstream issue AcademySoftwareFoundation#1927), rebased cleanly onto currentmain(OCIO 2.6-dev), then extended for ACES 2 / OCIO 2.5+, given Python bindings, tests, and bit-exact numerical validation against the referencepyocioamftool.Authorship: the baseline import commit is authored by giardiello with
Co-authored-by:credit to Chandy Navaratan and Bill Bowen (AJA); design guidance from Doug Walker. All commits are DCO signed-off.Two independent version axes (important, easy to conflate)
amf:v1.0vsamf:v2.0,SOPNodevsASC_SOP. Drives parsing.transformId:v1.5:(ACES 1.x) vs:v2.0:(ACES 2.x). Drives which reference config we resolve against.They are orthogonal: an
amf:v2.0file can carry ACES-1.5 transforms. Config selection is keyed on the ACES transform version, never the schema namespace (verified — anamf:v2.0+:v1.5:file correctly routes to the ACES 1.3 config).What changed (by commit)
main. Expat-based parser, publicAMFInfo,CreateFromAMF(), unit tests + data.amf_transform_ids—ElementMatchesTransformId()matches against the OCIO 2.5+getInterchangeAttribute("amf_transform_ids"), with description-substring fallback for older configs. Also fixes a latentsize_t/-1sentinel bug inm_numLooksBeforeWorkingLocationand clears warnings.:v2.0:transform IDs, use the ACES 2 studio builtin, else the ACES 1.3 studio builtin (explicitconfigFilePathstill overrides). Standard color spaces resolved via roles (works with configs that name e.g. CIE-XYZ-D65 differently); generated config version matches the reference config; interop IDs on copied color spaces are cleared before validation (andAMFInfonames re-pointed). Adds an ACES 2 end-to-end unit test.Config::CreateFromAMFfactory — mirrorsCreateFromFile/CreateFromBuiltinConfig.AMFInfotype +Config.CreateFromAMF(amfFilePath, configFilePath="")returning a(Config, AMFInfo)tuple.AMFTest.py(ACES 2, ACES 1, missing-file), throw on unopenable file, fetch ACES space by name for no-input AMFs.aces-amfswap points — file-level map +NOTE(aces-amf)anchors marking what a future official ACES AMF C++ library could replace (XML parse/model/validation) vs. what stays OCIO-specific (the AMF→Config mapping).aces-amf-libswap points + spec-compliant fixture — prototyped the boundary end-to-end against the official Pythonaces-amf-libv0.1.0; made the test fixture v2.0-spec-compliant (required uuid/dateTime).Validation (bit-exact vs
pyocioamf)Cross-checked our
CreateFromAMFconfig output against the establishedpyocioamfreference (baked CTF) on identical inputs::v2.0:transforms):v1.5:transforms)aces-v1.3-v2.0config viaconfigFilePathBoth exercise the full pipeline (input CSC/IDT → RGC look → CDL working-space sandwich → ODT / combined RRTODT), proving transforms are correctly applied, not merely resolved. All C++
AMFParsertests (3) and PythonAMFTestcases (3) pass.Relationship to the official ACES AMF library
The Academy's
aces-amflibrary (Python today atampas/aces-amf; a C++ version is planned) parses/validates/authors AMF. It would replace only the front-end (XML parse → object model + validation); the AMF→OCIO mapping (~70% of this code) stays ours regardless. Validated againstaces-amf-libv0.1.0: its object model exposes everything our mapping needs. Tradeoff: the library is currently AMF-schema-v2.0-only and strictly validates required fields, whereas this expat reader is more permissive and also reads schema v1.0. Swap points are annotated in-code withNOTE(aces-amf).Design notes / open decisions
CreateFromAMFcould default to it and drop auto-switching.AMFInfokept as the AJA struct (stable); an RcPtr-with-accessors rework is a public-API decision to defer to upstream discussion.Test plan
test_cpu_exec --run_only AMFParser(ACES 1 + ACES 2 + working-location)AMFTest(ACES 2, ACES 1, missing-file error)pyocioamf(ACES 1 and ACES 2)Path to upstream (when/if we choose)
Requires: EasyCLA signed; Core Library Changes process via issue AcademySoftwareFoundation#1927 (design proposal + TSC); 2 Committer approvals + green CI; final PR linked to AcademySoftwareFoundation#1927. Not started.
Made with Cursor