Timecodes generated by list-scenes may lead to off-by-one errors

[Breakthrough]

Description:

As discussed in #431, the timecodes outputted by the list-scenes command may lead to cuts being off by a frame. This is due to rounding as the output format is HH:MM:SS.nnn. Internally these values are stored as floats, with much more precision than three digits.

Suggested Fix:

When formatting the result, the final digit should not be rounded, but rather truncated.

[moi15moi]

You may want to check out this library: https://github.com/moi15moi/VideoTimestamps

You could use it or be inspired to see how you can avoid this error.

[Breakthrough]

I've been experimenting with overhauling the PySceneDetect timecode interfaces in a separate branch, but don't have a timeline for that yet (it's a rather major update).

You may want to check out this library: https://github.com/moi15moi/VideoTimestamps

You could use it or be inspired to see how you can avoid this error.

One major showstopper for switching to another library for timecodes is the ability to handle variable frame rate (VFR) and non-monotonic frame timing. For some additional context, the PySceneDetect timecode type does support all required rounding modes. The fix for this would be to change what rounding mode it's using, but it seems with this library you also need to be explicit about that, so I'm not sure if it gains much in terms of fixing this issue. Your library does seem to have a lot of functionality I would like for FrameTimecode, so I will definitely consider it if it handles VFR correctly!

[moi15moi]

For some additional context, the PySceneDetect timecode type does support all required rounding modes

I'm not sure what you mean by rounding modes.
ABCTimestamps.rounding_method from VideoTimestamps does require a rounding method, BUT it is only to guess the pts above the video lenght (which is clearly not something you need in your case).

But, since PySceneDetect use a video, it can technically get all the pts + timescale (or timebase) of the video, so there isn't any guessing needed, so there isn't any rounding methods required.

Note that using a float for the time is a bad idea. It should be a Fraction. Why?
Because:

$$\begin{gather} pts \in \mathbb{N} \\\ timebase \in \mathbb{Q}^{+} \\\ \end{gather}$$

Source for pts
Source for timebase

$time= pts \times timebase$. So, it means $time \in \mathbb{Q}$.
This mean we are 100% sure that a Fraction will correctly contain the time value and it will never have approximation error (like float).

[Breakthrough]

But, since PySceneDetect use a video, it can technically get all the pts + timescale (or timebase) of the video

PySceneDetect does use a fractional representation for timecodes, although the timebase is fixed to be the duration of a single frame (i.e. 1.0/FPS), and the numerator is always measured in frames. A big complication is that we support various video input backends, not all of which expose the PTS and timebase of a given frame. I believe only the PyAV backend provides this, but OpenCV and MoviePy do not.

so there isn't any guessing needed, so there isn't any rounding methods required.

The only limitation PySceneDetect has today is that it cannot deal with variable frame timing, but this is a separate issue from the one outlined here re: rounding. Values are only converted to seconds or timecodes when they are required for interop with other tools or output formats (e.g. the EDL output format requires SMPTE timecodes, OpenTimelineIO uses RationalTime which is similar to PySceneDetect). For splitting videos, we also use the ffmpeg command line interface, which requires converting all time values to seconds.

When splitting videos we use full precision the values, so this bug only affects the case of using the timestamp output (e.g. with the list-scenes command). This can sometimes lead to the issue as these timecodes might be slightly off.

This mean we are 100% sure that a Fraction will correctly contain the time value and it will never have approximation error (like float).

I agree with this approach and TIL that the standard library has a fractional type. This is similar to what I had in mind for my WIP branch, I was just using floats for simplicity in development. I agree using a fractional representation for timestamps where the base unit is provided by the container is the most accurate and correct way to do this.