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

FCP adapter uses clip item rate instead of source rate #177

Merged
merged 2 commits into from Dec 20, 2017

Conversation

Projects
None yet
3 participants
@bashesenaxis
Collaborator

bashesenaxis commented Oct 31, 2017

addresses #174

  • uses clip item rate instead of source rate
  • edited clipitem in premiere_example.xml that has different rate compared to its linked file
  • fixed bug in timeline_widget, transitions don`t have trimmed_range_in_parent method
  • fixed bug in timeline_widget, left over rename from stack to composition
]
if track_item.tag == 'clipitem':
if start == -1:
if start.value == -1:

This comment has been minimized.

@jminor

jminor Oct 31, 2017

Collaborator

Is -1 meant to be an indicator that there is no value yet? If so, maybe start == None would be a better sentiel. That way you can be extra sure that you don't accidentally do arithmetic on a -1 value.

@jminor

jminor Oct 31, 2017

Collaborator

Is -1 meant to be an indicator that there is no value yet? If so, maybe start == None would be a better sentiel. That way you can be extra sure that you don't accidentally do arithmetic on a -1 value.

This comment has been minimized.

@bashesenaxis

bashesenaxis Nov 1, 2017

Collaborator

-1 is actually the values that you'll encounter in an XML file.
If start or end is -1, then it means that item is preceded or succeeded by a transition. As such, you'll need to query neighboring items for the correct values.

@bashesenaxis

bashesenaxis Nov 1, 2017

Collaborator

-1 is actually the values that you'll encounter in an XML file.
If start or end is -1, then it means that item is preceded or succeeded by a transition. As such, you'll need to query neighboring items for the correct values.

This comment has been minimized.

@jminor

jminor Nov 1, 2017

Collaborator

Ok, got it.

@jminor

jminor Nov 1, 2017

Collaborator

Ok, got it.

for item in self.track:
timeline_range = item.trimmed_range_in_parent()
for n, item in enumerate(self.track):
timeline_range = self.track.trimmed_range_of_child_at_index(n)

This comment has been minimized.

@ssteinbach

ssteinbach Nov 1, 2017

Member

@jminor this is another case where we might want to consider folding transitions back into the regular item pattern.

@ssteinbach

ssteinbach Nov 1, 2017

Member

@jminor this is another case where we might want to consider folding transitions back into the regular item pattern.

This comment has been minimized.

@jminor

jminor Nov 1, 2017

Collaborator

I agree.

@jminor

jminor Nov 1, 2017

Collaborator

I agree.

This comment has been minimized.

@jminor

jminor Nov 1, 2017

Collaborator

That shouldn't block this change, we can do that later.

@jminor

jminor Nov 1, 2017

Collaborator

That shouldn't block this change, we can do that later.

This comment has been minimized.

@ssteinbach

ssteinbach Nov 1, 2017

Member

Agreed.

@ssteinbach

ssteinbach Nov 1, 2017

Member

Agreed.

@jminor

This is approved except for the lint issue that travis caught: fcp_xml.py:960:1: W391 blank line at end of file

bashesenaxis added some commits Oct 31, 2017

addresses #174
- uses clip item rate instead of source rate
- edited clipitem in premiere_example.xml that has different rate compared to its linked file
- fixed bug in timeline_widget, transitions don`t have trimmed_range_in_parent method
- fixed bug in timeline_widget, left over rename from stack to composition
adresses change requests in #177
- maintain clip item rate on both read and write
@bashesenaxis

This comment has been minimized.

Show comment
Hide comment
@bashesenaxis

bashesenaxis Dec 19, 2017

Collaborator

I have rebased my changes and addressed the issue that was causing the unit test to fail. Item rate is now maintained through the read / write process.

Collaborator

bashesenaxis commented Dec 19, 2017

I have rebased my changes and addressed the issue that was causing the unit test to fail. Item rate is now maintained through the read / write process.

@jminor

jminor approved these changes Dec 19, 2017

Looks great! Thanks Bas.
Do you have any more changes before we accept this PR?

@bashesenaxis

This comment has been minimized.

Show comment
Hide comment
@bashesenaxis

bashesenaxis Dec 20, 2017

Collaborator

Cool :)
I`ve got no more changes for this.

Collaborator

bashesenaxis commented Dec 20, 2017

Cool :)
I`ve got no more changes for this.

@jminor jminor merged commit 9cf5e93 into PixarAnimationStudios:master Dec 20, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment