Per-Track and Per-Clip Color

[semagnum]

Fixes #1614

Summarize your change.

Allow colors to be assigned per track and clip for organizational and tagging purposes. This change contains minimal API changes (colors are optional on both clip and track schemas). This work was part of ASWF Dev Days, thanks for the opportunity!

The new Color class with Python bindings also includes:

• Static premade colors for convenience (used list of colors specified in Marker::Color for reference)
• Utility functions to convert to different color formats: hex code, agbr integer (for Premiere), list of doubles, list of integers (given a base, e.g. 8-bit, 16-bit)

Reference associated tests.

• updated tests/baselines/empty_clip.json to compensate for new color entry
• new test_color.py test file
• I re-ran the clip and track tests, they seem to be are passing on my end :)

To Discuss

• Color uses doubles to play nice with the serializer since it does not support reading/writing floats. While the extra precision won't hurt anything, some may find it more reasonable to be stored as floats (in which case, we'd need to find a reliable way to support converting to/from doubles for the serializer). As Nick mentioned on Slack, OTIO doesn't have a standard regarding float precision and roundtripping of values. I added a docstring for the pybind's docstring to only expect accuracy within 1/255 of the intended value.
• I am aware of Marker::Color relying on string names instead of values (see Marker RGB colors #1873). Since integrating there would require a more drastic API change (not to mention more rigorous testing), I figured it was beyond scope. But future API changes and adapters should definitely take advantage of this class.
• I use named the utility functions from_float_list() and to_float_list() since Python does not differentiate between floats and doubles just for clarity. But let me know if you'd prefer to use double in the name, or have it be different between Python and C++.

[jminor]

Code-wise, this looks great. I think this work will align nicely with the #1873 work as you pointed out.

I have a couple of higher level design/spec questions:

• Would it make sense to add color to Item instead of just Track and Clip?
• In NLE applications, do they typically present a list of colors to assign to a Clip or Track (similar to Markers)?
• Do you have any of the existing adapters in mind for adding support for this?

[semagnum]

Would it make sense to add color to Item instead of just Track and Clip?

I think that would be fine, if we're already planning to add color-coding to all kinds of Items. The only interest I've seen is for individual Markers, which aren't Items.

In NLE applications, do they typically present a list of colors to assign to a Clip or Track (similar to Markers)?

Definitely in Davinci Resolve, can't confirm for others. I don't know how that data is saved, but that may be irrelevant with intermediary formats. But that's partially the goal for the to and from functions, to resolve any formatting differences.

Do you have any of the existing adapters in mind for adding support for this?

The original ticket mentions this information already in AAF and XML, but that's all I know of. I wasn't planning on contributing directly to the adapters, but I can help if they need it.

[camkerr]

I took a couple screenshots showing how NLEs handle clip and track color.

Avid Media Composer

You can do track and clip color in Media Composer a few different ways with some swatches or a picker.

Track Color

Source Color

Clip Color Window

DaVinci Resolve

Resolve allows users to change track and clip color from the same pre-defined list.

Track Color

Clip Color

NukeStudio

Not really an NLE, but NukeStudio allows users to change clip color using a color picker and recent swatches. While track color can change in the UI, it only happens as a result of changing a track's blend mode. Users can't pick individual colors for a track in the GUI nor via the Python API.

Clip Color

Final Cut Pro

FCP changes clip color when you assign a Role to it. When creating Roles, you do have the option to choose from a swatch of predefined colors.

Clips with Roles in Timeline

Roles Window

[jminor]

Thanks! This is very helpful. Seeing how some of these colors map to/from "roles" or other concepts like "offline" "proxy" etc... do we need a way for OTIO to carry those in addition to the RGB color?

Thinking out loud for a moment... if OTIO stored a color as {r=1.0, g=0.0, b=0.0, a=1.0, label="offline"} then an adapter or an application could choose to pull either the RGBA or the color label string, or both.

[semagnum]

Seeing how some of these colors map to/from "roles" or other concepts like "offline" "proxy" etc... do we need a way for OTIO to carry those in addition to the RGB color?
Thinking out loud for a moment... if OTIO stored a color as {r=1.0, g=0.0, b=0.0, a=1.0, label="offline"} then an adapter or an application could choose to pull either the RGBA or the color label string, or both.

I worry of cases where adapters/apps/devs only change one but not the other, or more specifically, where one is completely ignored for the other. Could lead to cases throwing the label and its RGBA value out of alignment. Since Color is already a SerializableObjectWithMetadata, I could just add metadata to the constructor and let that be used as labels initially unless there's a more standardized need?

[semagnum]

I added the name and metadata parameters to the Color constructor, so those could be explicitly defined and used by plugins.

Note that I put the name and metadata arguments at the end of the arg list so users could just define RGBA values without defining a name (it just defaults to "Color"). I figured that convenience might be easier when constructing colors, but let me know if you'd prefer otherwise.

[jminor]

Thanks for trying out that change! Let me see if I can get a couple of other people to take a look at this change.

My thoughts at the moment are:

• Adding a Color class is a good idea.
• Adding a color attribute to Clip and Track is a good idea.
• It might be better to add color to Item instead of Clip and Track separately.
• Adding the name attribute to Color may or may not be a good idea.
• After this lands, then we can look at Marker colors: Marker RGB colors #1873

[darbyjohnston]

Hi, and thanks for the changes! I just left a couple of comments about the C++ code.

[semagnum]

Added updates. And once approved, I'll squash the commits and resolve conflicts afterwards.

[semagnum]

Notes for myself based on the meeting today:

• attach Color to the Item class instead of Clip and Track for now. We could move it higher, but don't want it to be too high-level
• remove SerializableObjectWithMetadata parent class, turn it into a primitive that serializes directly. name can still be included as a parameter, defaults to an empty string
• add tests of handling colors within metadata
• from_hex and to_hex - try to support other hex orders (like agbr, or a provided order as a string), or at least explicitly define the expected order in the docs

[semagnum]

WIP on moving Color to Item. I've been juggling other priorities, and no one reached out about implementing a serializable primitive like mentioned in the last meeting. I'm having problems with the serializable aspect, constructing a track or clip crashes with an ImportError: undefined symbol related to the writer.

But the changes so far can at least give you a sense of the design.

[semagnum]

Tests are failing since all the schemas are now changed. Is the intent to make this a hard API change? Ideally it would only be saved/written to the file if provided at all. I'm not familiar with updating schemas.

[jminor]

Here's where the low-level types like Box2d are serialized:

OpenTimelineIO/src/opentimelineio/serialization.cpp

Line 611 in 7c58de1

void write_value(IMATH_NAMESPACE::Box2d const& value)

When you update a schema, the test baseline needs to be updated to match. I think there's a script or Makefile target which handles that. Let me find someone who remembers the details...

[darbyjohnston]

I haven't done it myself, but it looks like there is some documentation on updating the schema here:
https://opentimelineio.readthedocs.io/en/stable/tutorials/developing-a-new-schema.html

@rogernelson I think you made some schema changes with the spatial coordinates feature, do you have any tips?

Edit: This is the spatial coordinates PR which made some schema changes for reference:
#1219

[jminor]

Hmm... I see one of the CI builds is failing with ModuleNotFoundError: No module named 'pip._vendor.distlib' which seems totally unrelated to your change. I pressed Retry on it to see if that helps.

[darbyjohnston]

@jminor The Windows/MSYS2 build has been broken for awhile now, I'm not sure what the issue is. My personal preference would be to disable it, it seems to break periodically and I'm not sure we have the resources to support it.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 76.64234% with 64 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.62%. Comparing base (c0e97b0) to head (b8cefa1).
⚠️ Report is 61 commits behind head on main.

Files with missing lines	Patch %	Lines
src/opentimelineio/serialization.cpp	19.44%	29 Missing ⚠️
src/opentimelineio/deserialization.cpp	21.42%	11 Missing ⚠️
src/opentimelineio/color.h	41.66%	7 Missing ⚠️
src/opentimelineio/safely_typed_any.cpp	0.00%	4 Missing ⚠️
tests/test_color.py	93.33%	1 Missing and 3 partials ⚠️
src/opentimelineio/color.cpp	96.38%	3 Missing ⚠️
src/opentimelineio/item.h	40.00%	3 Missing ⚠️
...entimelineio-bindings/otio_serializableObjects.cpp	95.12%	2 Missing ⚠️
src/py-opentimelineio/opentimelineio/core/color.py	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1887      +/-   ##
==========================================
+ Coverage   84.11%   84.62%   +0.50%     
==========================================
  Files         198      181      -17     
  Lines       22241    13051    -9190     
  Branches     4687     1206    -3481     
==========================================
- Hits        18709    11044    -7665     
+ Misses       2610     1821     -789     
+ Partials      922      186     -736     

Flag	Coverage Δ
py-unittests	84.62% <76.64%> (+0.50%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/opentimelineio/clip.cpp	85.54% <100.00%> (+9.90%)	⬆️
src/opentimelineio/composition.cpp	79.36% <100.00%> (+12.80%)	⬆️
src/opentimelineio/composition.h	83.87% <ø> (+18.35%)	⬆️
src/opentimelineio/item.cpp	91.95% <100.00%> (+13.64%)	⬆️
src/opentimelineio/serializableObject.h	85.18% <ø> (+14.43%)	⬆️
src/opentimelineio/track.cpp	91.17% <100.00%> (+17.36%)	⬆️
src/opentimelineio/track.h	100.00% <ø> (ø)
...melineio/opentimelineio-bindings/otio_bindings.cpp	98.48% <100.00%> (+0.12%)	⬆️
...ntimelineio/opentimelineio-bindings/otio_utils.cpp	79.59% <100.00%> (+12.92%)	⬆️
.../py-opentimelineio/opentimelineio/core/__init__.py	83.01% <ø> (-0.32%)	⬇️
... and 13 more

... and 119 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69b7ea1...b8cefa1. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jminor]

Thanks for investigating the build failure @darbyjohnston - it looks like that CI task is passing now.

@semagnum can you update this branch with the latest changes from main (rebase or merge)? We will squash the commits when this PR merges. Almost landed!

[semagnum]

Thanks @jminor ! Just rebased the changes.

I think all of those notes were addressed.

[jminor]

Thanks @semagnum !