[timecode] Fix some failing timecode tests after updated arithmetic

Author: Breakthrough

scenedetect/common.py

@@ -210,9 +210,10 @@ def __init__(
         # Process the timecode value, storing it as an exact number of frames.
         if isinstance(timecode, str):
             self._seconds = self._timecode_to_seconds(timecode)
-            self._frame_num = self._seconds_to_frames(self._timecode_to_seconds(timecode))
+            self._frame_num = self._seconds_to_frames(self._seconds)
         else:
             self._frame_num = self._parse_timecode_number(timecode)
+            self._seconds = timecode if isinstance(timecode, float) else None
 
     @property
     def frame_num(self) -> ty.Optional[int]:
@@ -379,14 +380,14 @@ def _timecode_to_seconds(self, input: str) -> float:
         Raises:
             ValueError: Value could not be parsed correctly.
         """
-        assert self._framerate is not None
+        assert self._framerate is not None and self._framerate > MAX_FPS_DELTA
         input = input.strip()
         # Exact number of frames N
         if input.isdigit():
             timecode = int(input)
             if timecode < 0:
                 raise ValueError("Timecode frame number must be positive.")
-            return timecode * self.framerate
+            return timecode / self.framerate
         # Timecode in string format 'HH:MM:SS[.nnn]' or 'MM:SS[.nnn]'
         elif input.find(":") >= 0:
             values = input.split(":")
@@ -438,33 +439,45 @@ def _get_other_as_frames(self, other: ty.Union[int, float, str, "FrameTimecode"]
     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
         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 __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
         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
         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
         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
         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
         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)
@@ -495,11 +508,14 @@ def __iadd__(self, other: ty.Union[int, float, str, "FrameTimecode"]) -> "FrameT
             self._frame_num = None
             return self
 
-        if self._seconds and other._seconds:
+        other_has_seconds = isinstance(other, FrameTimecode) and other._seconds
+        if self._seconds is not None and other_has_seconds:
             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
 
     def __add__(self, other: ty.Union[int, float, str, "FrameTimecode"]) -> "FrameTimecode":
@@ -533,11 +549,14 @@ def __isub__(self, other: ty.Union[int, float, str, "FrameTimecode"]) -> "FrameT
             self._frame_num = None
             return self
 
-        if self._seconds and other._seconds:
+        other_has_seconds = isinstance(other, FrameTimecode) and other._seconds
+        if self._seconds is not None and other_has_seconds:
             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
 
     def __sub__(self, other: ty.Union[int, float, str, "FrameTimecode"]) -> "FrameTimecode":
@@ -585,3 +604,7 @@ def _get_other_as_seconds(self, other: ty.Union[int, float, str, "FrameTimecode"
         if isinstance(other, FrameTimecode):
             return other.seconds
         raise TypeError("Unsupported type for performing arithmetic with FrameTimecode.")
+
+
+def _compare_as_fixed(a: FrameTimecode, b: ty.Any) -> bool:
+    return a._framerate is not None and isinstance(b, FrameTimecode) and b._framerate is not None

tests/test_cli.py

@@ -691,7 +691,7 @@ def test_cli_load_scenes_with_time_frames():
 
 
 def test_cli_load_scenes_round_trip():
-    """Verify we can use `load-scenes` with the `time` command and get the desired output."""
+    """Verify we can use `load-scenes` and get the same scenes as output with `list-scenes`."""
     scenes_csv = """
 Scene Number,Start Frame
 1,49

tests/test_timecode.py

@@ -187,7 +187,8 @@ 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"
-    assert FrameTimecode(timecode="00:00:01.501", fps=10).get_timecode() == "00:00:01.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"
 
 
@@ -199,12 +200,6 @@ def test_equality():
     assert x == FrameTimecode(timecode=1.0, fps=10.0)
     assert x != FrameTimecode(timecode=10.0, fps=10.0)
     assert x != FrameTimecode(timecode=10.0, fps=10.0)
-    # Comparing FrameTimecodes with different framerates raises a TypeError.
-    with pytest.raises(ValueError):
-        assert x == FrameTimecode(timecode=1.0, fps=100.0)
-    with pytest.raises(ValueError):
-        assert x == FrameTimecode(timecode=1.0, fps=10.1)
-
     assert x == FrameTimecode(x)
     assert x == FrameTimecode(1.0, x)
     assert x == FrameTimecode(10, x)
@@ -232,44 +227,32 @@ def test_equality():
 
     assert FrameTimecode(timecode="00:00:00.5", fps=10) == "00:00:00.500"
     assert FrameTimecode(timecode="00:00:01.500", fps=10) == "00:00:01.500"
-    assert FrameTimecode(timecode="00:00:01.500", fps=10) == "00:00:01.501"
-    assert FrameTimecode(timecode="00:00:01.500", fps=10) == "00:00:01.502"
-    assert FrameTimecode(timecode="00:00:01.500", fps=10) == "00:00:01.508"
-    assert FrameTimecode(timecode="00:00:01.500", fps=10) == "00:00:01.509"
-    assert FrameTimecode(timecode="00:00:01.519", fps=10) == "00:00:01.510"
 
 
 def test_addition():
     """Test FrameTimecode addition (+/+=, __add__/__iadd__) operator."""
     x = FrameTimecode(timecode=1.0, fps=10.0)
     assert x + 1 == FrameTimecode(timecode=1.1, fps=10.0)
     assert x + 1 == FrameTimecode(1.1, x)
+    assert x + 10 == "00:00:02.000", str(x + 10)
     assert x + 10 == 20
     assert x + 10 == 2.0
-
     assert x + 10 == "00:00:02.000"
 
-    with pytest.raises(ValueError):
-        assert FrameTimecode("00:00:02.000", fps=20.0) == x + 10
-
 
 def test_subtraction():
     """Test FrameTimecode subtraction (-/-=, __sub__) operator."""
     x = FrameTimecode(timecode=1.0, fps=10.0)
     assert (x - 1) == FrameTimecode(timecode=0.9, fps=10.0)
     assert x - 2 == FrameTimecode(0.8, x)
     assert x - 10 == FrameTimecode(0.0, x)
-    # TODO(v1.0): Allow negative values
+    # TODO(v1.0): Allow negative values. For now we clamp.
     assert x - 11 == FrameTimecode(0.0, x)
     assert x - 100 == FrameTimecode(0.0, x)
-
     assert x - 1.0 == FrameTimecode(0.0, x)
     assert x - 100.0 == FrameTimecode(0.0, x)
-
     assert x - 1 == FrameTimecode(timecode=0.9, fps=10.0)
-
-    with pytest.raises(ValueError):
-        assert FrameTimecode("00:00:02.000", fps=20.0) == x - 10
+    assert FrameTimecode("00:00:00.000", fps=20.0) == x - 10
 
 
 @pytest.mark.parametrize("frame_num,fps", [(1, 1), (61, 14), (29, 25), (126, 24000 / 1001.0)])