Skip to content

Conversation

@davidbaraff
Copy link
Contributor

@davidbaraff davidbaraff commented Apr 9, 2019

Port of the OpenTimelineIO core into C++, wrapped with pybind11 python bindings. Only port of the core, all adapters and unit tests still run in python.

Also includes very preliminary swift bindings.

David Baraff and others added 30 commits April 8, 2019 14:43
added #pragma once
transactional semantics on python collection mutations
@davidbaraff davidbaraff added this to the 1.0 Release milestone Apr 9, 2019
@meshula
Copy link
Collaborator

meshula commented Apr 9, 2019

This is very exciting, I'm looking forward to playing with it.

My preliminary comments are that

  1. I think it would make sense to start with cmake 3 (3.5 is a safe bet)
  2. The AnyVector and AnyMap are subclassed, with using, from std containers. This will be a road block when it comes time to make this library available as a shared library for the msvc ABI, because the std containers are not decorated for shared visibility, and the ABI has rules about decorations on bases when subclasses are exported. A simple fix is to use composition instead of inheritance. That doesn't change the implementation much, as inlines are no more expensive than using hoists.
  3. Again, very excited to see this!

@davidbaraff
Copy link
Contributor Author

davidbaraff commented Apr 9, 2019 via email

@meshula
Copy link
Collaborator

meshula commented Apr 9, 2019

Subclassing Iex::BaseExc from std::string caused crashes for years on Windows shared library build of OpenEXR, we only just finally were able to merge the fixes, so it's kind of a once bitten twice shy sort of thing.

@meshula
Copy link
Collaborator

meshula commented Apr 9, 2019

Thinking.... There might be a pattern that gives you what you need without a mess of forwarding. Maybe don't change anything until I have a play with it?

@davidbaraff
Copy link
Contributor Author

davidbaraff commented Apr 9, 2019 via email

@codecov-io
Copy link

Codecov Report

Merging #486 into cxx will decrease coverage by 40.87%.
The diff coverage is 83.52%.

Impacted file tree graph

@@             Coverage Diff             @@
##              cxx     #486       +/-   ##
===========================================
- Coverage   88.23%   47.35%   -40.88%     
===========================================
  Files          68       66        -2     
  Lines        6900     6105      -795     
===========================================
- Hits         6088     2891     -3197     
- Misses        812     3214     +2402
Impacted Files Coverage Δ
.../opentimelineio_contrib/adapters/ffmpeg_burnins.py 0% <ø> (ø)
...ntimelineio_contrib/adapters/tests/test_burnins.py 92.85% <ø> (ø)
...ntrib/opentimelineio_contrib/adapters/extern_rv.py 0% <ø> (ø)
...melineio_contrib/adapters/extern_maya_sequencer.py 0% <ø> (ø)
.../opentimelineio_contrib/adapters/maya_sequencer.py 0% <ø> (ø)
contrib/opentimelineio_contrib/adapters/rv.py 0% <ø> (ø)
...entimelineio/opentimelineio/console/otioconvert.py 93.42% <ø> (ø)
...-opentimelineio/opentimelineio/console/otiostat.py 71.76% <ø> (ø)
...-opentimelineio/opentimelineio/adapters/adapter.py 94.36% <ø> (ø)
...y-opentimelineio/opentimelineio/console/otiocat.py 96.29% <ø> (ø)
... and 85 more

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 878c67f...bd32b19. Read the comment docs.

@ssteinbach
Copy link
Collaborator

@meshula I'm going to land this into the Cxx branch and then open a pull request from the cxx branch to master to leave open for a while, if you have any comments for this branch you can post them there or contact us directly!

@ssteinbach ssteinbach merged commit 643a703 into AcademySoftwareFoundation:cxx Apr 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants