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

New feature implementation of writing AAF files from otio files in the AAF adapter #432

Merged
merged 29 commits into from
Mar 6, 2019

Conversation

freesonluxo
Copy link
Contributor

This is an initial implementation of the AAF writer in AAF adapter.

Co-authored-by: Shahbaz Khan shahbazkhan8194@gmail.com

@freesonluxo freesonluxo marked this pull request as ready for review February 23, 2019 00:21
@freesonluxo freesonluxo changed the title [WIP] New feature implementation of writing AAF files from otio files in the AAF adapter New feature implementation of writing AAF files from otio files in the AAF adapter Feb 23, 2019
@shahbazk8194

This comment has been minimized.

freesonluxo and others added 25 commits March 4, 2019 11:28
Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
Co-author-by: Shahbaz Khan <shahbazk@pixar.com>
Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
with moving ownership of compositionmob to the AAFFileTranscriber
class

Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
Also conforming to pep8.

Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
["Length"] instead of duration. We really should look into this bug.
Right now working around. Using soft gets in aaf_transition() in places
where we might not have values.

Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
removing the point_list check in aaf_transition()

Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
This commit should be relatively clean.

Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
- clean up comments
- functionilized descriptors into base class
- fixing exceptions, raising stricter exceptions

Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
- Transition type checking
- Rename aaf_file_transcriber to root_file_transcriber
- Rename precheck to validate_metadata and made a free function

Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com
Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com
Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
@codecov-io
Copy link

codecov-io commented Mar 4, 2019

Codecov Report

Merging #432 into master will increase coverage by 0.63%.
The diff coverage is 95.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #432      +/-   ##
==========================================
+ Coverage   86.52%   87.15%   +0.63%     
==========================================
  Files          66       67       +1     
  Lines        6233     6734     +501     
==========================================
+ Hits         5393     5869     +476     
- Misses        840      865      +25
Impacted Files Coverage Δ
...neio_contrib/adapters/advanced_authoring_format.py 91.83% <84.37%> (-0.75%) ⬇️
...elineio_contrib/adapters/aaf_adapter/aaf_writer.py 93.66% <93.66%> (ø)
...elineio_contrib/adapters/tests/test_aaf_adapter.py 97.91% <98.95%> (+0.68%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8de8647...32f3489. Read the comment docs.

freesonluxo and others added 2 commits March 4, 2019 13:39
Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
@ssteinbach ssteinbach added this to the Public Beta 10 milestone Mar 4, 2019
@ssteinbach ssteinbach added this to To do in AAF Adapter via automation Mar 4, 2019
@ssteinbach ssteinbach added the enhancement A request for something new. label Mar 4, 2019
Copy link
Collaborator

@ssteinbach ssteinbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! These notes are down in the details and pretty nit picky, but they can help with legibility and maintainability down the road. Thanks for sticking through this, this is really close! Let me know if you have any questions!

opentimelineio_contrib/adapters/aaf_adapter/aaf_writer.py Outdated Show resolved Hide resolved
opentimelineio_contrib/adapters/aaf_adapter/aaf_writer.py Outdated Show resolved Hide resolved
opentimelineio_contrib/adapters/aaf_adapter/aaf_writer.py Outdated Show resolved Hide resolved
opentimelineio_contrib/adapters/aaf_adapter/aaf_writer.py Outdated Show resolved Hide resolved
opentimelineio_contrib/adapters/aaf_adapter/aaf_writer.py Outdated Show resolved Hide resolved
"""Get a unique tapemob, identified by clip metadata mob id."""
mob_id = otio_clip.metadata["AAF"]["SourceID"]
tape_mob = self._unique_tapemobs.get(mob_id)
if not tape_mob:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inverting this would allow you to dedent the block and make the else condition more clear:

if tape_mob:
  return tape_mob
...

# Currently, we only support clips that are already in Avid with a mob ID
mob_id = otio_clip.metadata["AAF"]["SourceID"]
master_mob = self._unique_mastermobs.get(mob_id)
if not master_mob:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inverting this could also make it more readable (see next comment).

AAF Adapter automation moved this from To do to In progress Mar 5, 2019
_TrackTranscriber. Additional comments.

Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
Copy link
Collaborator

@ssteinbach ssteinbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Thanks for that!

@ssteinbach ssteinbach merged commit 93ae4c7 into AcademySoftwareFoundation:master Mar 6, 2019
AAF Adapter automation moved this from In progress to Done Mar 6, 2019
ssteinbach pushed a commit that referenced this pull request Mar 6, 2019
…e AAF adapter (#432)

Initial commit of OTIO->AAF Writer from @freesonluxo and @shahbazk8194 .

Co-authored-by: Shahbaz Khan <shahbazkhan8194@gmail.com>
@freesonluxo freesonluxo deleted the aaf_writer branch May 24, 2019 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A request for something new.
Projects
AAF Adapter
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants