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

GeneratorReference implementation. #148

Merged
merged 8 commits into from Jan 12, 2018

Conversation

Projects
None yet
5 participants
@ssteinbach
Member

ssteinbach commented Aug 10, 2017

Adds support for a otio.schema.GeneratorReference class, and sketches out a SMPTEBars version of that class. Also adds support for this in the RV adapter.

Questions:

  • Should we follow the pattern in the Transition schema (with an Enum defining specific generators, and then a "generic" fallback)?

Reposted with the public fork (was #132)

@ssteinbach ssteinbach added this to the Public Beta 6 milestone Aug 10, 2017

@ssteinbach ssteinbach changed the title from Generator references to GeneratorReference implementation. Aug 10, 2017

@ssteinbach ssteinbach requested a review from bashesenaxis Sep 11, 2017

@ssteinbach ssteinbach self-assigned this Sep 11, 2017

@ssteinbach ssteinbach requested a review from reinecke Sep 11, 2017

@reinecke

This comment has been minimized.

Show comment
Hide comment
@reinecke

reinecke Sep 11, 2017

Collaborator

This looks totally reasonable to me. I like the idea of using an enum for generator kind, It will encourage some schema unification around the generator types commonly used (flat color, smpte bars, timecode overlay, etc.)

I'd be curious to know what kinds of generators people are encountering so we can try this approach against them, but it seems pretty solid to me.

Collaborator

reinecke commented Sep 11, 2017

This looks totally reasonable to me. I like the idea of using an enum for generator kind, It will encourage some schema unification around the generator types commonly used (flat color, smpte bars, timecode overlay, etc.)

I'd be curious to know what kinds of generators people are encountering so we can try this approach against them, but it seems pretty solid to me.

@bashesenaxis

This comment has been minimized.

Show comment
Hide comment
@bashesenaxis

bashesenaxis Sep 14, 2017

Collaborator

I guess the only concern I would have with this, is that using a generic "parameters" field leaves it quite open to what you put in there.
So I definitely agree with the idea of using an enum for commonly used kinds. And perhaps also a parameter spec for each common kind? Or do we want to rely on convention to make sure that all generator implementations of the adapters line up?

Collaborator

bashesenaxis commented Sep 14, 2017

I guess the only concern I would have with this, is that using a generic "parameters" field leaves it quite open to what you put in there.
So I definitely agree with the idea of using an enum for commonly used kinds. And perhaps also a parameter spec for each common kind? Or do we want to rely on convention to make sure that all generator implementations of the adapters line up?

@quelsolaar

This comment has been minimized.

Show comment
Hide comment
@quelsolaar

quelsolaar Sep 14, 2017

Collaborator

I'm generally in favor of using Enums as much as possible in the API, strings are error prone. Especialy since the underlying implementation will use a readable text string so that its easy to add new enums. If we use strings they should be backed up by defines for common usages.

Collaborator

quelsolaar commented Sep 14, 2017

I'm generally in favor of using Enums as much as possible in the API, strings are error prone. Especialy since the underlying implementation will use a readable text string so that its easy to add new enums. If we use strings they should be backed up by defines for common usages.

@ssteinbach

This comment has been minimized.

Show comment
Hide comment
@ssteinbach

ssteinbach Jan 11, 2018

Member

Rebased onto latest master.

Member

ssteinbach commented Jan 11, 2018

Rebased onto latest master.

Show outdated Hide outdated contrib/adapters/extern_rv.py
@jminor

jminor approved these changes Jan 11, 2018

@jminor jminor merged commit 180392b into PixarAnimationStudios:master Jan 12, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jminor jminor referenced this pull request Jan 12, 2018

Closed

Generator clips #61

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment