Conversation
Two EDL knobs that the current renderer hardcodes: - `fade_in` / `fade_out` on a range override the 30ms default fades. A hard cut into music or a cold open often wants a longer ramp; 30ms stays the default so existing EDLs render identically. - `"subtitles": false` on a range drops that cut's captions from master.srt while still advancing the output offset, so later cuts keep their timing. Useful for b-roll or a title card that should not carry burned-in text. Tests cover both, including the offset accounting for a skipped cut. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
4 issues 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:273">
P2: When a cut sets `fade_in` to `0` to disable the opening fade, this still emits `afade(...:d=0.000)`. FFmpeg interprets zero as the default 44,100-sample duration, so the cut unexpectedly fades in for roughly a second; omit each `afade` filter when its duration is zero.</violation>
<violation number="2" location="helpers/render.py:359">
P2: When an EDL supplies a negative, NaN, or infinite fade duration, this code passes it into the FFmpeg `afade` filter and the render fails. Validate both fade values as finite and non-negative before invoking `extract_segment`.</violation>
<violation number="3" location="helpers/render.py:441">
P2: When an EDL uses an existing `subtitles` SRT without `--build-subtitles`, this branch never runs. `main()` burns the unchanged file, so cues from `subtitles: false` cuts remain; apply the opt-out to the selected SRT or reject this mode unless the SRT is rebuilt.</violation>
</file>
<file name="tests/test_render_cut_options.py">
<violation number="1" location="tests/test_render_cut_options.py:36">
P3: No test covers a cut that declares only one of `fade_in`/`fade_out`. `extract_all_segments` reads each key independently (helpers/render.py:359-360), so e.g. a cut with only `fade_out` must still get the 0.03 fade-in default; a regression in that asymmetric path (both keys read together, or a wrong default) would pass all four current tests. Add a one-range case declaring a single fade to pin the independent defaults.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| fade_in = float(r.get("fade_in", 0.03)) | ||
| fade_out_val = float(r.get("fade_out", 0.03)) |
There was a problem hiding this comment.
P2: When an EDL supplies a negative, NaN, or infinite fade duration, this code passes it into the FFmpeg afade filter and the render fails. Validate both fade values as finite and non-negative before invoking extract_segment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/render.py, line 359:
<comment>When an EDL supplies a negative, NaN, or infinite fade duration, this code passes it into the FFmpeg `afade` filter and the render fails. Validate both fade values as finite and non-negative before invoking `extract_segment`.</comment>
<file context>
@@ -355,11 +356,13 @@ def extract_all_segments(
else:
seg_filter = resolved
+ fade_in = float(r.get("fade_in", 0.03))
+ fade_out_val = float(r.get("fade_out", 0.03))
note = r.get("beat") or r.get("note") or ""
</file context>
| fade_in = float(r.get("fade_in", 0.03)) | |
| fade_out_val = float(r.get("fade_out", 0.03)) | |
| fade_in = float(r.get("fade_in", 0.03)) | |
| fade_out_val = float(r.get("fade_out", 0.03)) | |
| if not all(0 <= value < float("inf") for value in (fade_in, fade_out_val)): | |
| raise ValueError("fade_in and fade_out must be finite and non-negative") |
| seg_end = float(r["end"]) | ||
| seg_duration = seg_end - seg_start | ||
|
|
||
| if r.get("subtitles") is False: |
There was a problem hiding this comment.
P2: When an EDL uses an existing subtitles SRT without --build-subtitles, this branch never runs. main() burns the unchanged file, so cues from subtitles: false cuts remain; apply the opt-out to the selected SRT or reject this mode unless the SRT is rebuilt.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/render.py, line 441:
<comment>When an EDL uses an existing `subtitles` SRT without `--build-subtitles`, this branch never runs. `main()` burns the unchanged file, so cues from `subtitles: false` cuts remain; apply the opt-out to the selected SRT or reject this mode unless the SRT is rebuilt.</comment>
<file context>
@@ -435,6 +438,10 @@ def build_master_srt(edl: dict, edit_dir: Path, out_path: Path) -> None:
seg_end = float(r["end"])
seg_duration = seg_end - seg_start
+ if r.get("subtitles") is False:
+ seg_offset += seg_duration
+ continue
</file context>
| fade_out_start = max(0.0, duration - 0.03) | ||
| af = f"afade=t=in:st=0:d=0.03,afade=t=out:st={fade_out_start:.3f}:d=0.03" | ||
| fade_out_start = max(0.0, duration - fade_out) | ||
| af = f"afade=t=in:st=0:d={fade_in:.3f},afade=t=out:st={fade_out_start:.3f}:d={fade_out:.3f}" |
There was a problem hiding this comment.
P2: When a cut sets fade_in to 0 to disable the opening fade, this still emits afade(...:d=0.000). FFmpeg interprets zero as the default 44,100-sample duration, so the cut unexpectedly fades in for roughly a second; omit each afade filter when its duration is zero.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/render.py, line 273:
<comment>When a cut sets `fade_in` to `0` to disable the opening fade, this still emits `afade(...:d=0.000)`. FFmpeg interprets zero as the default 44,100-sample duration, so the cut unexpectedly fades in for roughly a second; omit each `afade` filter when its duration is zero.</comment>
<file context>
@@ -267,9 +269,8 @@ def extract_segment(
- fade_out_start = max(0.0, duration - 0.03)
- af = f"afade=t=in:st=0:d=0.03,afade=t=out:st={fade_out_start:.3f}:d=0.03"
+ fade_out_start = max(0.0, duration - fade_out)
+ af = f"afade=t=in:st=0:d={fade_in:.3f},afade=t=out:st={fade_out_start:.3f}:d={fade_out:.3f}"
if draft:
</file context>
| af = f"afade=t=in:st=0:d={fade_in:.3f},afade=t=out:st={fade_out_start:.3f}:d={fade_out:.3f}" | |
| af_parts: list[str] = [] | |
| if fade_in > 0: | |
| af_parts.append(f"afade=t=in:st=0:d={fade_in:.3f}") | |
| if fade_out > 0: | |
| af_parts.append(f"afade=t=out:st={fade_out_start:.3f}:d={fade_out:.3f}") | |
| af = ",".join(af_parts) or "anull" |
| self.assertEqual(calls[0].kwargs["fade_in"], 0.03) | ||
| self.assertEqual(calls[0].kwargs["fade_out"], 0.03) | ||
|
|
||
| def test_per_cut_fades_apply_only_to_the_cut_that_declares_them(self): |
There was a problem hiding this comment.
P3: No test covers a cut that declares only one of fade_in/fade_out. extract_all_segments reads each key independently (helpers/render.py:359-360), so e.g. a cut with only fade_out must still get the 0.03 fade-in default; a regression in that asymmetric path (both keys read together, or a wrong default) would pass all four current tests. Add a one-range case declaring a single fade to pin the independent defaults.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_render_cut_options.py, line 36:
<comment>No test covers a cut that declares only one of `fade_in`/`fade_out`. `extract_all_segments` reads each key independently (helpers/render.py:359-360), so e.g. a cut with only `fade_out` must still get the 0.03 fade-in default; a regression in that asymmetric path (both keys read together, or a wrong default) would pass all four current tests. Add a one-range case declaring a single fade to pin the independent defaults.</comment>
<file context>
@@ -0,0 +1,89 @@
+ self.assertEqual(calls[0].kwargs["fade_in"], 0.03)
+ self.assertEqual(calls[0].kwargs["fade_out"], 0.03)
+
+ def test_per_cut_fades_apply_only_to_the_cut_that_declares_them(self):
+ calls = self._extract([
+ {"source": "a", "start": 0, "end": 1, "fade_in": 0.5, "fade_out": 0.25},
</file context>
Two knobs for things the renderer currently hardcodes. Both default to today's behavior, so existing EDLs render byte-for-byte the same.
fade_in/fade_outper rangeextract_segmentbakes 30ms audio fades at both edges. That is the right default for pop prevention, but a cold open or a hard cut into music often wants a longer ramp — and there was no way to ask for one short of editing the source.{ "source": "a", "start": 0, "end": 3, "fade_in": 0.5, "fade_out": 0.25 }Omit them and you get 0.03 as before.
"subtitles": falseper rangebuild_master_srtcaptions every range that has a transcript. B-roll and title cards usually should not carry burned-in text, but suppressing them meant hand-editingmaster.srtafter every render.{ "source": "broll", "start": 5, "end": 7, "subtitles": false }The skipped cut still advances
seg_offset, so every later cut keeps its timing. That accounting is the part worth a test, and it has one.Tests
tests/test_render_cut_options.py, following the shape oftests/test_render_fps.py—extract_segmentmocked for the fade kwargs, a real SRT built from a temp transcript for the opt-out. Full suite: 20 passed, 15 subtests.Also verified end to end against a synthetic 1080x1920 / 29.97fps source: fps and orientation preserved alongside #55 and #137, the
subtitles: falsecut emits no cues, and a 0.5s fade measures 11dB below a 0.03s one over the first 200ms.🤖 Generated with Claude Code
Summary by cubic
Adds per-cut audio fades and a per-cut subtitle opt-out to the renderer. Both default to today's behavior, so existing EDLs render byte-for-byte the same.
New Features
fade_in/fade_outon a range override the default 30ms audio fades."subtitles": falseon a range skips that cut's captions inmaster.srtwhile still advancing the output offset for later cuts.Written for commit 0d3dd25. Summary will update on new commits.