diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ec7cd5..8b2db7a 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index 9605de4..bf8373b 100644 --- a/README.md +++ b/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. diff --git a/src/keycut/__init__.py b/src/keycut/__init__.py index 5ddf631..7210e95 100644 --- a/src/keycut/__init__.py +++ b/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", diff --git a/src/keycut/keyframes.py b/src/keycut/keyframes.py index 173db99..17fbae4 100644 --- a/src/keycut/keyframes.py +++ b/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 diff --git a/tests/test_keyframes.py b/tests/test_keyframes.py index a309793..d3581a3 100644 --- a/tests/test_keyframes.py +++ b/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"