diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b2db7a..50c4cb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,24 @@ 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 +## 0.1.2 - 2026-10-27 ### Fixed +- Every ffmpeg and ffprobe step now sizes its timeout to the file it reads, + at a 20 MB/s floor (`MIN_IO_BYTES_PER_SEC`, `io_timeout`), instead of a + flat window. The keyframe probe (600 s floor), a stream copy (1800 s floor) + and the concat (1800 s floor, twice the bytes with `faststart`) all read + the whole file. A 110 GB master on a slow mount took longer than 600 s to + probe, so keyframes came back unknown and every range fell back to a + re-encode. +- The re-encode fallback seeks on the input side (`-ss S -i master -ss 0 + -t D`) instead of the output side, so it decodes at most one GOP before + the cut rather than everything from the start of the file. It is still + frame-exact. The output-side `-ss 0` drops the stream-copied audio's + pre-cut packets, which the concat demuxer would otherwise play. Its + timeout is at least 20 s per second of footage + (`REENCODE_SEC_PER_SEC`), with a 3600 s floor. - `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 diff --git a/README.md b/README.md index d560382..9048fa4 100644 --- a/README.md +++ b/README.md @@ -161,8 +161,12 @@ extract_and_concat_ranges( Two things change. The merge refuses to close a gap containing an excluded second, however narrow — merging would re-admit exactly what you banned. And for any range where a copy would still snap back across an excluded second, -that one range is re-encoded with a frame-accurate seek (`-ss` after `-i`), -while every other range is still copied losslessly. +that one range is re-encoded with a frame-accurate seek, while every other +range is still copied losslessly. The re-encode seeks on the input side +(`-ss S -i master -ss 0 -t D`): the demuxer jumps to the keyframe before the +cut, the decoder drops frames up to the exact start, and the output-side +`-ss 0` drops the copied audio's pre-cut packets. That decodes at most one GOP +before the cut, not the whole file before it. The re-encode arguments are derived from the master's own codec, pixel format and colour metadata, because the concat demuxer joins streams without @@ -342,7 +346,8 @@ The pieces, usable on their own: | `snap_start_to_keyframe(start, end_limit, keyframes)` | Move a cut start forward onto a keyframe. | | `keyframe_at_or_before(t, keyframes)` | Where `-ss t -c copy` would actually land. | | `merge_ranges(ranges, max_gap_sec, is_excluded)` | Collapse overlapping, touching and sub-GOP-separated ranges. | -| `stream_copy_range` / `reencode_range` / `concat_demuxer` | The three ffmpeg invocations. | +| `stream_copy_range` / `reencode_range` / `concat_demuxer` | The three ffmpeg invocations. Each timeout is a floor, raised to fit the file's size or the range's length. | +| `io_timeout(floor_sec, nbytes)` | A timeout long enough to read `nbytes` at `MIN_IO_BYTES_PER_SEC` (20 MB/s). | | `default_reencode_args(master)` | Concat-compatible encoder arguments derived from the master. | | `probe_video_params(master)` | Codec, pixel format and colour metadata of the first video stream. | diff --git a/pyproject.toml b/pyproject.toml index f448e6c..dbb75c6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "keycut" -version = "0.1.1" +version = "0.1.2" description = "Lossless multi-range video extraction with ffmpeg stream copy, without the GOP-replay artefact." readme = "README.md" requires-python = ">=3.11" diff --git a/src/keycut/__init__.py b/src/keycut/__init__.py index 7210e95..fd93d1e 100644 --- a/src/keycut/__init__.py +++ b/src/keycut/__init__.py @@ -23,6 +23,7 @@ from .attribution import assign_clips_by_source_window from .extract import ( DEFAULT_CRF, + REENCODE_SEC_PER_SEC, concat_demuxer, default_reencode_args, extract_and_concat_ranges, @@ -34,8 +35,10 @@ from .keyframes import ( DEFAULT_GOP_DURATION_SEC, FAILED_PROBE_RETRY_SEC, + MIN_IO_BYTES_PER_SEC, clear_keyframe_cache, estimate_gop_duration, + io_timeout, keyframe_at_or_before, probe_keyframes, snap_start_to_keyframe, @@ -56,12 +59,14 @@ segments_to_master_ranges, ) -__version__ = "0.1.0" +__version__ = "0.1.2" __all__ = [ "DEFAULT_CRF", "DEFAULT_GOP_DURATION_SEC", "FAILED_PROBE_RETRY_SEC", + "MIN_IO_BYTES_PER_SEC", + "REENCODE_SEC_PER_SEC", "MaskLike", "Range", "Segment", @@ -76,6 +81,7 @@ "estimate_gop_duration", "extract_and_concat_ranges", "extract_and_concat_segments", + "io_timeout", "keyframe_at_or_before", "merge_ranges", "probe_keyframes", diff --git a/src/keycut/extract.py b/src/keycut/extract.py index a7cdfc0..a14a1f2 100644 --- a/src/keycut/extract.py +++ b/src/keycut/extract.py @@ -11,6 +11,7 @@ from .keyframes import ( DEFAULT_GOP_DURATION_SEC, estimate_gop_duration, + io_timeout, probe_keyframes, snap_start_to_keyframe, ) @@ -135,6 +136,12 @@ def default_reencode_args( return args +REENCODE_SEC_PER_SEC = 20.0 +"""Seconds of wall-clock allowed per second of footage re-encoded by +:func:`reencode_range` — 1.5 fps at 30 fps, slower than x265 ``-preset slow`` +manages on 4K with a modern CPU.""" + + def stream_copy_range( master_path: Path | str, start_sec: float, @@ -156,6 +163,9 @@ def stream_copy_range( Pass a keyframe-aligned start (see :func:`keycut.snap_start_to_keyframe`) or the output will contain footage from before the cut. + + ``timeout`` is a floor, raised to the time to read the whole master (see + :func:`keycut.io_timeout`), since no range can be larger than that. """ output_path = Path(output_path) output_path.parent.mkdir(parents=True, exist_ok=True) @@ -172,7 +182,7 @@ def stream_copy_range( "copy", str(output_path), ] - return _run_ffmpeg(cmd, "stream copy", timeout) + return _run_ffmpeg(cmd, "stream copy", io_timeout(timeout, _size(master_path))) def reencode_range( @@ -187,11 +197,20 @@ def reencode_range( ) -> bool: """Decode and re-encode ``[start_sec, end_sec)`` with a frame-accurate seek. - ``-ss``/``-to`` go *after* ``-i``, making them output options: ffmpeg - decodes from the beginning of the file and discards frames until the exact - requested timestamp. That costs a full decode of everything before the cut, - and it re-encodes what it keeps — but it lands on the requested frame - rather than the previous keyframe. + ``-ss`` goes *before* ``-i``: the demuxer seeks to the keyframe at or before + ``start_sec``, and because this transcodes, ffmpeg's ``-accurate_seek`` (on + by default) decodes and drops the frames between that keyframe and the + exact start. The output lands on the requested frame, like an output-side + seek, but decodes at most one GOP before the cut instead of everything + from the start of the file. Timestamps restart at zero after an input + seek, so the length is given with ``-t``. + + Stream-copied audio is not trimmed by the input seek: its packets from the + keyframe on survive at negative timestamps behind an edit list, which the + concat demuxer ignores. An output-side ``-ss 0`` drops them. + + ``timeout`` is a floor, raised to :data:`REENCODE_SEC_PER_SEC` seconds per + second of footage: a long 4K range through a CPU encoder runs for hours. This is the escape hatch for the two cases a stream copy cannot serve: keyframe positions are unknown, or the snap-back region contains footage @@ -199,19 +218,24 @@ def reencode_range( """ output_path = Path(output_path) output_path.parent.mkdir(parents=True, exist_ok=True) + duration = float(end_sec) - float(start_sec) cmd = [ ffmpeg_bin, "-y", + "-ss", + f"{float(start_sec)}", "-i", str(master_path), "-ss", - f"{float(start_sec)}", - "-to", - f"{float(end_sec)}", + "0", + "-t", + f"{duration}", *list(encode_args), str(output_path), ] - return _run_ffmpeg(cmd, "re-encode", timeout) + return _run_ffmpeg( + cmd, "re-encode", max(float(timeout), duration * REENCODE_SEC_PER_SEC) + ) def concat_demuxer( @@ -227,6 +251,10 @@ def concat_demuxer( Writes the ``file ''`` manifest to ``list_file`` first, single-quote- escaped, and passes ``-safe 0`` so absolute paths are accepted. + + ``timeout`` is a floor, raised to the time to move the pieces' bytes (see + :func:`keycut.io_timeout`) — twice over with ``faststart``, which rewrites + the whole output a second time to move the moov atom. """ list_file = Path(list_file) output_path = Path(output_path) @@ -242,7 +270,15 @@ def concat_demuxer( if faststart: cmd += ["-movflags", "+faststart"] cmd.append(str(output_path)) - return _run_ffmpeg(cmd, "concat", timeout) + nbytes = sum(_size(p) for p in parts) * (2 if faststart else 1) + return _run_ffmpeg(cmd, "concat", io_timeout(timeout, nbytes)) + + +def _size(path: Path | str) -> int: + try: + return Path(path).stat().st_size + except OSError: + return 0 def _run_ffmpeg(cmd: list[str], what: str, timeout: float) -> bool: diff --git a/src/keycut/keyframes.py b/src/keycut/keyframes.py index 17fbae4..4efdb57 100644 --- a/src/keycut/keyframes.py +++ b/src/keycut/keyframes.py @@ -36,6 +36,24 @@ most once per window rather than on every call. """ +MIN_IO_BYTES_PER_SEC = 20_000_000 +"""Slowest read/write rate, in bytes per second, a healthy ffmpeg or ffprobe +run is assumed to sustain. + +Every subprocess timeout here is a hang guard, so it must outlast honest work. +Reading a 118 GB day master over a network or 9p mount (WSL's ``/mnt/c``) +runs at roughly 50 MB/s; a flat 600 s or 1800 s window kills that read +part-way through. :func:`io_timeout` sizes each window to the bytes involved +at this floor. +""" + + +def io_timeout(floor_sec: float, nbytes: int) -> float: + """``floor_sec``, or the time to move ``nbytes`` at + :data:`MIN_IO_BYTES_PER_SEC`, whichever is longer.""" + return max(float(floor_sec), nbytes / MIN_IO_BYTES_PER_SEC) + + # 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. # Each value is ``(keyframes, retry_at)``. A success has ``retry_at=None`` and @@ -70,9 +88,12 @@ 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); a - failed probe is cached only for :data:`FAILED_PROBE_RETRY_SEC` seconds, so - a transient failure recovers on a later call. + of footage on a local disk; ffprobe still reads every byte, so on a slow + mount it takes as long as reading the file. ``timeout`` is a floor: the + window grows with the file's size (see :func:`io_timeout`). 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. @@ -108,7 +129,9 @@ def probe_keyframes( keyframes: list[float] | None = None try: - result = subprocess.run(cmd, capture_output=True, timeout=timeout) + result = subprocess.run( + cmd, capture_output=True, timeout=io_timeout(timeout, st.st_size) + ) if result.returncode == 0: parsed: list[float] = [] for line in (result.stdout or b"").decode("utf-8", errors="replace").splitlines(): diff --git a/tests/conftest.py b/tests/conftest.py index 57ca3fa..3e2b806 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -56,11 +56,13 @@ def __init__(self, *, keyframe_stdout: bytes, stream_stdout: bytes, returncode: self.stream_stdout = stream_stdout self.returncode = returncode self.commands: list[list[str]] = [] + self.timeouts: list[float | None] = [] self.list_files: list[str] = [] def __call__(self, cmd, *args, **kwargs): cmd = list(cmd) self.commands.append(cmd) + self.timeouts.append(kwargs.get("timeout")) joined = " ".join(cmd) if cmd[0].endswith("ffprobe"): if "packet=" in joined: diff --git a/tests/test_extract.py b/tests/test_extract.py index ea8230a..6f33dac 100644 --- a/tests/test_extract.py +++ b/tests/test_extract.py @@ -16,6 +16,7 @@ ) from keycut import ( Segment, + concat_demuxer, default_reencode_args, extract_and_concat_ranges, extract_and_concat_segments, @@ -38,6 +39,16 @@ def _to(cmd): return float(cmd[cmd.index("-to") + 1]) +def _t(cmd): + return float(cmd[cmd.index("-t") + 1]) + + +def _sparse(path, size): + with path.open("wb") as fh: + fh.truncate(size) # no disk is used + return path + + class TestHappyPath: def test_one_extraction_per_range_then_a_concat(self, master, tmp_path): with fake_ffmpeg() as fake: @@ -169,8 +180,17 @@ def test_unknown_keyframes_force_an_accurate_seek_re_encode(self, master, tmp_pa assert result is not None cmd = fake.extract_commands[0] assert is_reencode(cmd) - assert not seek_is_input_side(cmd), "an accurate seek must be output-side" + assert seek_is_input_side(cmd), ( + "an output-side seek decodes everything before the cut; a re-encode " + "with an input-side seek is just as exact (ffmpeg's -accurate_seek)" + ) assert _ss(cmd) == 100.0, "the re-encode lands on the exact requested frame" + assert _t(cmd) == 60.0, "the input seek resets timestamps, so length is -t" + after_input = cmd[cmd.index("-i"):] + assert "-ss" in after_input and float(after_input[after_input.index("-ss") + 1]) == 0.0, ( + "stream-copied audio keeps the packets back to the keyframe at negative " + "timestamps; an output-side -ss 0 drops them" + ) def test_a_clean_cut_still_takes_the_lossless_path(self, master, tmp_path): with fake_ffmpeg() as fake: @@ -376,3 +396,62 @@ def test_the_default_stream_json_fixture_is_what_the_probe_reads(self, master): # Guards the fixture itself: if DEFAULT_STREAM_JSON drifts, every # assertion above silently starts testing something else. assert b"h264" in DEFAULT_STREAM_JSON + + +class TestLargeMasterTimeouts: + """A day master can be 100+ GB on a slow mount. Every ffmpeg step gets a + window sized to the work, so a healthy run is never killed as a hang.""" + + def test_a_stream_copy_window_scales_with_the_master(self, tmp_path): + master = _sparse(tmp_path / "day_master.mp4", 118_000_000_000) + with fake_ffmpeg() as fake: + extract_and_concat_ranges( + master, [(100.0, 160.0)], tmp_path / "out.mp4", tmp_path / "work", + keyframes=[100.0, 200.0], + ) + copy_idx = fake.commands.index(fake.extract_commands[0]) + assert fake.timeouts[copy_idx] >= 118_000_000_000 / 20_000_000 + + def test_a_small_stream_copy_keeps_the_1800s_floor(self, master, tmp_path): + with fake_ffmpeg() as fake: + extract_and_concat_ranges( + master, [(100.0, 160.0)], tmp_path / "out.mp4", tmp_path / "work", + keyframes=[100.0, 200.0], + ) + copy_idx = fake.commands.index(fake.extract_commands[0]) + assert fake.timeouts[copy_idx] == 1800.0 + + def test_a_re_encode_window_scales_with_the_range_length(self, master, tmp_path): + """A 1147 s range of 4K re-encoded with x265 on a CPU runs for hours, + far past the flat 3600 s.""" + with fake_ffmpeg() as fake: + extract_and_concat_ranges( + master, [(3000.0, 4147.0)], tmp_path / "out.mp4", tmp_path / "work", + keyframes=[], reencode_args=["-c:v", "libx265"], + ) + idx = fake.commands.index(fake.extract_commands[0]) + assert fake.timeouts[idx] >= 1147.0 * 20 + + def test_a_short_re_encode_keeps_the_3600s_floor(self, master, tmp_path): + with fake_ffmpeg() as fake: + extract_and_concat_ranges( + master, [(100.0, 102.0)], tmp_path / "out.mp4", tmp_path / "work", + keyframes=[], reencode_args=["-c:v", "libx265"], + ) + idx = fake.commands.index(fake.extract_commands[0]) + assert fake.timeouts[idx] == 3600.0 + + def test_a_concat_window_scales_with_the_pieces(self, tmp_path): + parts = [ + _sparse(tmp_path / "a.mp4", 30_000_000_000), + _sparse(tmp_path / "b.mp4", 21_000_000_000), + ] + with fake_ffmpeg() as fake: + concat_demuxer(parts, tmp_path / "out.mp4", tmp_path / "list.txt", + faststart=False) + concat_demuxer(parts, tmp_path / "out2.mp4", tmp_path / "list2.txt") + plain, faststart = fake.timeouts + assert plain >= 51_000_000_000 / 20_000_000 + assert faststart >= 2 * 51_000_000_000 / 20_000_000, ( + "+faststart rewrites the whole output a second time" + ) diff --git a/tests/test_integration_ffmpeg.py b/tests/test_integration_ffmpeg.py index ca45123..6d94555 100644 --- a/tests/test_integration_ffmpeg.py +++ b/tests/test_integration_ffmpeg.py @@ -266,10 +266,10 @@ def test_an_accurate_seek_re_encode_delivers_the_exact_span(self, source, tmp_pa """``keyframes=[]`` declares them unusable, so every range re-encodes. This is the one path that returns exactly what was asked for — 60 - frames, no more and no fewer — and it pins the semantics keycut relies - on: with ``-ss``/``-to`` placed after ``-i``, both are still read in the - input's timeline, so ``-to 12.5`` means 12.5 in the source and not 12.5 - seconds after the seek. + frames, no more and no fewer. The audio is stream-copied, and after an + input seek it would start at the keyframe (10.0), half a second early, + hidden behind an edit list the concat demuxer ignores. The output-side + ``-ss 0`` drops it, so the file is 2.0 s long in both streams. """ out = extract_and_concat_ranges( source, [(10.5, 12.5)], tmp_path / "out.mp4", tmp_path / "work", @@ -277,6 +277,9 @@ def test_an_accurate_seek_re_encode_delivers_the_exact_span(self, source, tmp_pa ) assert out is not None assert len(video_packets(out)) == int(2.0 * FIXTURE_FPS) + assert probe_duration(out) == pytest.approx(2.0, abs=0.1), ( + "audio from before the cut leaked into the re-encoded range" + ) assert decodes_cleanly(out) diff --git a/tests/test_keyframes.py b/tests/test_keyframes.py index d3581a3..98846f5 100644 --- a/tests/test_keyframes.py +++ b/tests/test_keyframes.py @@ -104,6 +104,31 @@ def _fake_run(cmd, *a, **kw): assert len(calls) == 1, "a second probe of the same file must hit the cache" assert calls[0][0] == "ffprobe" + def test_the_probe_window_scales_with_the_file_size(self, tmp_path): + """ffprobe reads every packet, so the probe takes as long as reading + the whole file. A 110 GB day master on a slow mount needed far more + than the flat 600 s and was killed, which sent every cut down the + re-encode path.""" + big = tmp_path / "day_master.mp4" + with big.open("wb") as fh: + fh.truncate(118_000_000_000) # sparse: no disk is used + small = tmp_path / "short.mp4" + small.write_bytes(b"not really a video") + timeouts = [] + + def _fake_run(cmd, *a, **kw): + timeouts.append(kw.get("timeout")) + return MagicMock(returncode=0, stdout=b"0.000000,K__\n", stderr=b"") + + with patch("keycut.keyframes.subprocess.run", side_effect=_fake_run): + probe_keyframes(small) + probe_keyframes(big) + + assert timeouts[0] == 600.0, "a small file keeps the 600 s floor" + assert timeouts[1] >= 118_000_000_000 / 20_000_000, ( + "a 118 GB file gets at least 20 MB/s worth of time" + ) + def test_missing_file_returns_none(self, tmp_path): assert probe_keyframes(tmp_path / "nope.mp4") is None