[timecode] Fix remaining timecode issues

Author: Breakthrough

scenedetect/_cli/controller.py

@@ -175,10 +175,15 @@ def _load_scenes(context: CliContext) -> ty.Tuple[SceneList, CutList]:
         if context.load_scenes_column_name not in csv_headers:
             raise ValueError("specified column header for scene start is not present")
         col_idx = csv_headers.index(context.load_scenes_column_name)
-        cut_list = sorted(
-            FrameTimecode(row[col_idx], fps=context.video_stream.frame_rate) - 1
-            for row in file_reader
-        )
+
+        def calculate_timecode(value: str) -> FrameTimecode:
+            # Assume other columns are in seconds except frame numbers.
+            if value.isdigit():
+                # Frame numbers start from index 1 in the CLI output so we correct for that.
+                return FrameTimecode(int(value) - 1, fps=context.video_stream.frame_rate)
+            return FrameTimecode(value, fps=context.video_stream.frame_rate)
+
+        cut_list = sorted(calculate_timecode(row[col_idx]) for row in file_reader)
         # `SceneDetector` works on cuts, so we have to skip the first scene and place the first
         # cut point where the next scenes starts.
         if cut_list:

scenedetect/common.py

@@ -207,17 +207,27 @@ def __init__(
         ):
             raise ValueError("Framerate must be positive and greater than zero.")
         self._framerate = float(fps)
-        # Process the timecode value, storing it as an exact number of frames.
+        # Process the timecode value, storing it as an exact number of frames only if required.
+        if isinstance(timecode, str) and timecode.isdigit():
+            timecode = int(timecode)
         if isinstance(timecode, str):
             self._seconds = self._timecode_to_seconds(timecode)
-            self._frame_num = self._seconds_to_frames(self._seconds)
+        elif isinstance(timecode, float):
+            if timecode < 0.0:
+                raise ValueError("Timecode frame number must be positive and greater than zero.")
+            self._seconds = timecode
+        elif isinstance(timecode, int):
+            if timecode < 0:
+                raise ValueError("Timecode frame number must be positive and greater than zero.")
+            self._frame_num = timecode
         else:
-            self._frame_num = self._parse_timecode_number(timecode)
-            self._seconds = timecode if isinstance(timecode, float) else None
+            raise TypeError("Timecode format/type unrecognized.")
 
     @property
     def frame_num(self) -> ty.Optional[int]:
         if self._timecode:
+            # We need to audit anything currently using this property to guarantee temporal
+            # consistency when handling VFR videos (i.e. no assumptions on fixed frame rate).
             warnings.warn(
                 message="TODO(https://scenedetect.com/issue/168): Update caller to handle VFR.",
                 stacklevel=2,
@@ -227,6 +237,8 @@ def frame_num(self) -> ty.Optional[int]:
             # time base itself.
             (num, den) = (self._timecode.time_base * self._timecode.pts).as_integer_ratio()
             return num / den
+        if self._seconds is not None:
+            return self._seconds_to_frames(self._seconds)
         return self._frame_num
 
     @property
@@ -306,19 +318,26 @@ def get_seconds(self) -> float:
         )
         return self.seconds
 
-    def get_timecode(self, precision: int = 3, use_rounding: bool = True) -> str:
+    def get_timecode(
+        self, precision: int = 3, use_rounding: bool = True, nearest_frame: bool = True
+    ) -> str:
         """Get a formatted timecode string of the form HH:MM:SS[.nnn].
 
         Args:
             precision: The number of decimal places to include in the output ``[.nnn]``.
             use_rounding: Rounds the output to the desired precision. If False, the value
                 will be truncated to the specified precision.
+            nearest_frame: Ensures that the timecode is moved to the nearest frame boundary if this
+                object has a defined framerate, otherwise has no effect.
 
         Returns:
             str: The current time in the form ``"HH:MM:SS[.nnn]"``.
         """
         # Compute hours and minutes based off of seconds, and update seconds.
-        secs = self.seconds
+        if nearest_frame and self.framerate:
+            secs = self.frame_num / self.framerate
+        else:
+            secs = self.seconds
         hrs = int(secs / _SECONDS_PER_HOUR)
         secs -= hrs * _SECONDS_PER_HOUR
         mins = int(secs / _SECONDS_PER_MINUTE)
@@ -440,7 +459,7 @@ def __eq__(self, other: ty.Union[int, float, str, "FrameTimecode"]) -> bool:
         if other is None:
             return False
         if _compare_as_fixed(self, other):
-            return self._frame_num == other._frame_num
+            return self.frame_num == other.frame_num
         if self._timecode or self._seconds is not None:
             return self.seconds == self._get_other_as_seconds(other)
         return self.frame_num == self._get_other_as_frames(other)
@@ -449,35 +468,35 @@ def __ne__(self, other: ty.Union[int, float, str, "FrameTimecode"]) -> bool:
         if other is None:
             return True
         if _compare_as_fixed(self, other):
-            return self._frame_num != other._frame_num
+            return self.frame_num != other.frame_num
         if self._timecode or self._seconds is not None:
             return self.seconds != self._get_other_as_seconds(other)
         return self.frame_num != self._get_other_as_frames(other)
 
     def __lt__(self, other: ty.Union[int, float, str, "FrameTimecode"]) -> bool:
         if _compare_as_fixed(self, other):
-            return self._frame_num < other._frame_num
+            return self.frame_num < other.frame_num
         if self._timecode or self._seconds is not None:
             return self.seconds < self._get_other_as_seconds(other)
         return self.frame_num < self._get_other_as_frames(other)
 
     def __le__(self, other: ty.Union[int, float, str, "FrameTimecode"]) -> bool:
         if _compare_as_fixed(self, other):
-            return self._frame_num <= other._frame_num
+            return self.frame_num <= other.frame_num
         if self._timecode or self._seconds is not None:
             return self.seconds <= self._get_other_as_seconds(other)
         return self.frame_num <= self._get_other_as_frames(other)
 
     def __gt__(self, other: ty.Union[int, float, str, "FrameTimecode"]) -> bool:
         if _compare_as_fixed(self, other):
-            return self._frame_num > other._frame_num
+            return self.frame_num > other.frame_num
         if self._timecode or self._seconds is not None:
             return self.seconds > self._get_other_as_seconds(other)
         return self.frame_num > self._get_other_as_frames(other)
 
     def __ge__(self, other: ty.Union[int, float, str, "FrameTimecode"]) -> bool:
         if _compare_as_fixed(self, other):
-            return self._frame_num >= other._frame_num
+            return self.frame_num >= other.frame_num
         if self._timecode or self._seconds is not None:
             return self.seconds >= self._get_other_as_seconds(other)
         return self.frame_num >= self._get_other_as_frames(other)
@@ -513,9 +532,11 @@ def __iadd__(self, other: ty.Union[int, float, str, "FrameTimecode"]) -> "FrameT
             self._seconds = max(0, self._seconds + other._seconds)
             return self
 
-        self._frame_num = max(0, self._frame_num + self._get_other_as_frames(other))
         if self._seconds is not None:
             self._seconds = max(0.0, self._seconds + self._get_other_as_seconds(other))
+            return self
+
+        self._frame_num = max(0, self._frame_num + self._get_other_as_frames(other))
         return self
 
     def __add__(self, other: ty.Union[int, float, str, "FrameTimecode"]) -> "FrameTimecode":
@@ -554,9 +575,11 @@ def __isub__(self, other: ty.Union[int, float, str, "FrameTimecode"]) -> "FrameT
             self._seconds = max(0, self._seconds - other._seconds)
             return self
 
-        self._frame_num = max(0, self._frame_num - self._get_other_as_frames(other))
         if self._seconds is not None:
             self._seconds = max(0.0, self._seconds - self._get_other_as_seconds(other))
+            return self
+
+        self._frame_num = max(0, self._frame_num - self._get_other_as_frames(other))
         return self
 
     def __sub__(self, other: ty.Union[int, float, str, "FrameTimecode"]) -> "FrameTimecode":
@@ -579,7 +602,9 @@ def __str__(self) -> str:
     def __repr__(self) -> str:
         if self._timecode:
             return f"{self.get_timecode()} [pts={self._timecode.pts}, time_base={self._timecode.time_base}]"
-        return "%s [frame=%d, fps=%.3f]" % (self.get_timecode(), self._frame_num, self._framerate)
+        if self._seconds is not None:
+            return f"{self.get_timecode()} [seconds={self._seconds}, fps={self._framerate}]"
+        return f"{self.get_timecode()} [frame_num={self._frame_num}, fps={self._framerate}]"
 
     def __hash__(self) -> int:
         if self._timecode:

tests/test_timecode.py

@@ -188,8 +188,14 @@ def test_get_timecode():
     assert FrameTimecode(timecode="00:00:02.0000", fps=1).get_timecode() == "00:00:02.000"
     assert FrameTimecode(timecode="00:00:00.5", fps=10).get_timecode() == "00:00:00.500"
     # If a value is provided in seconds, we store that value internally now.
-    assert FrameTimecode(timecode="00:00:01.501", fps=10).get_timecode() == "00:00:01.501"
-    assert FrameTimecode(timecode="00:01:00.000", fps=1).get_timecode() == "00:01:00.000"
+    assert (
+        FrameTimecode(timecode="00:00:01.501", fps=10).get_timecode(nearest_frame=False)
+        == "00:00:01.501"
+    )
+    assert (
+        FrameTimecode(timecode="00:00:01.501", fps=10).get_timecode(nearest_frame=True)
+        == "00:00:01.500"
+    )
 
 
 def test_equality():

website/pages/changelog.md

@@ -713,3 +713,4 @@ Although there have been minimal changes to most API examples, there are several
  * Deprecated functionality preserved from v0.6 now uses the `warnings` module
  * Add properties to access `frame_num`, `framerate`, and `seconds` from `FrameTimecode` instead of getter methods
  * Add new `Timecode` type to represent frame timings in terms of the video's source timebase
+ * Expand `FrameTimecode` representations to preserve accuracy (previously all timecodes were rounded to frame boundaries)