Add support for official OTIO Clip Color schema#161
Conversation
|
|
1f01a69 to
f13f627
Compare
|
Hey @harshitgupta62, thank you for your contribution. @jminor what do you think of this approach? Would it be cleaner to check the schema versions via |
jminor
left a comment
There was a problem hiding this comment.
Hi, it looks like this PR just moves the color string property to a different place in the metadata instead of the new color property, which is not a string. Please read this merged PR for details on the new Color schema: AcademySoftwareFoundation/OpenTimelineIO#1887
0f0edc2 to
1fb4480
Compare
Signed-off-by: Harshit Gupta <harshit.ashish.nigoti@gmail.com>
1fb4480 to
a744cef
Compare
|
@jminor All checks are passing now and the implementation uses the official OTIO Color schema. Please take another look. |
|
Hi @harshitgupta62 - could you tell us a little more about yourself and your goals for this PR? |
|
Hi @jminor, Sure! I'm currently pursuing a B.Tech in Information Technology at ABV-IIITM Gwalior, and I've been getting more involved in open-source contributions to learn from real-world projects. My goal with this PR is to update Raven to support the official OTIO Clip Color schema while maintaining backward compatibility with the existing Raven metadata. I picked up this issue because it seemed like a great opportunity to better understand the OpenTimelineIO ecosystem and contribute something useful to the project. I really appreciate the feedback and I'm happy to make any further changes if needed. |
|
@harshitgupta62 welcome! I can see some missing concepts in how you've implemented this PR. My suggestion is to familiarize yourself with how clip colors are used in some NLEs (Resolve, Kdenlive, Adobe Premiere, etc.) so you build a better understanding of how a user would expect this feature to work. Then I think you will see what is missing here. |
|
Hi @jminor, thanks for the feedback. I spent some time reading through the Color schema PR and looking at how Raven currently handles clip colors. I noticed that the UI still routes colors through Am I correct that the main missing piece is that Raven should preserve and use the full OTIO |
|
Yes. Some NLEs support only a set of named colors while others let you pick arbitrary RGB values. We want Raven to support both, and to read/write an OTIO file without losing anything. |
|
Hi @jminor, thanks, that makes sense. Looking through Raven, I noticed the current UI only exposes a combo box of predefined named colors (DrawColorChooser()), so even though clip colors now use the OTIO Color type internally, the editor itself can't represent arbitrary RGB values yet. Would you expect this PR to extend the UI with an RGB color picker (while still offering the predefined named colors), or would you prefer preserving arbitrary RGB values on read/write first and handling richer editing in a follow-up PR? |
|
Hi @jminor,could you please have a look at my comment, so that i could start working upon this issue. |
|
Normally I would say that adding this feature in small steps would be best. That would make it easier for you, the author, and for us, the reviewers, to ensure that each step makes sense. In this case there's a slight complication. If the UI only supports a fixed set of colors, and Raven read an OTIO with some other custom color, then we'd need to make sure that color wasn't lost. Raven's main purpose is to inspect what's in an OTIO, so by definition it should show the content as-is without modification. Can you propose a way to accomplish that without expanding the scope of this feature too big? |
b91074b to
5824b97
Compare
Signed-off-by: Harshit Gupta <harshit.ashish.nigoti@gmail.com>
|
Hi @jminor, Pushed an update that addresses this directly. The bug: the old name↔color mapping in editing.cpp silently defaulted any unrecognized color to "WHITE". So a custom RGB color wasn't just hard to see — if a user then touched the color chooser, it could get overwritten with actual white without warning. I've consolidated this into colors.h/colors.cpp, reusing the 11-color palette UIColorFromName() already supports, and unmatched colors now return "" (custom, don't touch) instead of guessing a name. Display: when a clip's color doesn't match a named preset, the inspector now shows a swatch + tooltip instead of a blank combo, so the custom value is visible but stays untouched unless the user explicitly picks a new preset. Bonus find: timeline.cpp already had the correct pattern for clips — checking item->color() first and rendering the true RGB, falling back to named colors only if none is set. Tracks never got that treatment and were rendering custom colors as plain gray, so I brought them in line with the same pattern rather than inventing a new one. Scope is still just "preserve and correctly show what's already there" — no new RGB picker UI. Let me know if this covers what you were looking for. |
|
Hi @harshitgupta62 your last comment looks a lot like LLM output. Am I talking to a human or an LLM? |
|
Hi @jminor — I've been using AI assistance to help me understand the codebase and draft these explanations while I'm still learning C++/OTIO in depth. But I've reviewed and understand every change in this PR — happy to answer any specific questions directly to confirm that. |
Fixes #160
Updates GetItemColor and SetItemColor to read/write the official
OTIO Clip Color schema field, falling back to Raven's legacy
"raven.color" metadata for backward compatibility.