fix(keyframes): expire cached probe failures so they can recover

CHANGELOG.md

@@ -4,6 +4,16 @@ All notable changes to this project are documented here. The format follows
 [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and versions follow
 [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Fixed
+
+- `probe_keyframes` no longer caches a failed probe forever: `None` is now
+  remembered for `FAILED_PROBE_RETRY_SEC` (30 s), so a transient ffprobe
+  failure on unchanged media recovers, and repeated failures are re-probed at
+  most once per window. Successful probes are still cached, and a changed file
+  is still re-probed immediately.
+
 ## 0.1.1 - 2026-08-03
 
 ### Added

README.md

@@ -304,7 +304,9 @@ nothing.
   video and audio.
 - **Probing costs a full packet scan.** Roughly a second per hour of footage on
   a local disk. Results are cached per (path, mtime, size) for the life of the
-  process; pass `keyframes=[…]` to skip it entirely.
+  process; a failed probe (`None`) is remembered for only 30 seconds
+  (`FAILED_PROBE_RETRY_SEC`), so a transient failure recovers on a later call
+  without a restart. Pass `keyframes=[…]` to skip probing entirely.
 - **Open-GOP sources are not handled specially.** With open GOPs a "keyframe"
   can still depend on the previous group, and the first frames after a cut may
   show artefacts. Encode with `open-gop=0` if you control the source.

src/keycut/__init__.py

@@ -33,6 +33,7 @@
 )
 from .keyframes import (
     DEFAULT_GOP_DURATION_SEC,
+    FAILED_PROBE_RETRY_SEC,
     clear_keyframe_cache,
     estimate_gop_duration,
     keyframe_at_or_before,
@@ -60,6 +61,7 @@
 __all__ = [
     "DEFAULT_CRF",
     "DEFAULT_GOP_DURATION_SEC",
+    "FAILED_PROBE_RETRY_SEC",
     "MaskLike",
     "Range",
     "Segment",

src/keycut/keyframes.py

@@ -10,6 +10,7 @@
 import bisect
 import logging
 import subprocess
+import time
 from collections.abc import Sequence
 from itertools import pairwise
 from pathlib import Path
@@ -26,10 +27,21 @@
 failed entirely.
 """
 
+FAILED_PROBE_RETRY_SEC = 30.0
+"""How long, in seconds, a failed probe is remembered before it may be retried.
+
+Successful probes are cached for the life of the process; a failure is only
+remembered this long, so a transient problem (ffprobe briefly unavailable, a
+file still being written) can recover, while a persistent one is re-probed at
+most once per window rather than on every call.
+"""
+
 # Keyframe timestamps keyed by (resolved path, mtime_ns, size) so a file is
 # probed at most once per process even when several extractions share it.
-# A failed probe caches ``None`` so it is not retried in a loop.
-_keyframe_cache: dict[tuple[str, int, int], list[float] | None] = {}
+# Each value is ``(keyframes, retry_at)``. A success has ``retry_at=None`` and
+# never expires. A failed probe caches ``(None, deadline)`` where ``deadline``
+# is a ``time.monotonic()`` reading; the entry is ignored once it passes.
+_keyframe_cache: dict[tuple[str, int, int], tuple[list[float] | None, float | None]] = {}
 
 
 def clear_keyframe_cache() -> None:
@@ -58,7 +70,9 @@ def probe_keyframes(
     join.
 
     Because it walks the whole packet index, expect roughly a second per hour
-    of footage on a local disk. Results are cached per (path, mtime, size).
+    of footage on a local disk. Results are cached per (path, mtime, size); a
+    failed probe is cached only for :data:`FAILED_PROBE_RETRY_SEC` seconds, so
+    a transient failure recovers on a later call.
 
     ``None`` means "unknown", not "no keyframes". Callers must treat it as a
     reason to re-encode rather than a reason to cut blind.
@@ -71,8 +85,13 @@ def probe_keyframes(
         return None
 
     key = (str(path.resolve()), st.st_mtime_ns, st.st_size)
-    if use_cache and key in _keyframe_cache:
-        return _keyframe_cache[key]
+    if use_cache:
+        cached = _keyframe_cache.get(key)
+        if cached is not None:
+            cached_keyframes, retry_at = cached
+            if retry_at is None or time.monotonic() < retry_at:
+                return cached_keyframes
+            del _keyframe_cache[key]
 
     cmd = [
         ffprobe_bin,
@@ -123,7 +142,8 @@ def probe_keyframes(
         )
 
     if use_cache:
-        _keyframe_cache[key] = keyframes
+        retry_at = None if keyframes is not None else time.monotonic() + FAILED_PROBE_RETRY_SEC
+        _keyframe_cache[key] = (keyframes, retry_at)
     return keyframes
 
 

tests/test_keyframes.py

@@ -8,6 +8,7 @@
 
 from keycut import (
     DEFAULT_GOP_DURATION_SEC,
+    FAILED_PROBE_RETRY_SEC,
     clear_keyframe_cache,
     estimate_gop_duration,
     keyframe_at_or_before,
@@ -145,3 +146,92 @@ def _fake_run(cmd, *a, **kw):
             probe_keyframes(master, use_cache=False)
             probe_keyframes(master, use_cache=False)
         assert len(calls) == 2
+
+
+class TestFailedProbeRecovery:
+    """A failed probe is remembered only briefly, so a transient failure
+    (ffprobe momentarily unavailable, file still being written) can recover
+    without a process restart, while a persistent failure is not re-run on
+    every call."""
+
+    def _master(self, tmp_path):
+        master = tmp_path / "m.mp4"
+        master.write_bytes(b"x")
+        return master
+
+    def test_failure_then_success_recovers_after_the_retry_window(self, tmp_path):
+        master = self._master(tmp_path)
+        clock = [1000.0]
+        results = [
+            MagicMock(returncode=1, stdout=b"", stderr=b"transient"),
+            MagicMock(returncode=0, stdout=b"0.0,K__\n2.0,K__\n", stderr=b""),
+        ]
+        calls = []
+
+        def _fake_run(cmd, *a, **kw):
+            calls.append(1)
+            return results[len(calls) - 1]
+
+        with (
+            patch("keycut.keyframes.subprocess.run", side_effect=_fake_run),
+            patch("keycut.keyframes.time.monotonic", side_effect=lambda: clock[0]),
+        ):
+            assert probe_keyframes(master) is None
+            clock[0] += FAILED_PROBE_RETRY_SEC + 1
+            assert probe_keyframes(master) == [0.0, 2.0]
+            assert probe_keyframes(master) == [0.0, 2.0]
+        assert len(calls) == 2, "the recovered result is cached like any success"
+
+    def test_repeated_failures_inside_the_window_do_not_reprobe(self, tmp_path):
+        master = self._master(tmp_path)
+        clock = [1000.0]
+        calls = []
+
+        def _fake_run(cmd, *a, **kw):
+            calls.append(1)
+            return MagicMock(returncode=1, stdout=b"", stderr=b"boom")
+
+        with (
+            patch("keycut.keyframes.subprocess.run", side_effect=_fake_run),
+            patch("keycut.keyframes.time.monotonic", side_effect=lambda: clock[0]),
+        ):
+            for _ in range(5):
+                assert probe_keyframes(master) is None
+                clock[0] += FAILED_PROBE_RETRY_SEC / 10
+        assert len(calls) == 1
+
+    def test_persistent_failure_reprobes_at_most_once_per_window(self, tmp_path):
+        master = self._master(tmp_path)
+        clock = [1000.0]
+        calls = []
+
+        def _fake_run(cmd, *a, **kw):
+            calls.append(1)
+            return MagicMock(returncode=1, stdout=b"", stderr=b"boom")
+
+        with (
+            patch("keycut.keyframes.subprocess.run", side_effect=_fake_run),
+            patch("keycut.keyframes.time.monotonic", side_effect=lambda: clock[0]),
+        ):
+            assert probe_keyframes(master) is None
+            clock[0] += FAILED_PROBE_RETRY_SEC + 1
+            assert probe_keyframes(master) is None
+            assert probe_keyframes(master) is None
+        assert len(calls) == 2
+
+    def test_changed_file_is_reprobed_inside_the_failure_window(self, tmp_path):
+        master = self._master(tmp_path)
+        calls = []
+
+        def _fake_run(cmd, *a, **kw):
+            calls.append(1)
+            return MagicMock(returncode=1, stdout=b"", stderr=b"boom")
+
+        with (
+            patch("keycut.keyframes.subprocess.run", side_effect=_fake_run),
+            patch("keycut.keyframes.time.monotonic", return_value=1000.0),
+        ):
+            assert probe_keyframes(master) is None
+            master.write_bytes(b"longer content")
+            assert probe_keyframes(master) is None
+        assert len(calls) == 2, "file identity still invalidates a cached failure"