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

Timecode rate is ignored #612

Conversation

ssteinbach
Copy link
Collaborator

Fix: #606

This addresses the issue as written... but because having an implicit rescale in to_timecode seems iffy, I'm also proposing another PR that drops the rate argument from to_timecode entirely in favor of explicitly calling rescale_to in calling code. Thoughts?

@ssteinbach ssteinbach added the bug A problem, flaw, or broken functionality. label Nov 6, 2019
@ssteinbach ssteinbach added this to the Public Beta 12 milestone Nov 6, 2019
@codecov-io
Copy link

codecov-io commented Nov 6, 2019

Codecov Report

Merging #612 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #612      +/-   ##
==========================================
+ Coverage   81.67%   81.69%   +0.01%     
==========================================
  Files          72       72              
  Lines        2729     2731       +2     
==========================================
+ Hits         2229     2231       +2     
  Misses        500      500
Flag Coverage Δ
#py27 81.67% <100%> (?)
#py36 81.67% <100%> (-0.01%) ⬇️
#py37 81.67% <100%> (-0.01%) ⬇️
Impacted Files Coverage Δ
src/opentime/rationalTime.cpp 85.71% <100%> (-0.15%) ⬇️
...eio/opentimelineio-bindings/otio_anyDictionary.cpp 100% <0%> (ø) ⬆️
...elineio/opentimelineio-bindings/otio_anyVector.cpp 100% <0%> (ø) ⬆️
...ntimelineio/opentimelineio-bindings/otio_utils.cpp 87.2% <0%> (+0.15%) ⬆️

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 0c0087b...5ecba56. Read the comment docs.

Copy link
Collaborator

@reinecke reinecke left a comment

Choose a reason for hiding this comment

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

Marking review as "request changes"

src/opentime/rationalTime.cpp Outdated Show resolved Hide resolved
@ssteinbach ssteinbach merged commit 765d3c6 into AcademySoftwareFoundation:master Mar 13, 2020
@ssteinbach ssteinbach deleted the timecode_rate_is_ignored branch March 13, 2020 22:20
andrewmoore-nz added a commit to andrewmoore-nz/OpenTimelineIO that referenced this pull request Apr 19, 2020
* master: (23 commits)
  Indicate Empty track in otioview and display track name (AcademySoftwareFoundation#677)
  FCP 7 XML - Fix failure on empty name tags (AcademySoftwareFoundation#674)
  xges: Effects and Markers Support (AcademySoftwareFoundation#609)
  Add List of Supported Formats to Conform.py Help Text (AcademySoftwareFoundation#676)
  Update Copyright/License on ffmpeg_burnins.py (AcademySoftwareFoundation#679)
  Fix the windows build (AcademySoftwareFoundation#669)
  Version bump to beta 13
  Set final version for beta 12.0 (AcademySoftwareFoundation#665)
  Rodeofx fix cmx 3600 multiple markers per clip issue 593 (AcademySoftwareFoundation#664)
  Tweaks to cmake so that pip and local builds both work (AcademySoftwareFoundation#663)
  Fixed issue where CMX3600 adapter would try to add the same clip to multiple tracks. Also moved some code out of a loop it didn't need to be in. (AcademySoftwareFoundation#644)
  RV adapter metadata updates (AcademySoftwareFoundation#640)
  Expose json indent to the otio_json adapter (AcademySoftwareFoundation#641)
  fix otioconvert for Kdenlive with python3 (AcademySoftwareFoundation#646)
  Add basic debugging instructions to quickstart. (AcademySoftwareFoundation#655)
  Add kdenlive adapter to adapters list. (AcademySoftwareFoundation#661)
  Timecode rate is ignored (AcademySoftwareFoundation#612)
  Detect if plugin doesn't have a docstring and return an error which says which plugin it is that doesn't have the docstring. (AcademySoftwareFoundation#635)
  Add hook function args to otioview and otioconvert (AcademySoftwareFoundation#651)
  Updating Copyright notices (AcademySoftwareFoundation#660)
  ...

# Conflicts:
#	contrib/opentimelineio_contrib/adapters/advanced_authoring_format.py
#	src/py-opentimelineio/opentimelineio/adapters/fcp_xml.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A problem, flaw, or broken functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

to_timecode ignoring RationalTime's rate
3 participants