Skip to content

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
@ssteinbach ssteinbach requested a review from reinecke November 6, 2019 19:23
@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"

@ssteinbach ssteinbach force-pushed the timecode_rate_is_ignored branch from 5b8687e to 5ecba56 Compare March 13, 2020 22:08
@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