2021-10-04

October 4, 2021

Host: Michael Dolan

Attendees:

Apologies:

• Doug Walker

OCIO TSC Meeting Notes

• Process proposal: Require only 1 approver to merge into RB-* branches:

  • NOTE: Need quorum to make a decision on this, so just discussion for now. 

  • Michael: Propose requiring one approver for merges into RB branches instead of two to ease OCIO release manager procedure.

  • Patrick: Only applies to PRs cherry picked from master, so code review should have been done before. If PR is new development, will wait and follow usual rules.

  • Carol: No objections

  • Remi: Agree. Only risk is to merge something to break ABI

  • Patrick: Good point. Different from discussion, but discussed tool for checking ABI compatibility.

  • Michael: Topic came up in TAC meeting. Could have workflow to compare current and proposed build in PRs to RB branches.

  • Patrick: There is also a tool to check to check shader compatibility. Could be extremely useful for supporting testing of emitted shaders.

  • Rémi: Could that go in GPU test? What do you mean by compare?

  • Patrick: Doesn't run, just static analysis for shader code. Opens the door to validate more translations.

  • ABI checking tool: https://lvc.github.io/abi-compliance-checker/

  • TODO: Open issue about adding ABI compliance checker workflow.

• Rename master branch to main:

  • Michael: GH makes the transition simple with automatic link forwarding, etc: https://github.com/github/renaming

    • We could try to change the branch name in place, or create a main branch from master and retire master over time.

  • Patrick: Good to create main, and slowly retire master.

  • Michael: Also an opportunity to adjust overall branch strategy if needed. May not be, but could consider develop/main branches for example.

  • Remi: Already a lot of CI builds.

  • Kevin: Fewer branches better for me.

  • Patrick: Second that.

  • Michael: That sounds good. Main benefit of develop/main separation would be support for more CD workflows, but not needed.

  • TODO: Michael will research and propose main branch switch for next TSC meeting.

• PR #1500:

  • https://github.com/AcademySoftwareFoundation/OpenColorIO/pull/1500 

  • Patrick: Fixes a problem with creating PNG with ocionconvert. App wasn't saving color space metadata and defaulting to linear, so not working while reading the PNG. Talking with Larry to follow OIIO workflow. Only affects metadata and convention for tagging color space. What if we are tagging display/view, when there isn't a clear color space. One option to save display and view, another to store color space for resulting image, another could provide instructions for inverse, but could have issues with getting outdated. Seeing display and view isn't necessarily clear as to what color space the pixels are stored. Read the thread for conversation, but should be color space, which is more helpful. Can merge this branch, since first part is clear and it fixes a bug and backport to 2.1. If we need more time to discuss convention for display/view, we can wait.

  • Thomas: For context, it's using OIIO metadata, correct?

  • Patrick: Yes. Metadata is oiio:ColorSpace.

  • Kevin: Yes, not other image-specific metadata, text to say what color space we think it's in. It's a hint, not a concrete thing. In agreement the color space is useful in most cases. Display/view is more of an edge case, since it could help you invert, but more risky.

  • Carol: Color space names don't have to be standard either but more likely to be discernable.

  • Kevin: Exactly, especially if using the reference config. Makes sense to use color space since could be used to get to linear, but with display/view, can't be as sure.

  • Patrick: Tricky part is when you have look applied, which could change a lot of things.

  • Kevin: Almost zero of my display/view combinations are just color spaces. Most have looks. Look can be a 3d cube from someone. Might be worth looking at Nuke metadata for quicktimes. Tries to put foundry metadata in there to help decode, but needs supported app to work. Happy with patch going in PR.

  • Michael: Metadata tends to be most useful for a specific pipeline, where a studio can use it as needed. Harder to support all of those cases with generalized tools.

  • TODO: PR #1500 is approved for merge.

• PR #1496:

  • https://github.com/AcademySoftwareFoundation/OpenColorIO/pull/1496

  • Michael: Introduces new dependency on fast_float, and improves floating point localization.

  • Patrick: New dependency, but hardcoded in library in PR.

  • Rémi: Usually embedded dependency for modified library.

  • Michael: Says in PR it's amalgamated, but not certain if it's modified.

  • Kevin: Actual repo is header only. If we didn't want to include it, could use submodule to pull down or something.

  • Rémi: Think we should test it for performance, compatibility, etc.

  • Kevin: Provides amalgamated file in GH project. Apache 2 license. Is this the right code to be adding? Is this the right solution to solving locale issue? I know every month there's something better or faster, and eventually some of this will make it into standard library. Is the core idea is to solve locale parsing/serializing problem? Is this library a reasonable solution for that?

  • Patrick: Handles locale and improves performance, but doesn't resolve precision issues. Merge should not be done before testing to see if it's a performance improvement, or make sure not to break precision, etc. Was going to request unit tests to demonstrate improvement, and do local testing.

  • Rémi: Don't like that spi1d and spi3d are using different float to string conversion methods. What is the CLF format using for this conversion?

  • Patrick: Using C methods for performance, also suffering from locale issues. Can test and also fix CLF read/write if it's effective.

  • Rémi: stringstream is slow

  • Patrick: Did experiments on this a while ago and agree. The header file seems to be the same as upstream repo, so can use find module.

  • Kevin: Would prefer it to be external dependency. As long as it doesn't break or regress, ok with this library. Should make sure it doesn't change a lot. Doesn't have a lot of commits.

• OCIO 2.1.1:

  • Patrick: Since 2.1.0 we fixed many build breaks and other fixes. Finishing OSL improvements before doing 2.1.1. Interesting improvement. With latest change of OIIO and Imath, many cases where it doesn't build. Many fixes in PR, some already merged. Do we want to do ASAP 2.1.1 to contain build break fixes. Soon after have 2.1.2 with OSL improvement?

  • Michael: Isolating bug fixes to a release may be safer, and then update OSL.

  • Carol: Agree. Should be willing to do more frequent patches. Even at more common cadence, every few weeks, etc.

  • Patrick: Good point.

  • Rémi: Build breaks are a patch and OSL updates also a patch?

  • Patrick: The problem we have is VFX ref platform is 2.1.x. Moving only once a year. Why we rushed to have OSL in 2.1.0. Now to do minor or major update, need to wait a year.

  • Kevin: Doesn't stop us releasing, just stops apps from using the release.

  • Patrick: Same thing with OpenGL ES. Typically would be minor version but would have to wait a year to have it.

  • Rémi: Would that fail the ABI test?

  • Patrick: It's not an ABI break. It's an API addition.

  • Rémi: OpenJPEG uses ABI check in CI, and adding enum made the test fail.

  • Michael: Would be good to look at ABI compliance test to verify before releasing.

  • TODO: Confirm ABI compatibility before including OpenGL ES enum changes in 2.1.1.

  • Rémi: Should also avoid 2.1.1 release until expat issue resolved - see issue: https://github.com/AcademySoftwareFoundation/OpenColorIO/issues/1491