Remove "rate" argument from "opentime.to_timecode"

[ssteinbach]

follow on to #612 - remove the rate argument and avoid the implicit rescale_to in to_timecode.

The confusing thing is when rate and dropframe/non-dropframe get conflated.
See this:

>>> import opentimelineio as otio 
>>> rt = otio.opentime.from_frames(107957, 29.97)
# "I want a non drop frame timecode for this 29.97 media (ignoring the 29.97
# being the wrong NTSC value for a second)"

# because 30 is the non-drop frame rate, I pass that in as an argument, not 
# realizing it'll rescale to 30
>>> otio.opentime.to_timecode(rt, 30, drop_frame=False)
u'01:00:02:05' 

# What the author (probably) intended
>>> otio.opentime.to_timecode(rt, 29.97, drop_frame=False)
u'00:59:58:17

# Gets you back to #1, but this time with the ; instead of the :
>>> otio.opentime.to_timecode(rt, 29.97, drop_frame=True)
u'01:00:02;05' (also probably not what you wanted?)

# (btw these are from one of the unit tests that we needed to adjust)

[meshula]

I prefer this explicit conversion to the embedded rate, rather than the existing code where the rate must be passed around. The existing code seems prone to coding errors.

[jminor]

Here's my thinking. When you pass drop_frame=True/False it should affect the labeling of the output, but not the position in time. It is still the same frame at the same moment in time, it just outputs a timecode string with a different scheme. The only time adjustment is a snapping/rounding to a frame, since timecode doesn't have fractional frames as far as I am aware.

If you want to actually rescale the time value to be at a different moment in time (e.g. scaling from 30fps playback to 30000/1001 NTSC playback) then the caller can do the rescale themselves based on the desired workflow.

For cases where you're going to/from matching pairs of integer and NTSC rates, the rate difference is subtle, but important. In practice, I would guess, that you often don't want to rescale the value, but rather just set the rate, keeping the value as an integer frame number: to_timecode(from_frames(duration.to_frames(), 30000/1001), drop_frame=True) By doing this, you are actually shifting in absolute time.

However, if you're switching from wildly different rates (e.g. 24 to 60) then the rescale is more likely to be desired. Again, the caller will know what they want, and to_timecode can just take what it was given: to_timecode(duration.rescaled_to(60), drop_frame=False) Here you're not shifting in time, just snapping to 60fps.

If these are too verbose, then perhaps two variants could be supported?

• to_timecode(t, replace_rate=r, drop_frame=d)
• to_timecode(t, scale_rate=r, drop_frame=d)

[reinecke]

This solution trusts the embedded rate part of RationalTime is the media FPS, that is not a correct assumption. The rate used in RationalTime instances often is the media frame rate, but it isn't always. For instance, in the case where a user is preserving exact rational values for 23.976 content, you'll end up with frame number 27 expressed as RationalTime(27027, 24000). Additionally, you could have a timeline where the edit rate (the timebase in which you're expressing edit decisions) may not match your original media rate. This can happen in certain high frame rate workflows where 48fps content edits are authored at 24fps resolution to keep different variants of the cut in-line with one another.
Given the embedded rate should never be relied on, there is no correct usage of this version of the method without using rescaled_to first. This seems incredibly prone to user error.

[ssteinbach]

We talked about this further offline and I understand better what @reinecke is driving at. He points out that as a result of normal math, RationalTime's denominator will mutate if not all rates are the same, and thus the programmer should not expect that the RationalTime they pass into to_timecode should be in the rate that they want to evaluate the timecode of. I think we need to think about this some more before we accept it.

[jminor]

Another way to make this clear would be to change to_timecode so that it takes a frame number instead of a RationalTime: to_timecode(frame_number, rate, drop_frame=d)

That would completely separate the responsibility for rescaling, snapping, etc.