Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Common Edit Command Library #711

Open
mccartnm opened this issue May 19, 2020 · 9 comments
Open

Common Edit Command Library #711

mccartnm opened this issue May 19, 2020 · 9 comments
Labels

Comments

@mccartnm
Copy link

mccartnm commented May 19, 2020

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:

  • Overwrite
  • Trim
  • Cut
  • Slip
  • Slide
  • Ripple
  • Roll
  • Fill (3/4 Point Edit)

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:

class EditEvent {
public:
    EditEventKind kind; // e.g. Insert, Append, Remove, Modify
    Retainer<Composition> parent;
    Retainer<Composable> composable;

    // ... Additional fields to execute the above as required

    bool run(ErrorStatus* error_status);
    bool revert();
};

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.

for (EditEvent &event: events) {
    event.run(error_status);
    if (*error_status) {
        for (auto it = completed_events.rbegin(); /*...*/)
            (*it).revert();
        break;
    }
    completed_events.push_back(event);
}

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:

otio.edit.place(item, into=track, at=rat_time, placement_kind=otio.edit.PlacementKind)
otio.edit.trim(item, edge=otio.edit.ItemEdge)
etc.

Additional commands can be added as the editing foundation is built.

Pros:

  • Atomic events make reuse simple and should scale well with the complexity of edit commands
  • Hooks up nicely to an undo/redo framework on third party software

Cons:

  • Additional code to handle event care

Questions/Considerations:

  • Could we handle preview flags for edit commands? This would let third party tools inspect the outcome of events before committing them.
  • Should EditEvent be a class system (e.g. InsertEvent, ModifyEvent) or something else entirely?
@ssteinbach
Copy link
Collaborator

ssteinbach commented May 20, 2020

Couple of suggestions:

  • Describing what the edit operations do would be helpful (slip, slide, ripple, roll, etc)
  • I would argue that having a single edit API wrapped in a transactional system is overkill, but having another library or layer that wraps the operations in a transaction system would be ok. In other words, having a piece that is just a functional module for doing the edits, and having another layer that exposes a transaction system. I suspect the 80% use case of an edit module will not need a transaction system.
  • If you're going to build a transaction system, then being able to serialize and replay a set of manipulations could be an interesting way of communicating and translating the difference between two states of a given file (we get asked periodically about change lists).

@mccartnm
Copy link
Author

mccartnm commented May 21, 2020

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.

Overwrite

  • Image Reference
  • Anything overlapping is destroyed on contact. This may split an item.

API

otio.edit.place(
    item: Item,
    into: Composition,
    at: RationalTime,
    placement: otio.edit.PlacementKind = otio.edit.PlacementKind.Overwrite,
    fill_template: Composable = None, # Default to Gap
)

Insert

  • Image Reference
  • Items past the in point are shifted by the new items duration.
  • Item may be split if required.

API

otio.edit.place(
    ...,
    placement: otio.edit.PlacementKind = otio.edit.PlacementKind.Insert,
)

Trim

  • Image Reference
  • Adjust a single item's start time or duration.
  • Do not affect other clips.
  • Fill now-empty time with gap or template.
  • Clamps source_range to non-gap boundary (can only overwrite gaps)

API

otio.edit.trim(
    item: Item,
    adjust_in: RationalTime = None,  # Duration of change
    adjust_out: RationalTime = None  # -Duration of change
)

Slice

  • Image Reference
  • Slice an item, generating a clone of self and augment both item's source_range.

API

otio.edit.slice(
    item: Item,
    at: RationalTime,
)

Slip

  • Image Reference
  • Adjust the start_time of an item's source_range.
  • Do not affect surrounding items.
  • Clamp to available_range of media (if available)

API

otio.edit.slip(
    item: Item,
    delta: RationalTime, # Relative (e.g. RationalTime(-1, 24) sources one frame earlier)
)

Slide

  • Image Reference
  • Adjust start time of item and trim adjacent items to fill
  • Do not change main item's duration, only adjacent
  • Clamp to available range of adjacent items (if available)

API

otio.edit.slide(
    item: Item,
    delta: RationalTime, # Relative like slide
)

Ripple

  • Image Reference
  • Adjust a source_range without adjusting any other items
  • This effectively shifts all currently adjacent items to stay at the edges
  • (Impl detail for otio, this is the same as adjusting the source_range of the item)

API

otio.edit.ripple(
    item: Item,
    adjust_in: RationalTime = None,  # Duration of change
    adjust_out: RationalTime = None  # -Duration of change
)

Roll

  • Image Reference
  • Any trim-like action results in adjacent items source_range being adjusted to fit
  • No new items are ever created
  • Clamped to available media (if available)

API

otio.edit.roll(
    item: Item,
    adjust_in: RationalTime = None,  # Duration of change
    adjust_out: RationalTime = None  # -Duration of change
)

3/4 Point Edit

  • Image Reference
  • The most complex - this "fills" a gap based on a source in/out point or track in/out point
  • Often used to patch in items as edit is built
  • Note: This can be accomplished by a conjunction of commands above

API

otio.edit.fill(
    item: Item,
    into: Composition,
    replace: Item,
    reference_point: otio.edit.ReferencePoint.SourceOut
)

@mccartnm
Copy link
Author

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.

@ssteinbach
Copy link
Collaborator

ssteinbach commented May 21, 2020

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:

  • It seems like most of these arguments that you've marked as 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?
  • For any parameter that is a time coordinate (for example, the 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).
  • similar to how the 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.
  • After thinking about it, we have an 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?

@jminor
Copy link
Collaborator

jminor commented May 21, 2020

This is fantastic, and fills a gap (pun intended) in OTIO that we have talked about for ages.

A couple of notes:

  • It might be easier to understand if place was separated into two overwrite and insert functions.
  • I can see clearly how most of these would work when modifying one or more clips in a track. However, if you use 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?
  • The additional 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.

@mccartnm
Copy link
Author

mccartnm commented May 21, 2020

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

  • I see the need to specify the arguments more clearly for the various calls. We just have to decide if we're taking this api as either:

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.

  • For the time coordinates, I've had good luck with relative deltas wherever they fit.** So for the command: "Trim this item's start point three frames in.", We only have to know the item we're adjusting, and it's left-sided neighbor, if any. So perhaps 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

  • I think splitting the 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.

@meshula
Copy link
Collaborator

meshula commented May 25, 2020

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:

  • given a set of tracks and clips
  • propose an operation such as split
  • learn which things would be split (a list of elements to remove)
  • and learn which things would be created (a list of the things resulting from the split)
  • filter the proposed edit to create a new edit, or completely discard the proposed edit.
  • apply the edit according to the { removals, adds }

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.

@mccartnm
Copy link
Author

@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":

  1. Common Edit Commands - Simple tasks with a clean OOP feel "e.g. Slice this item at time X in it's parent time coordinates" - the commands above represent this layer.
  2. Raw Edit Commands - the underlying methods that do the work of the common commands. These allow the user to filter/modify the event list to their liking, or generate their own event list from scratch to make additional procedures.

* 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.

@darbyjohnston
Copy link
Contributor

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: #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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants