Add global_start_time as a serializable_field. #477
Conversation
ssteinbach
commented
Apr 4, 2019
ssteinbach
commented
- Also add the ability to layer over baselines in tests
- global_start_time has a default value with a rate of 24
- Also add the ability to layer over baselines in tests - global_start_time has a default value with a rate of 24
| global_start_time = core.serializable_field( | ||
| "global_start_time", | ||
| opentime.RationalTime, | ||
| doc="Global starting time value and rate of the timeline." |
There was a problem hiding this comment.
Hmm.. interesting that this also implies the edit rate of the timeline.
There was a problem hiding this comment.
�Yeah. What do you think? Really it gives the timecode of the first time in the timeline.
There was a problem hiding this comment.
...but that implies the rate
There was a problem hiding this comment.
I don't mind that so much in principle, but given that there's a default rate of 24 I think it will be a problem in practice.
There was a problem hiding this comment.
Is there a clean way to remove the default rate?
There was a problem hiding this comment.
I'm not sure that there is a clean way to remove the default. RationalTime objects are immutable, so it can be None or not set AFAIK. we should discuss further.
|
The default value of the global_time_offset can be `None`. That would imply a value of 0 with a rate of… something?
… On Apr 4, 2019, at 3:47 PM, Joshua Minor ***@***.***> wrote:
@jminor commented on this pull request.
In opentimelineio/schema/timeline.py <#477 (comment)>:
> @@ -68,6 +68,11 @@ def __init__(
dict,
"Metadata dictionary."
)
+ global_start_time = core.serializable_field(
+ "global_start_time",
+ opentime.RationalTime,
+ doc="Global starting time value and rate of the timeline."
Is there a clean way to remove the default rate?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#477 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADuWVBDlqFeHVbhCt_8hmcOjD1dRGEUks5vdoEZgaJpZM4cduqj>.
|
|
I did just notice the other day that global_start_time is not being serialized, and wondered if that was intentional or not.
are we going to start actually serializing it? if so, i’ll adjust the C++ implementation to follow.
|
|
If we can change the default to be None that would be better. If anyone is doing arithmetic with it, they'll have to check for None first though. Currently the fcp_xml and maya adapters put real values in there, and I'm about to update the AAF adapter to do the same. We might have to bump the schema version to Timeline.2. Lets discuss more before we commit to this change. |
|
Either way I would argue this change should land in beta 11 and not beta 10. Thoughts? |
|
Addresses #480 |
- Remove the code that was setting it to a default of 0, 24 - Update baseline for the Timeline - In the FCP7 round trip test, FCP round trips the rate but not the offset. Add some code to exersize this.
Codecov Report
@@ Coverage Diff @@
## master #477 +/- ##
==========================================
+ Coverage 88.08% 88.23% +0.14%
==========================================
Files 68 68
Lines 6881 6899 +18
==========================================
+ Hits 6061 6087 +26
+ Misses 820 812 -8
Continue to review full report at Codecov.
|
|
We are going to land this in beta 10 after all. This change should be a sooner-not-later change. |
…tion#477) * Add global_start_time as a serializable_field. * The default value for global_start_time is None
