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

EDL support transition writes #3

Open
wants to merge 44 commits into
base: aaf_adapter
Choose a base branch
from

Conversation

mikemahony
Copy link

Generates EDLs with dissolve transitions that look like:

001 AX V C     00:00:00:01 00:00:01:13 00:00:00:00 00:00:01:12
002 AX V C     00:00:01:13 00:00:01:13 00:00:01:12 00:00:01:12
002 AX V D 036 00:00:00:01 00:00:02:13 00:00:01:12 00:00:04:00
003 AX V C     00:00:00:01 00:00:02:01 00:00:04:00 00:00:06:00

@mikemahony
Copy link
Author

mikemahony commented Dec 17, 2017

Hi @jminor, I'm not 100% on this but I wanted to get it on your radar.

This line, for example, doesn't seem quite right yet: https://github.com/mikemahony/OpenTimelineIO/blob/bab455428c065840160a908c5748f038c363ad4f/opentimelineio/adapters/cmx_3600.py#L646 - I do need to account for the handles in some way like this, yes? I think I'm hardly accounting for them at all at this point... :-/

I'd love to hear your thoughts when you have some time.

PS. I hope you don't mind that this MR points to your aaf_adapter branch; I'm currently using that branch for all its excellent AAF goodness.

@mikemahony
Copy link
Author

Added a new commit that's more correct but still not right. Is it possible that track.range_of_child_at_index is somehow miscalculating?

@mikemahony
Copy link
Author

Ok. test_nucoda_edl_write_with_transition_2 feels like it's on the right track. Will reconcile with test_nucoda_edl_write_with_transition next.

@mikemahony
Copy link
Author

Sorry for all the craziness! I made myself some diagrams and re-inspected the AVID-generated EDL I received from CZuber, confirming that it imports into Nucoda correctly. I'm now pretty certain that the following test will indeed need to pass for the EDL to import into Nucoda: https://github.com/mikemahony/OpenTimelineIO/blob/be2f4808e8bdad55cf6992bd10cf582dfcec2025/tests/test_cmx_3600_adapter.py#L428

It might be best to talk in person, but just in case, here are my diagram notes:

Data from original CrossDissolve_Day-Night_Long_1.aaf (via OTIO)
v330_21f    130 275     145 frames                               00:00:05:10 00:00:11:11
trans       -57 -43
v330_23e    222 422     200 frames                               00:00:09:06 00:00:17:14

Source ranges, no transition
A |-------------------------|                               130 275     00:00:05:10 00:00:11:11                   
B               145         |-------------------------|     222 422     00:00:09:06 00:00:17:14
                                        200

Visualized on generic timeline
A |---------------------|\--|                               130 232     00:00:05:10 00:00:09:16
B                 |----\|-------------------|               165 422     00:00:06:21 00:00:17:14
                  |-----|---|
                      57 43

Visualized EDL style                                                                                            Cumulative time
A |---------------|xxxxxxxxx|                               130 175     00:00:05:10 00:00:07:07                 45         00:00:01:21
B       45        |\------------------------|               222 422     00:00:09:06 00:00:17:14                 245        00:00:10:05
                              200
                  |---------|
                      100


Generated by this adapter code
TITLE: CrossDissolve_Day-Night_Long_1
001  AX       V     C        00:00:05:11 00:00:07:08 00:00:00:00 00:00:01:21 (45)
002  AX       V     C        00:00:07:08 00:00:07:08 00:00:01:21 00:00:01:21 (0)
002  AX       V     D 100    00:00:09:07 00:00:17:15 00:00:01:21 00:00:14:09 (300)   <--- This seems wrong, like it's including the transition duration?

The track.range_of_child_at_index value for each child:
(0, 145.0)
(95.0, 195.0)
(145.0, 345.0)

  0              95        145
A |--------------|----------|        195 
D                |--------------------|              345             
B                           |---------|---------------|    
                                         200

I confirmed that this is what it needs to look like:
001  v330_21f V     C        00:00:05:11 00:00:07:08 00:00:00:00 00:00:01:21 (45)
002  v330_21f V     C        00:00:07:08 00:00:07:08 00:00:01:21 00:00:01:21 (0)
002  v330_23e V     D    100 00:00:09:07 00:00:17:15 00:00:01:21 00:00:10:05 (200)

jminor and others added 26 commits December 21, 2017 17:13
Added unit tests for the recursion behaviour of each_child.
Turned off lots of debugging print statements.
@mikemahony mikemahony force-pushed the edl_support_transition_writes branch from e9a1f2b to eae42b1 Compare January 2, 2018 23:01
@mikemahony mikemahony force-pushed the edl_support_transition_writes branch from eae42b1 to 1dc8f64 Compare January 2, 2018 23:02
@jminor jminor force-pushed the aaf_adapter branch 2 times, most recently from c6e5014 to 58e2505 Compare January 8, 2018 19:35
jminor pushed a commit that referenced this pull request Oct 9, 2018
Summary: This change merges the entire OTIO wiki into the main OTIO repo. This lets us version the documentation along with the code, and makes it easier to publish static documentation (e.g. to readthedocs.org).

Details:

* Remove old documentation files

They are out of date, and the generated ones for the API should not be checked into source control. By creating them at build time you can run tests against the documentation and ensure that the examples in the documentation are up to date with the code that has been written.

Additionally, I'm moving the doc folder over to docs in order to keep parity with the plurality of 'tests'

* Add MANIFEST.in file

By adding a Manifest file, we control what will get built into the final package so that we're not adding files that don't need to go into the package on pypi. This will be run in the tox step that will be added in a commit that is coming up which will build out a temporary distribution and check that the results in the output package file match up with what we've defined in here. I'd also like to exclude the examples folder, but if I exclude it, everything in my virtualenv goes nuts and breaks and I can't figure out why and I don't feel like putting more time into that.

* Migrate Wiki over to tutorials and generate all API documentation

Migrating the Wiki was actually the easy part. By adding the recommonmark sphinx plugin, I could just download the Wiki from Github and use the Markdown files that existed, with slight changes to fit into the sphinx-rtd-theme. NOTE that this isn't done or perfect, but it's 95% of the work done. So you might be wondering why one would want to move the wiki over to documentation in the project and the answer is simple - the wiki is not version controlled in alignment with the package and releases of the package.

Building the documentation along with the version of the package allows the user to see changes and explanation alongside whatever version they're using, while the Wiki does not. Yes, they will have to generate it, but with the integration of tox (coming in a separate commit), we make that so much easier on the user with the command `tox -e build-docs`. This practice also helps encourage developers to keep updating their documentation and it's worked great at the last few studios I've worked at and implemented it at.

* Add tox to dev dependencies and add tox file

Since this is a new dependency, you'll have to rerun the install command with `pip install -e .[dev]`, deactivate your virtualenv, and then source it again. After that, you only have to run `tox` when you're sourced inside of your virtualenv to run all the tests, and you can run `tox -e build-docs` to build the documentation. This is not finished yet, I plan to add a couple more steps to the tox script in a coming commit, but for the purpose of running tests and building documentation, this works.

If a tox environment ever gets funky, you can run `tox -r` to rebuild the environments. I have only ever had to do that once.

* Update file documentation to build API docs properly

Since I added the ability for Sphinx to build ALL of the API documentation, there were docstrings that were never touched by the build stuff that was set up previously, and thus had issues that prevented documentation from building. This was a rather annoying chore to have to go through, and I should not have messed with any code at all, it should have only been docstrings getting messed with (minus the import statements in tests). I would appreciate you all looking at this and making sure, though.

* Documentation formatting updates

Adding images, formatting, fixing tables, making links work.

* Add ReadTheDocs configuration file

* Add apidoc to Sphinx building and move module tree in index

* Cleanup for conf.py file and adding documentation link to README

* Flake8 cleanup and update manifest file

* Remove pip installs from Travis, change pep8 to pycodestyle, add to tox

* Test moving coverage call to tox

* Change py.test command to module being called from python for Travis

* Add tox-travis plugin to travis file

* Fix coverage on Travis

* Actually fix coverage on Travis and in Tox

* Add coverage dependency to travis pip installation

Didn't think about the fact that it should be needed outside of the virtualenvs that Tox creats

* Minor fixes to new doc system (#3)

* Fixed indentation issue in doc string.

* Changed Makefile doc-html target to run new tox -e build-docs.

* Put lambda idiom back the way it was.

* Adjust version string in conf.py for documentation
@jminor
Copy link
Owner

jminor commented Oct 10, 2018

Hey @mikemahony did all of this get merged into master? This old PR onto my repo is still here. I wanted to make sure it all got in before I close this.

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

Successfully merging this pull request may close these issues.

2 participants