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

AAF writer: Adding support for audio transitions #454

Merged

Conversation

shahbazk8194
Copy link
Contributor

Creating abstract method _transition_parameters that is called in _TrackTranscriber.aaf_transition to support both video and audio transitions

side note: Using metadata clip length instead of duration() in aaf_sourceclip. We'll fix this later

Co-authored-by: Freeson Wang freeson@pixar.com

_transition_parameters

Co-authored-by: Freeson Wang <freeson@pixar.com>
@codecov-io
Copy link

codecov-io commented Mar 9, 2019

Codecov Report

Merging #454 into master will increase coverage by <.01%.
The diff coverage is 96.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #454      +/-   ##
==========================================
+ Coverage   87.16%   87.17%   +<.01%     
==========================================
  Files          67       67              
  Lines        6788     6799      +11     
==========================================
+ Hits         5917     5927      +10     
- Misses        871      872       +1
Impacted Files Coverage Δ
...neio_contrib/adapters/advanced_authoring_format.py 90.57% <ø> (-0.05%) ⬇️
...elineio_contrib/adapters/tests/test_aaf_adapter.py 98.08% <100%> (ø) ⬆️
...elineio_contrib/adapters/aaf_adapter/aaf_writer.py 93.6% <96.87%> (-0.06%) ⬇️

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 0d6c8ce...2ebba59. Read the comment docs.

@ssteinbach ssteinbach added this to the Public Beta 10 milestone Mar 11, 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.

Looks good to me, thanks!

@ssteinbach ssteinbach added this to To do in AAF Adapter via automation Mar 11, 2019
@ssteinbach ssteinbach added the enhancement A request for something new. label Mar 11, 2019
@ssteinbach ssteinbach moved this from To do to In progress in AAF Adapter Mar 11, 2019
Copy link
Contributor

@stefanschulze stefanschulze left a comment

Choose a reason for hiding this comment

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

Code looks good to me. Only thing my mind was hesitating for a sec. is the self.media_kind usage in the aaf_transition method. When is it set? And is it context-sensitive, i.e. changes per transition type?

Copy link
Collaborator

@jminor jminor left a comment

Choose a reason for hiding this comment

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

This looks good. My only concern is the use of metadata["AAF"]["Length"] instead of duration().value. Do you know what the issue in the reader is? Is there an open issue for that?

@shahbazk8194
Copy link
Contributor Author

@jminor We don't know what the issue is regarding why the duration().value. There is an issue for that. It is issue #438

I imagine it would be a small change to the reader. We plan on tackling that along side the work that's happening to reduce metadata coupling throughout the writer

@ssteinbach ssteinbach merged commit 4ad489f into AcademySoftwareFoundation:master Mar 13, 2019
AAF Adapter automation moved this from In progress to Done Mar 13, 2019
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

5 participants