[frame_timecode] Fix and validate rounding behavior when converting t…

…o string #354

scenedetect/frame_timecode.py

@@ -73,6 +73,10 @@
 MAX_FPS_DELTA: float = 1.0 / 100000
 """Maximum amount two framerates can differ by for equality testing."""
 
+_SECONDS_PER_MINUTE = 60.0
+_SECONDS_PER_HOUR = 60.0 * _SECONDS_PER_MINUTE
+_MINUTES_PER_HOUR = 60.0
+
 # TODO(0.6.3): Replace uses of Union[int, float, str] with TimecodeValue.
 TimecodeValue = Union[int, float, str]
 """Named type for values representing timecodes. Must be in one of the following forms:
@@ -201,22 +205,27 @@ def get_timecode(self, precision: int = 3, use_rounding: bool = True) -> str:
         """
         # Compute hours and minutes based off of seconds, and update seconds.
         secs = self.get_seconds()
-        base = 60.0 * 60.0
-        hrs = int(secs / base)
-        secs -= (hrs * base)
-        base = 60.0
-        mins = int(secs / base)
-        secs -= (mins * base)
-        # Convert seconds into string based on required precision.
-        if precision > 0:
-            if use_rounding:
-                secs = round(secs, precision)
-            msec = format(secs, '.%df' % precision)[-precision:]
-            secs = '%02d.%s' % (int(secs), msec)
-        else:
-            secs = '%02d' % int(round(secs, 0)) if use_rounding else '%02d' % int(secs)
+        hrs = int(secs / _SECONDS_PER_HOUR)
+        secs -= (hrs * _SECONDS_PER_HOUR)
+        mins = int(secs / _SECONDS_PER_MINUTE)
+        secs = max(0.0, secs - (mins * _SECONDS_PER_MINUTE))
+        if use_rounding:
+            secs = round(secs, precision)
+        secs = min(_SECONDS_PER_MINUTE, secs)
+        # Guard against emitting timecodes with 60 seconds after rounding/floating point errors.
+        if int(secs) == _SECONDS_PER_MINUTE:
+            secs = 0.0
+            mins += 1
+            if mins >= _MINUTES_PER_HOUR:
+                mins = 0
+                hrs += 1
+        # We have to extend the precision by 1 here, since `format` will round up.
+        msec = format(secs, '.%df' % (precision + 1)) if precision else ''
+        # Need to include decimal place in `msec_str`.
+        msec_str = msec[-(2 + precision):-1]
+        secs_str = f"{int(secs):02d}{msec_str}"
         # Return hours, minutes, and seconds as a formatted timecode string.
-        return '%02d:%02d:%s' % (hrs, mins, secs)
+        return '%02d:%02d:%s' % (hrs, mins, secs_str)
 
     # TODO(v1.0): Add a `previous` property to replace the existing one and deprecate this getter.
     def previous_frame(self) -> 'FrameTimecode':

tests/test_frame_timecode.py

@@ -275,3 +275,23 @@ def test_identity(frame_num, fps):
     assert FrameTimecode(frame_time_code.get_frames(), fps=fps) == frame_time_code
     assert FrameTimecode(frame_time_code.get_seconds(), fps=fps) == frame_time_code
     assert FrameTimecode(frame_time_code.get_timecode(), fps=fps) == frame_time_code
+
+
+def test_precision():
+    """Test rounding and precision, which has implications for rounding behavior."""
+
+    fps = 1000.0
+
+    assert FrameTimecode(110, fps).get_timecode(precision=2, use_rounding=True) == "00:00:00.11"
+    assert FrameTimecode(110, fps).get_timecode(precision=2, use_rounding=False) == "00:00:00.11"
+    assert FrameTimecode(110, fps).get_timecode(precision=1, use_rounding=True) == "00:00:00.1"
+    assert FrameTimecode(110, fps).get_timecode(precision=1, use_rounding=False) == "00:00:00.1"
+    assert FrameTimecode(110, fps).get_timecode(precision=0, use_rounding=True) == "00:00:00"
+    assert FrameTimecode(110, fps).get_timecode(precision=0, use_rounding=False) == "00:00:00"
+
+    assert FrameTimecode(990, fps).get_timecode(precision=2, use_rounding=True) == "00:00:00.99"
+    assert FrameTimecode(990, fps).get_timecode(precision=2, use_rounding=False) == "00:00:00.99"
+    assert FrameTimecode(990, fps).get_timecode(precision=1, use_rounding=True) == "00:00:01.0"
+    assert FrameTimecode(990, fps).get_timecode(precision=1, use_rounding=False) == "00:00:00.9"
+    assert FrameTimecode(990, fps).get_timecode(precision=0, use_rounding=True) == "00:00:01"
+    assert FrameTimecode(990, fps).get_timecode(precision=0, use_rounding=False) == "00:00:00"

website/pages/changelog.md

@@ -38,6 +38,7 @@ This release focuses on bugfixes and quality of life improvements. This has help
  - [bugfix] Fix `SceneManager.detect_scenes` warning when `duration` or `end_time` are specified as timecode strings [#346](https://github.com/Breakthrough/PySceneDetect/issues/346)
  - [improvement] When converting strings representing seconds to `FrameTimecode`, the `s` suffix is now optional, and whitespace is ignored (note that values without decimal places are still interpreted as frame numbers)
  - [improvement] The `VideoCaptureAdapter` in `scenedetect.backends.opencv` now attempts to report duration if known
+ - [bugfix] Ensure correct string conversion behavior for `FrameTimecode` when rounding is enabled [#354](https://github.com/Breakthrough/PySceneDetect/issues/354)
 
 ### 0.6.2 (July 23, 2023)