Skip to content
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

Make display EOTFs in built-in transforms mirrored and unclamped to preserve sub-black and super-white #1968

Closed
nick-shaw opened this issue Apr 26, 2024 · 11 comments
Assignees
Labels
Feature Request New addition to OCIO functionality. Needs Discussion Needs discussion before implmentation, which could result in changes, or a decision not to proceed.
Milestone

Comments

@nick-shaw
Copy link

In broadcast scenarios it is necessary for all processing to pass both sub-black and super-white values. BT.2111 test patterns, for example, include PLUGE, and the ramp goes from -7 to 109%. Broadcast engineers expect conversion to pass these values, so a config for broadcast use cannot currently be created using the built-in XYZ-D65 to display transforms.

@carolalynn carolalynn added Feature Request New addition to OCIO functionality. Needs Discussion Needs discussion before implmentation, which could result in changes, or a decision not to proceed. labels Apr 29, 2024
@doug-walker doug-walker added this to the OCIO 2.4.0 milestone Jul 20, 2024
cozdas added a commit to autodesk-forks/OpenColorIO that referenced this issue Aug 27, 2024
…ns) and AcademySoftwareFoundation#1992 (LUT-free builtins Pt.1)

For issue AcademySoftwareFoundation#1992 - "Make LUT-free implementations of certain built-in transforms"
- Adding fixed-functions PQ_TO_LINEAR and LINEAR_TO_PQ. These will be used to handle following built-in transforms if the LUT support is turned off:
-- CURVE - ST-2084_to_LINEAR
-- CURVE - LINEAR_to_ST-2084
-- DISPLAY - CIE-XYZ-D65_to_REC.2100-PQ
-- DISPLAY - CIE-XYZ-D65_to_ST2084-P3-D65
- Implemented CPU renderers for scalar, SSE2 (with fastPower) and SSE2 (with Intel SVML) intrinsics targets for the new fixed-function.
- Implemented GPU shader generator for the new fixed-function
- Remaining fixed-functions will be implemented in an upcoming PR.

For issue AcademySoftwareFoundation#1968 - "Make display EOTFs in built-in transforms mirrored and unclamped to preserve sub-black and super-white"
- Existing LUT-based HLG curve is now unclamped at both ends and mirrored around origin.
- Both the existing LUT-based and the new LUT-free implementations of the PQ curve are now unclamped at both ends and mirrored around origin.

Aux Changes
- GetFixedFunctionCPURenderer() now takes a new bool parameter fastLogExpPow.
- Added a stand-in OCIO_LUT_SUPPORT preprocessor macro in preparation for ocio-lite where the lut support can be turned off.
- Added util functions to Config_tests.cpp to help creating configs with arbitrary version.
- Added ability to test fixed-function cpu renderers with and without fastLogExpPow.
- Added capability to specify custom extended ranges in the GPU unit tests. Default value is [-1.0,2.0] same as the previously hard-coded range.
- Fixed a bug in the GPU unit tests where the computed domain values would overshoot.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
cozdas added a commit to autodesk-forks/OpenColorIO that referenced this issue Aug 27, 2024
…ns) and AcademySoftwareFoundation#1992 (LUT-free builtins Pt.1)

For issue AcademySoftwareFoundation#1992 - "Make LUT-free implementations of certain built-in transforms"
- Adding fixed-functions PQ_TO_LINEAR and LINEAR_TO_PQ. These will be used to handle following built-in transforms if the LUT support is turned off:
-- CURVE - ST-2084_to_LINEAR
-- CURVE - LINEAR_to_ST-2084
-- DISPLAY - CIE-XYZ-D65_to_REC.2100-PQ
-- DISPLAY - CIE-XYZ-D65_to_ST2084-P3-D65
- Implemented CPU renderers for scalar, SSE2 (with fastPower) and SSE2 (with Intel SVML) intrinsics targets for the new fixed-function.
- Implemented GPU shader generator for the new fixed-function
- Remaining fixed-functions will be implemented in an upcoming PR.

For issue AcademySoftwareFoundation#1968 - "Make display EOTFs in built-in transforms mirrored and unclamped to preserve sub-black and super-white"
- Existing LUT-based HLG curve is now unclamped at both ends and mirrored around origin.
- Both the existing LUT-based and the new LUT-free implementations of the PQ curve are now unclamped at both ends and mirrored around origin.

Aux Changes
- GetFixedFunctionCPURenderer() now takes a new bool parameter fastLogExpPow.
- Added a stand-in OCIO_LUT_SUPPORT preprocessor macro in preparation for ocio-lite where the lut support can be turned off.
- Added util functions to Config_tests.cpp to help creating configs with arbitrary version.
- Added ability to test fixed-function cpu renderers with and without fastLogExpPow.
- Added capability to specify custom extended ranges in the GPU unit tests. Default value is [-1.0,2.0] same as the previously hard-coded range.
- Fixed a bug in the GPU unit tests where the computed domain values would overshoot.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
@doug-walker
Copy link
Collaborator

@nick-shaw could you please review PR#2029 to make sure the math is what you expect for the PQ and HLG functions?

@nick-shaw
Copy link
Author

The PQ and HLG curves appear to be correctly mirrored about zero to the best of my understanding (see my comment in #1969 regarding the OOTF). However since LINEAR_TO_PQ(0.0) != 0.0 I am not sure if there is a standard way to remove the discontinuity resulting from a simple mirroring. I have in the past used an approach similar to the ACES "stretch black" to join a mirrored PQ curve at zero. But there may be other approaches that I am not aware of.

@doug-walker
Copy link
Collaborator

@nick-shaw , thanks for having a look. Regarding your comment about 0, LINEAR_TO_PQ(0.0) does equal zero, so I'm not following you about a discontinuity at 0.

@nick-shaw
Copy link
Author

By my calculations is close to zero but not zero.
image
For input of zero the PQ value is c_1^m_2.

@doug-walker
Copy link
Collaborator

doug-walker commented Sep 13, 2024

@nick-shaw , what you wrote is true, but that is a PQ value of 0.836^78.84 = 7.36e-07. A single 16-bit code value in PQ would be 1/65535 = 1.53e-05. So we're talking about a value in PQ that is over 20x smaller than a single 16-bit code value. To me, that is 0, for all practical purposes. Are there any real-world situations you're aware of where that causes a problem?

@nick-shaw
Copy link
Author

No. I am not aware of any actual problem. I am just a pedant, for which I apologise!

@doug-walker
Copy link
Collaborator

Ha ha, don't apologize Nick! You raised a valid point. In fact, you made me realize that the CPU and GPU paths are giving slightly different results at 0 due to how the mirroring is done. It is way below visual threshold, but it still warrants a comment in the code to make people aware of the issue. So thank you again for raising the question.

@doug-walker
Copy link
Collaborator

doug-walker commented Sep 21, 2024

@nick-shaw , the remaining fix for the OOTF (and hence part of the EOTF) is in PR #2051.

Once these are merged, it would be great to get feedback on whether all your HLG issues are addressed.

zachlewis pushed a commit that referenced this issue Sep 26, 2024
…2029)

* Adsk Contrib - Issues #1968 (mirrored builtins) and #1992 (LUT-free builtins Pt.1)

For issue #1992 - "Make LUT-free implementations of certain built-in transforms"
- Adding fixed-functions PQ_TO_LINEAR and LINEAR_TO_PQ. These will be used to handle following built-in transforms if the LUT support is turned off:
-- CURVE - ST-2084_to_LINEAR
-- CURVE - LINEAR_to_ST-2084
-- DISPLAY - CIE-XYZ-D65_to_REC.2100-PQ
-- DISPLAY - CIE-XYZ-D65_to_ST2084-P3-D65
- Implemented CPU renderers for scalar, SSE2 (with fastPower) and SSE2 (with Intel SVML) intrinsics targets for the new fixed-function.
- Implemented GPU shader generator for the new fixed-function
- Remaining fixed-functions will be implemented in an upcoming PR.

For issue #1968 - "Make display EOTFs in built-in transforms mirrored and unclamped to preserve sub-black and super-white"
- Existing LUT-based HLG curve is now unclamped at both ends and mirrored around origin.
- Both the existing LUT-based and the new LUT-free implementations of the PQ curve are now unclamped at both ends and mirrored around origin.

Aux Changes
- GetFixedFunctionCPURenderer() now takes a new bool parameter fastLogExpPow.
- Added a stand-in OCIO_LUT_SUPPORT preprocessor macro in preparation for ocio-lite where the lut support can be turned off.
- Added util functions to Config_tests.cpp to help creating configs with arbitrary version.
- Added ability to test fixed-function cpu renderers with and without fastLogExpPow.
- Added capability to specify custom extended ranges in the GPU unit tests. Default value is [-1.0,2.0] same as the previously hard-coded range.
- Fixed a bug in the GPU unit tests where the computed domain values would overshoot.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - Renderer_PQ_TO_LINEAR_SSE high precision implementation is now limited to MSVC 2019+ compiler as it has the built-in intel _mm_pow_ps() SVML implementation.
- LUT-based PQ to Linear implementation now uses half-domain LUT to handle extended range. Linear to PQ direction was alredy using half-domain lut (probably due to normal input range being 0 to 100.
- BT_2100 namespace is renamed to HLG
- In BuiltinTransform_tests it's now possible (and necessary) to specify the error threshold for each built-in transform. This was needed as the hard-coded 1e-6 threshold was too tight for the PQ curve which uses nested power functions and thus numerical stability was depending on the implantation flavor.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - Reversing the forward direction of the PQ curve, now the forward direction is linear -> PQ . Changed the enum names accordinly, updated the tests etc.
- Adding a new Hybrid Log Gamma fixed function. Similar to the PQ curve, the forward direction is linear -> HLG. SIMD implementation is currently missing, can be done in the future.
- minor fixes in the PQ cpu op
- some adjustments to the CPU test values and thresholds.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - Ah! gcc and clang being more picky about the unused variable bit me again.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - Completed outstanding to-do items.
- Code beautification.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - Adding few missing pieces I noticed for PQ and HLG fixed functions
- Since we changed the default direction of the PQ and HLG fixed functions, I moved the order of the functions in the source files so that forward becomes before inverse.
- Adding the first cut of the FIXED_FUNCTION_LINEAR_TO_DOUBLE_LOG_AFFINE fixed function. This is still in WIP, needs some optimizations and tests implemented. Also to be able to handle the Apple Camera curve, it needs to be extended a bit. But currently it can handle the Canon CLog2 and CLog3 curves with all tests passing.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - unbreak gcc and clang compilers

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - Adding GpuShaderText::floatXGreaterThanEqual() functions which are needed for precise break-point handling in non-continuous piecewise functions.
- changes for double log gamma fixed function
-- Fixed couple of bugs
-- Added basic parameter validation
-- implemented fwd and inv gpu shaders
-- Fixed CLog2 and CLog3 double log gamma constants.
-- implemented cpu and gpu shaders

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - HLG fixed-function is now parametrized and the previously hard-coded Rec.2100 HLG curve parameters are now passed to the fixed-function by the built-in transforms and by some of the test suites.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - Adding a new parameter to the HLG fixed-function to control the point mirroring is applied. Below the mirror point the input values will be mirrored around that point and the output sign will be flipped. Although this does not guarantee by itself a monotonously increasing (thus invertible) and a continuous function, when the parameters are proper these properties will be achieved.
- Both the Rec.2100 HLG curve and Apple camera curve are now implemented by the HLG fixed-function when the LUT-support is turned off.

for now checking-in with LUT_SUPPORT turned off to run the CI tests on the implementation. I'll turn it on later.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - Unlike msvc, gcc and clang could not auto-determine the template parameter for the base class. I'm making it explicit, hope that it'll unbreak the builds.
- minor comment fix :)

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - gcc and clang are still failing. Removing the template for the HLG, hope this will un-confuse the compilers.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - handling the unused variable warning/error when the LUT-support is turned off.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - Fighting with more unused variable warning turned into error. At this stage I'm not sure that turning this warning on the compilers is a good idea or not. Almost all cases are simple unused variabled due to different cmake switches and will be happily optimized out by the compiler without any side-effect. I'm curious to see if that compiler flag helped anyone to catch a bug early on at all.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* silencing more unused vars.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - now that all the compiler are happy when the lut-support is turned off, turning it back on.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* - minor fixes, mostly formatting.

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>

* Refine enums

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

* Add opdata test

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

* Add hlg oetf builtin

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

* Update CTF version

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

* Add builtin version tests

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

* Fix typo

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

---------

Signed-off-by: cuneyt.ozdas <cuneyt.ozdas@autodesk.com>
Signed-off-by: Doug Walker <doug.walker@autodesk.com>
Co-authored-by: Doug Walker <doug.walker@autodesk.com>
@doug-walker
Copy link
Collaborator

This should be fixed via PR #1992. Assigning to @nick-shaw to confirm it meets his expectations.

@doug-walker doug-walker assigned nick-shaw and unassigned cozdas Sep 26, 2024
@nick-shaw
Copy link
Author

My testing confirms that with the new changes configs can be built for broadcast use which are able to pass sub-black and super-white values.

@doug-walker
Copy link
Collaborator

Thanks for checking Nick! Closing this as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request New addition to OCIO functionality. Needs Discussion Needs discussion before implmentation, which could result in changes, or a decision not to proceed.
Projects
None yet
Development

No branches or pull requests

4 participants