Open mccartnm opened 4 years ago
Couple of suggestions:
Thanks for the feedback! I agree that the transaction system is a whole extra dragon to slay. I'll try to proceed with the edit layer sans-transactions and keep lookout for a nice way to transition into a layer atop/aside it.
As for the initial edit commands, I've generated some images and assessed what their possible API call might look like. It's a rather long message so I've just linked the images. For documentation I would set them all together.
otio.edit.place(
item: Item,
into: Composition,
at: RationalTime,
placement: otio.edit.PlacementKind = otio.edit.PlacementKind.Overwrite,
fill_template: Composable = None, # Default to Gap
)
otio.edit.place(
...,
placement: otio.edit.PlacementKind = otio.edit.PlacementKind.Insert,
)
otio.edit.trim(
item: Item,
adjust_in: RationalTime = None, # Duration of change
adjust_out: RationalTime = None # -Duration of change
)
otio.edit.slice(
item: Item,
at: RationalTime,
)
otio.edit.slip(
item: Item,
delta: RationalTime, # Relative (e.g. RationalTime(-1, 24) sources one frame earlier)
)
otio.edit.slide(
item: Item,
delta: RationalTime, # Relative like slide
)
otio.edit.ripple(
item: Item,
adjust_in: RationalTime = None, # Duration of change
adjust_out: RationalTime = None # -Duration of change
)
otio.edit.roll(
item: Item,
adjust_in: RationalTime = None, # Duration of change
adjust_out: RationalTime = None # -Duration of change
)
otio.edit.fill(
item: Item,
into: Composition,
replace: Item,
reference_point: otio.edit.ReferencePoint.SourceOut
)
One additional command might be a general swap. This can effectively be a "delete" and will replace an item with a gap or some fill template.
I think there is enough here, and assuming you're interested in doing the work, that a good way to go would be to put this into a .md
file in the documentation folder and make a PR to start iterating on it. It will be easier to make notes on the md using github's code review system, given how long this is. We took a similar approach with the C++ core, first making a PR with documentation and design and then following up with implementation.
The images (which are great!) that you made can also be added inline in that case.
The document ^^ can also serve to seed the future document which is more user facing describing how to use these functions to execute common editing operations on OTIO files.
Some notes:
item: Item
probably want to be compositions, right? or example, does Slide make sense on a single free item? It seems like you want to give it a Composition
(and maybe even more specifically a Track
), and the implementation finds the item that you want to transform within the Composition
/Track
. Does that make sense?at
parameter, or the trim
arguments), we're trying to get better at documenting the coordinate system that the argument is meant to be in. For example, if you have trackA, and you are calling trim
, I would expect the arguments to be in the track's output coordinate system (so after any kinds of effects or trimming has been done).place
function has multiple uses, can slice
, slip
, and slide
use the same underlying function with different replacement policies? Similar question for ripple and roll.algorithms
module, which has things like flattening and filtering at the moment. Would it make sense to have an edit_algo
module under that? Or are these distinct enough that users would expect to see them in an olio.edit
namespace rather than olio.algorithms.splice
for example?This is fantastic, and fills a gap (pun intended) in OTIO that we have talked about for ages.
A couple of notes:
place
was separated into two overwrite
and insert
functions.place
to insert a clip into a stack, then it is less clear - does the clip only go into the top track of the stack? Does a new track get inserted? Similarly if I insert a stack into a stack, should it match up each track?swap
function seems like a special case of fill
- having a clear distinction between which one will trim/extend versus applying a speed up/down effect to stretch the clip to fit would be good. No one wants to accidentally apply a speed effect :)When I was writing the flatten algorithm, I wished I had these. And in fact, there are some flaws in the flatten algorithm that could be addressed once we have these. That's more evidence that maybe flatten should be in the same module namespace as these.
Thanks again for the notes! I'm excited to take on the challenge. I'll work on a design doc in a PR next. The structure is definitely something I want to get right. The otio.algorithms
module could be a great spot to expose the library, as long as we make it clear that it hosts the edit toolkit.
@ssteinbach
A. "do the edit to whatever item is at this time in a parent" --- or --- B. "apply the edit to the item I've got hold of"
All of these commands start with a single item and the edit algorithm decides what other elements need augmenting/removal. Because of that, the second way speaks to me more but I'm open to either path. I'll whip up some additional diagrams for this in the design doc to gain consensus.
delta_start
and delta_end
would fit better? Another great topic for the design doc. The at
parameter could use a name change (parent_time
? - would the into
argument be better as parent
?) **@jminor
place
command at the api level is a good idea. Under the hood they can still use nearly the same code path. That goes to @ssteinbach's point about the merging commands. The implementation will reuse a lot of the code, we just have to decide how explicit/general we want our end API to feel.** - Now that I read it again - one major exception is slice
because that have to be either the child coordinates or parent coordinates explicitly. I'm in favor of argument modifiers to do the math for the end-user automatically.
A few more notes on the notion of a transaction system, and a set of editing operations, and their relationships.
I would like to be able to use such a system of transacted edits as follows:
In order to filter the edit, the list of added objects would have to reference the parent objects. For example, if I propose a split, and wish to accept every split except those referencing audio media, I'd want to filter the audio related elements from the proposed deletes, and the corresponding new elements from the proposed adds.
I think these separable operations are implicit in the proposal, but I'd like to call them out explicitly, because another possibility for implementation is that an operation such as split occurs invisibly, so in order to learn what's changed, one would need to do some kind of memoization before and after and attempt a diff.
@meshula This is the kind of use case I hope to cover with the EditEvent
system. Essentially this might boil down to two exposed "layers":
* It's worth pointing out that all commands from both layers would return a vector of events. The CEC just returns them already processed, so they can be introspected afterwards. The REC would have the ability to generate event lists for later processing. Which may lead to otio.algorithms.process(events)
or class AbstractEdit(...):
of some sort.
I've got an initial sketch of the design doc here: https://github.com/mccartnm/OpenTimelineIO/blob/openedit_design/docs/design/editorial_design.md
I'm just waiting on the CLA response to start the PR but in the mean time I'll add some additional details about this.
I recently came across this and noticed there hasn't been any activity for awhile? It looks really interesting so as an experiment I tried implementing a couple of the edit commands to get a feel for the work involved: https://github.com/AcademySoftwareFoundation/OpenTimelineIO/pull/1518
Great images BTW, they were very helpful for visualizing the operations. The code was fairly tricky to get working correctly so I wrote the tests first and then iterated until everything passed. I haven't added error handling and there are probably corner cases I missed.
One thing that occurred to me while working on this is that handling transitions or any future subclass of Composable
that returns true
for overlapping()
, is going to make these much more complicated. Especially trying to satisfy the constraints mentioned in the documentation:
https://opentimelineio.readthedocs.io/en/latest/tutorials/otio-timeline-structure.html#transitions
Overview
The industry is full of timeline management software but nearly all of them include a fundamental set of tools for manipulation. The most well known being:
OpenTimelineIO, while not seeking to become a fully realized NLE, could benefit from having a simple toolkit to help automate building/modifying timelines based on time rather than index alone.
Proposal
For editing commands, we should strive to do all validation and assert that everything works before committing anything on the timeline. I'm proposing the
EditEvent
which might something like:An event is atomic in nature and does "one thing" to a
Composable
. Each edit maneuver (e.g.otio.edit.place(...)
) would generate a vector of these events that can be played forward and, possibly, backward. The result of them collectively is the commands result.Alternatively, we could build a more robust transaction system since all items can be cloned but there are memory considerations for that and beyond the scope of this initial proposal.
API
The API might look like:
Additional commands can be added as the editing foundation is built.
Pros:
Cons:
Questions/Considerations:
preview
flags for edit commands? This would let third party tools inspect the outcome of events before committing them.EditEvent
be a class system (e.g.InsertEvent
,ModifyEvent
) or something else entirely?