render: probe each source once, not once per EDL range

[ravsau]

The problem

is_portrait_source() and is_hdr_source() read stream-level facts (width/height/rotation side data, and color_transfer) that belong to the source file, not to the cut range. extract_segment() called both on every range.

A 40-range EDL over 2 sources spawns 80 ffprobe processes to answer 4 questions. The cost is per range, so it grows with cut count. Fast-cut social edits pay the most.

The change

extract_all_segments() probes each distinct source once and passes the answers down. The new portrait/hdr arguments default to None, which probes exactly as before, so extract_segment() still works when called on its own.

26 insertions, 3 deletions in helpers/render.py.

Measured

40-range EDL over 2 sources (4K landscape + 1080x1920 portrait), grade: neutral_punch, loudnorm on. Median of 3 runs each, same machine, ffmpeg 9.0.1:

	before	after
ffprobe processes	81	5
wall clock	15.27 s	13.17 s (-14%)
output		byte-identical

Byte equality checked with cmp and framemd5 on the final mp4, on this EDL and on a separate 6-range EDL.

Correctness

The probes are pure functions of the file and both already swallow their own errors, so a cached answer is what the per-range call would have returned.

Portrait, HDR and rotation-metadata sources are all covered. My local ffmpeg has no zscale, so I could not execute the tonemap path end to end. Instead I captured every ffmpeg command both versions build for an EDL mixing an HLG source, a portrait source, a rotated-landscape source and grade: "auto". The commands are character-for-character identical, with the tonemap branch taken on 3 of 5 ranges.

Tests

New tests/test_render_source_probes.py, 8 cases. The probe mocks use a per-source side_effect, so a fact leaking from one source to another fails the suite rather than passing it.

I mutated the code to confirm the tests catch what they claim:

mutation	result
cache returns the first source's facts for every source	2 tests fail
portrait/hdr swapped when unpacked	1 test fails

Full suite: 23 passing.

Relation to #63

#63 got to this idea first. It lists "one cached ffprobe per source instead of two per segment" among many other changes, at +1737/-42 across 12 files. It has been conflicting since June.

This PR is only that one change, with a measurement and tests. If you would rather land #63, close this one and lose nothing.

https://claude.ai/code/session_01DXA5U6zRSz9pKbCXXGEymU

---

Summary by cubic

Render extraction now probes each distinct source once instead of once per EDL range, reducing setup cost without changing output. Standalone extract_segment() calls still probe automatically when source facts are not provided.

• A 40-range EDL over two sources dropped ffprobe processes from 81 to 5 and wall time by 14%; output remained byte-identical.
• Added tests for per-source probe isolation, fact forwarding, HDR and portrait handling, and standalone fallback behavior.
• Stubbed the source probes in the fps tests so the suite no longer shells out and runs without ffprobe on PATH.

^{Written for commit 2fbbe3c. Summary will update on new commits.}

[cubic-dev-ai]

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="helpers/render.py">

<violation number="1" location="helpers/render.py:355">
P2: When `extract_segment` is stubbed, `extract_all_segments` now still invokes real `ffprobe` through `facts_for`, breaking the existing FPS tests' isolation and causing three test errors without the media tool. Update those tests to stub the source probes, or make source-fact probing injectable at this boundary.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: When extract_segment is stubbed, extract_all_segments now still invokes real ffprobe through facts_for, breaking the existing FPS tests' isolation and causing three test errors without the media tool. Update those tests to stub the source probes, or make source-fact probing injectable at this boundary.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At helpers/render.py, line 355:

<comment>When `extract_segment` is stubbed, `extract_all_segments` now still invokes real `ffprobe` through `facts_for`, breaking the existing FPS tests' isolation and causing three test errors without the media tool. Update those tests to stub the source probes, or make source-fact probing injectable at this boundary.</comment>

<file context>
@@ -337,6 +346,15 @@ def extract_all_segments(
+
+    def facts_for(path: Path) -> tuple[bool, bool]:
+        if path not in source_facts:
+            source_facts[path] = (is_portrait_source(path), is_hdr_source(path))
+        return source_facts[path]
+
</file context>

[ravsau]

Valid, and reproduced. Fixed in 2fbbe3c.

extract_all_segments now probes source facts itself, so stubbing extract_segment no longer keeps those probes out of the test. Simulating a machine with no ffprobe on PATH gave exactly the three failures you named:

FAILED test_render_fps.py::RenderRateTests::test_explicit_rate_skips_probe_and_applies_to_every_segment
FAILED test_render_fps.py::RenderRateTests::test_failed_probe_falls_back_to_24_for_every_segment
FAILED test_render_fps.py::RenderRateTests::test_multi_source_render_resolves_one_rate_from_first_source

The uncaught exception is FileNotFoundError from is_hdr_source, which catches only CalledProcessError, while is_portrait_source catches OSError too.

I took the test-only fix and stubbed both probes in those three tests. I deliberately did not widen is_hdr_source's exception handling to match is_portrait_source. That asymmetry looks like a latent bug, but changing it would swap a crash for a silently skipped tonemap, which is a runtime semantic this PR does not name. Worth its own PR.

The suite now passes with no ffprobe on PATH, and no longer shells out at all (0.02s, down from 0.3s).