Skip to content

feat(render): compose independent picture audio and caption timelines - #170

Open
DonIsmaelito wants to merge 35 commits into
browser-use:mainfrom
DonIsmaelito:submit/rendering
Open

DonIsmaelito wants to merge 35 commits into
browser-use:mainfrom
DonIsmaelito:submit/rendering

Conversation

@DonIsmaelito

@DonIsmaelito DonIsmaelito commented Sep 17, 2026

Copy link
Copy Markdown

Why

Picture cuts should not force music, dialogue and captions to restart together. This connects the editing helpers so one declared timeline can render and check the finished video.

Builds on #147, #167 and #169.

Changes

  • Add an explicit composition timeline with independently placed picture, audio, captions and moving layers.
  • Connect it to the render command while retaining the existing EDL v1/v2 path and ASS caption interface.
  • Check encoded frames, timing, audio alignment and loudness, and generate review sheets. Protect existing outputs and reports.
  • Three focused commits cover validation, rendering and documentation. All 211 branch tests pass; one RAQM-dependent test is skipped. Encoded review frames were inspected.

Limits

The new composition path requires 30 fps and declared non-silent audio, delivered at 48 kHz stereo. It needs FFmpeg and the existing editing dependencies. Other frame rates and silent-only composition exports are not supported yet.

Technical checks do not replace visual and listening review or verify transcription and asset ownership. Caption timing checks use the manifest rather than OCR. Listening review remains pending.

Existing renderer proposals overlap this integration area. This does not resolve legacy caption drift in #161, AAC joins in #163 or probe caching in #150. The separate montage command and specialized workflow skills remain outside this PR. No new assets or fonts are bundled.

@DonIsmaelito
DonIsmaelito marked this pull request as ready for review September 17, 2026 22:20

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

40 issues found across 58 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/edl.py">

<violation number="1" location="helpers/edl.py:181">
P2: When a deliverable width or height is fractional, `int()` truncates it and silently changes the requested output dimensions. Reject non-integral dimensions before converting them to integers.</violation>

<violation number="2" location="helpers/edl.py:359">
P1: When a declared deliverable output already exists, `normalize_deliverables` accepts it and legacy rendering overwrites it with ffmpeg `-y`. Reject existing or symlink outputs, including paths aliasing inputs, before rendering.</violation>

<violation number="3" location="helpers/edl.py:404">
P2: When a tracked JSON file contains invalid UTF-8, `validate_edl` raises `UnicodeDecodeError` and the CLI prints a traceback instead of an actionable validation error. Catch `UnicodeDecodeError` and include it in the track-data validation problem.</violation>

<violation number="4" location="helpers/edl.py:426">
P2: When a grade has leading or trailing whitespace, validation accepts it but rendering receives the original value and fails as an invalid filter. Reject surrounding whitespace or normalize the grade before rendering.</violation>

<violation number="5" location="helpers/edl.py:453">
P2: When an integration imports `helpers.edl` from the project root, version-3 validation raises `ModuleNotFoundError` before checking the manifest. Make the helper imports package-safe, or consistently establish the helpers module path for library callers.</violation>
</file>

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

<violation number="1" location="helpers/render.py:346">
P1: When a segment declares a valid reframe with `zoom` 1 (or only default focus fields), `build_reframe_filter` returns an empty string and this appends a trailing comma to the FFmpeg filter chain. Skip empty reframe filters before appending them.</violation>

<violation number="2" location="helpers/render.py:915">
P2: When an overlay contains `NaN` or infinity timing, the current bounds check accepts it and emits non-finite FFmpeg timestamps. Reject non-finite `start_in_output` and `duration` before comparing their bounds.</violation>

<violation number="3" location="helpers/render.py:1421">
P1: When a declared legacy deliverable already exists, `render_one_output` allows FFmpeg's `-y` commands to overwrite it. Reject existing files and symlinks before starting the render.</violation>
</file>

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

<violation number="1" location="helpers/verify_edit.py:44">
P1: When a caption card contains multiple linked words, this compares the card’s single entry frame with every word’s start. The later word is normally more than one frame after the card entry, so `caption_schedule` becomes false and a valid composition render fails; compare the entry against the first linked word or audit the card interval against the linked word range.</violation>

<violation number="2" location="helpers/verify_edit.py:114">
P2: On supported installations using an older Pillow release, `ImageDraw.text` rejects `font_size`, so `--sheets` fails for every review request. Use a loaded `ImageFont` through the supported `font` parameter or raise the project’s minimum Pillow version.</violation>
</file>

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

<violation number="1" location="helpers/captions.py:90">
P2: Malformed character timestamps can silently move a cue to time zero or crash ASS generation because this path lacks the finite, nonnegative, and ordered-time checks used for word entries. Validate each character start/end before buffering it and reject invalid alignment data with a `ValueError`.</violation>

<violation number="2" location="helpers/captions.py:125">
P2: When callers pass `break_on_punctuation`, `max_characters` is ignored because this delegation forwards only `max_words` and punctuation. Enforce the character limit in the compatibility path or reject the incompatible option before returning overlong cues.</violation>

<violation number="3" location="helpers/captions.py:219">
P1: When `master.ass` already exists, `write_substation` overwrites it unconditionally. Reject existing files and symlinks before writing, matching the other caption writers’ output-protection behavior.</violation>
</file>

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

<violation number="1" location="helpers/track_mask.py:103">
P2: When callers pass a NumPy polygon, `track` leaves an ndarray in the seed row and `main` fails in `save_json` after tracking. Store the normalized polygon with `.tolist()` like the propagated rows.</violation>

<violation number="2" location="helpers/track_mask.py:181">
P1: When `prepared_picture` is not 30 fps, `main` indexes masks by native frames while composition consumes them on its 30-fps global clock, causing matte drift or late validation failures. Reject non-30-fps inputs here or emit a timestamp-remapped track.</violation>
</file>

<file name="tests/test_track_mask.py">

<violation number="1" location="tests/test_track_mask.py:8">
P2: When the optional editing dependency is not installed, this import fails during test collection and blocks the entire suite. Import `pytest` first and guard OpenCV with `pytest.importorskip("cv2", reason="install the editing extra")`.</violation>
</file>

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

<violation number="1" location="helpers/prepare_source.py:13">
P2: When `<out>.log` or `<out>.json` already exists, this guard still allows preparation to overwrite that diagnostic or provenance artifact. Reject both sidecar paths, including symlinks, before creating the derivative.</violation>
</file>

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

<violation number="1" location="helpers/source_scan.py:80">
P1: When a source contains multiple video streams, `selected_frames()` can decode a different stream than the one cataloged. Map `0:v:0` explicitly so frame selection and stream metadata remain aligned.</violation>

<violation number="2" location="helpers/source_scan.py:142">
P2: When `--out` is a hard link to the source, this pathname check passes and `save_json()` truncates the source media. Compare existing output and source with `Path.samefile()` as well, in both source-writing CLIs.</violation>

<violation number="3" location="helpers/source_scan.py:147">
P2: When `--out` points to an existing report, this command overwrites the prior source evidence. Reject existing output paths before catalog generation instead of unconditionally calling `save_json()`.</violation>
</file>

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

<violation number="1" location="helpers/map_transcript.py:131">
P2: When `--out` points at an existing review artifact, this command overwrites it. Reject existing output paths, including symlinks, before calling `save_json`.</violation>
</file>

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

<violation number="1" location="helpers/cut_list.py:164">
P2: When an audio clip declares invalid `gain_points`, `validate()` skips them and rendering starts expensive staging before `mix_audio` rejects the clip. Validate the declared envelope against `clip["sample_count"]` here.</violation>

<violation number="2" location="helpers/cut_list.py:165">
P2: Word records bypass `known()`, so unsupported word treatments are accepted and ignored despite the validator's no-silent-discard contract. Reject extra word fields before reading the timestamps.</violation>
</file>

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

<violation number="1" location="helpers/edit_io.py:85">
P2: When a manifest encodes `study_media` as a string, this loop iterates its characters and lets reference-only sources enter the render graph. Validate that `study_media` is a list before iterating it.</violation>
</file>

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

<violation number="1" location="helpers/effects.py:106">
P2: A one-frame shot cannot use `time_map`: `validate_time_map` requires two points even though one output frame needs only one mapping point. Permit one point when `count == 1`.</violation>

<violation number="2" location="helpers/effects.py:115">
P2: Malformed `time_map` output frames can raise `TypeError` instead of a validation error when a middle frame is non-numeric. Check output-frame types before sorting them.</violation>

<violation number="3" location="helpers/effects.py:340">
P2: A layer with `fill` but no `fill_opacity` passes validation and renders fully opaque: `validate_layers` checks the fallback 0 while `LayerCompositor.frame` blends with fallback 1.0 (`Image.blend(im, filled, ...)`), so the fill completely covers the layer image. Make the defaults agree (validation should also default to 1, or the blend should default to 0) and add a test for fill without fill_opacity.</violation>
</file>

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

<violation number="1" location="helpers/caption_raster.py:73">
P2: When an SRT contains one malformed timestamp block, `parse_srt` raises instead of skipping that block, so a single bad cue aborts caption-track generation. Catch `ValueError` while parsing each cue and continue to the next block.</violation>
</file>

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

<violation number="1" location="helpers/cards.py:414">
P2: When FFmpeg or frame rendering fails, `write_movie` leaves a partial movie/log; its existence checks then reject retries. Render to temporary paths and remove them on failure, renaming only after success.</violation>
</file>

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

<violation number="1" location="helpers/mix_audio.py:95">
P2: When a CLI manifest contains a malformed `gain_points` entry, this code indexes `p[1]` before checking its shape. `build()` then exits with `IndexError`/`TypeError` instead of a clear validation error; validate each point as a two-item numeric pair first.</violation>

<violation number="2" location="helpers/mix_audio.py:251">
P2: When `--out-dir` is an existing directory symlink without these artifact names, the mixer follows it and writes the new mix into its target. Reject symlinked output directories before creating artifacts.</violation>
</file>

<file name="references/overlays.md">

<violation number="1" location="references/overlays.md:35">
P2: The composition rule contradicts the documented `picture_in_picture` exception. Scope this restriction to fixed-layout compositions or explicitly exempt `picture_in_picture`, otherwise authors may reject a supported custom-rect manifest.</violation>

<violation number="2" location="references/overlays.md:57">
P2: For a custom `captions.safe_region` that is not full-width, `render.py` rejects a `full` layout rather than shortening it. Qualify this sentence to describe the full-width-only behavior, or change the renderer if all safe regions should be shortened.</violation>
</file>

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

<violation number="1" location="helpers/_composition.py:354">
P2: The composition render never generates review sheets: `build()` only calls `verify()`, while `review_sheets()` runs only through the separate verifier CLI. Call `review_sheets(m, out, work / "review")` and record its paths before returning.</violation>
</file>

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

<violation number="1" location="helpers/find_shot.py:36">
P2: When RANSAC returns a matrix with no inliers, `.mean()` produces NaN and `save_json` aborts instead of writing the candidate report. Return the zero-inlier/`None` result before calculating reprojection error.</violation>
</file>

<file name="tests/test_render_treatment.py">

<violation number="1" location="tests/test_render_treatment.py:155">
P2: The integration test does not prove that treatment graphics or captions reach the encoded video. Assert representative output pixels or inspect a decoded frame/filter invocation so generated-but-unapplied overlays cannot pass.</violation>
</file>

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

<violation number="1" location="helpers/visuals.py:403">
P2: When `initial` and `minimum` have opposite parity, `_fit_font` skips the declared minimum size and can reject valid text. Iterate down by one or explicitly include `minimum`.</violation>
</file>

<file name="SKILL.md">

<violation number="1" location="SKILL.md:200">
P2: When an EDL customizes `captions.safe_region`, the documented `captions.py` command ignores it and emits captions in the default bottom 16% rail. Match the CLI's bottom-rail setting to the EDL or document that only the default bottom rail is supported.</violation>
</file>

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

<violation number="1" location="helpers/project_state.py:56">
P2: When an existing context omits `artifacts`, validation accepts it but both `record()` and `view()` later index the missing key, so an accepted empty context cannot be initialized or shown. Normalize loaded contexts with `setdefault("artifacts", {})` and use the same default in `view()`, or reject missing `artifacts` during validation.</violation>
</file>

<file name="tests/test_visuals.py">

<violation number="1" location="tests/test_visuals.py:254">
P2: This test invokes the ffmpeg binary unconditionally, so on a machine or CI runner without ffmpeg it fails the whole test file with FileNotFoundError instead of skipping the one ffmpeg-dependent case. Every other ffmpeg-dependent test in this suite (test_audio_tracks.py:15, test_composition.py:35, test_render_treatment.py:54-56, test_sources.py:29, test_mix_audio.py:19, test_cards.py:111) guards with `shutil.which(...)` and `pytest.skip`/`skipif`. Add the same guard here.</violation>
</file>

Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic

Comment thread helpers/edl.py
raise EDLValidationError(f"{label} currently supports only 48000 Hz audio")
declared_file = str(item.get("file") or f"deliverables/{deliverable_id}.mp4")
# an output directory override replaces the declared file name
output_path = (

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When a declared deliverable output already exists, normalize_deliverables accepts it and legacy rendering overwrites it with ffmpeg -y. Reject existing or symlink outputs, including paths aliasing inputs, before rendering.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/edl.py, line 359:

<comment>When a declared deliverable output already exists, `normalize_deliverables` accepts it and legacy rendering overwrites it with ffmpeg `-y`. Reject existing or symlink outputs, including paths aliasing inputs, before rendering.</comment>

<file context>
@@ -0,0 +1,567 @@
+            raise EDLValidationError(f"{label} currently supports only 48000 Hz audio")
+        declared_file = str(item.get("file") or f"deliverables/{deliverable_id}.mp4")
+        # an output directory override replaces the declared file name
+        output_path = (
+            (output_dir / f"{deliverable_id}.mp4").resolve()
+            if output_dir is not None
</file context>
Fix with cubic

Comment thread helpers/render.py
Comment on lines +346 to +347
if reframe:
vf_parts.append(build_reframe_filter(reframe))

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When a segment declares a valid reframe with zoom 1 (or only default focus fields), build_reframe_filter returns an empty string and this appends a trailing comma to the FFmpeg filter chain. Skip empty reframe filters before appending them.

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

<comment>When a segment declares a valid reframe with `zoom` 1 (or only default focus fields), `build_reframe_filter` returns an empty string and this appends a trailing comma to the FFmpeg filter chain. Skip empty reframe filters before appending them.</comment>

<file context>
@@ -253,16 +331,20 @@ def extract_segment(
     if is_hdr_source(source):
         vf_parts.append(TONEMAP_CHAIN)
     vf_parts.append(scale)
+    if reframe:
+        vf_parts.append(build_reframe_filter(reframe))
     if grade_filter:
</file context>
Suggested change
if reframe:
vf_parts.append(build_reframe_filter(reframe))
if reframe:
reframe_filter = build_reframe_filter(reframe)
if reframe_filter:
vf_parts.append(reframe_filter)
Fix with cubic

Comment thread helpers/verify_edit.py
if not cards:
continue
first = min(cards, key=lambda c: c["start_frame"])
delta = frame_to_sample(first["start_frame"]) - word["start_sample"]

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When a caption card contains multiple linked words, this compares the card’s single entry frame with every word’s start. The later word is normally more than one frame after the card entry, so caption_schedule becomes false and a valid composition render fails; compare the entry against the first linked word or audit the card interval against the linked word range.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/verify_edit.py, line 44:

<comment>When a caption card contains multiple linked words, this compares the card’s single entry frame with every word’s start. The later word is normally more than one frame after the card entry, so `caption_schedule` becomes false and a valid composition render fails; compare the entry against the first linked word or audit the card interval against the linked word range.</comment>

<file context>
@@ -0,0 +1,312 @@
+        if not cards:
+            continue
+        first = min(cards, key=lambda c: c["start_frame"])
+        delta = frame_to_sample(first["start_frame"]) - word["start_sample"]
+        rows.append(
+            {
</file context>
Fix with cubic

Comment thread helpers/captions.py
f"{_wrap_two_lines(text)}"
)
output.parent.mkdir(parents=True, exist_ok=True)
output.write_text(header + "\n".join(events) + "\n", encoding="utf-8")

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When master.ass already exists, write_substation overwrites it unconditionally. Reject existing files and symlinks before writing, matching the other caption writers’ output-protection behavior.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/captions.py, line 219:

<comment>When `master.ass` already exists, `write_substation` overwrites it unconditionally. Reject existing files and symlinks before writing, matching the other caption writers’ output-protection behavior.</comment>

<file context>
@@ -0,0 +1,260 @@
+            f"{_wrap_two_lines(text)}"
+        )
+    output.parent.mkdir(parents=True, exist_ok=True)
+    output.write_text(header + "\n".join(events) + "\n", encoding="utf-8")
+
+
</file context>
Suggested change
output.write_text(header + "\n".join(events) + "\n", encoding="utf-8")
if output.exists() or output.is_symlink():
raise FileExistsError("choose a new subtitle output path")
output.write_text(header + "\n".join(events) + "\n", encoding="utf-8")
Fix with cubic

Comment thread helpers/render.py
deliverable: dict | None = None,
) -> None:
"""Composite and normalize one legacy output or declared deliverable."""
out_path.parent.mkdir(parents=True, exist_ok=True)

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When a declared legacy deliverable already exists, render_one_output allows FFmpeg's -y commands to overwrite it. Reject existing files and symlinks before starting the render.

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

<comment>When a declared legacy deliverable already exists, `render_one_output` allows FFmpeg's `-y` commands to overwrite it. Reject existing files and symlinks before starting the render.</comment>

<file context>
@@ -594,92 +644,900 @@ def apply_loudnorm_two_pass(
+    deliverable: dict | None = None,
+) -> None:
+    """Composite and normalize one legacy output or declared deliverable."""
+    out_path.parent.mkdir(parents=True, exist_ok=True)
+    render_base = base_path
+    reframed_path: Path | None = None
</file context>
Suggested change
out_path.parent.mkdir(parents=True, exist_ok=True)
if out_path.exists() or out_path.is_symlink():
raise FileExistsError(f"{out_path}; choose a new versioned output")
out_path.parent.mkdir(parents=True, exist_ok=True)
Fix with cubic

Comment thread helpers/project_state.py
@@ -0,0 +1,126 @@
"""Small dependency-aware project context with explicit evidence and stale-state detection."""

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When an existing context omits artifacts, validation accepts it but both record() and view() later index the missing key, so an accepted empty context cannot be initialized or shown. Normalize loaded contexts with setdefault("artifacts", {}) and use the same default in view(), or reject missing artifacts during validation.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/project_state.py, line 56:

<comment>When an existing context omits `artifacts`, validation accepts it but both `record()` and `view()` later index the missing key, so an accepted empty context cannot be initialized or shown. Normalize loaded contexts with `setdefault("artifacts", {})` and use the same default in `view()`, or reject missing `artifacts` during validation.</comment>

<file context>
@@ -0,0 +1,126 @@
+def record(context, ident, entry):
+    """Save an artifact fingerprint and the dependency versions it was built from."""
+    context = Path(context)
+    data = load_json(context) if context.exists() else {"version": 1, "artifacts": {}}
+    path = resolve(context.parent, entry["path"])
+    if not path.is_file():
</file context>
Fix with cubic

Comment thread helpers/track_mask.py
rows = {
seed_frame: {
"frame": seed_frame,
"polygon": polygon,

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When callers pass a NumPy polygon, track leaves an ndarray in the seed row and main fails in save_json after tracking. Store the normalized polygon with .tolist() like the propagated rows.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/track_mask.py, line 103:

<comment>When callers pass a NumPy polygon, `track` leaves an ndarray in the seed row and `main` fails in `save_json` after tracking. Store the normalized polygon with `.tolist()` like the propagated rows.</comment>

<file context>
@@ -0,0 +1,200 @@
+    rows = {
+        seed_frame: {
+            "frame": seed_frame,
+            "polygon": polygon,
+            "needs_review": False,
+            "seed": True,
</file context>
Suggested change
"polygon": polygon,
"polygon": points_array.tolist(),
Fix with cubic

Comment thread helpers/cut_list.py

filter_chain(clip.get("filters", []))
gain_envelope(1, [], clip.get("gain_db", 0))
for ident, word in words.items():

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Word records bypass known(), so unsupported word treatments are accepted and ignored despite the validator's no-silent-discard contract. Reject extra word fields before reading the timestamps.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/cut_list.py, line 165:

<comment>Word records bypass `known()`, so unsupported word treatments are accepted and ignored despite the validator's no-silent-discard contract. Reject extra word fields before reading the timestamps.</comment>

<file context>
@@ -0,0 +1,312 @@
+
+        filter_chain(clip.get("filters", []))
+        gain_envelope(1, [], clip.get("gain_db", 0))
+    for ident, word in words.items():
+        if word.get("source") not in manifest["sources"]:
+            raise ValueError(f"{ident}: missing word source")
</file context>
Fix with cubic

Comment thread tests/test_visuals.py

# generated canvas filters execute successfully with actual ffmpeg input
@pytest.mark.parametrize("fit", ["contain", "cover", "blur"])
def test_canvas_filters_encode(fit, tmp_path):

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This test invokes the ffmpeg binary unconditionally, so on a machine or CI runner without ffmpeg it fails the whole test file with FileNotFoundError instead of skipping the one ffmpeg-dependent case. Every other ffmpeg-dependent test in this suite (test_audio_tracks.py:15, test_composition.py:35, test_render_treatment.py:54-56, test_sources.py:29, test_mix_audio.py:19, test_cards.py:111) guards with shutil.which(...) and pytest.skip/skipif. Add the same guard here.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_visuals.py, line 254:

<comment>This test invokes the ffmpeg binary unconditionally, so on a machine or CI runner without ffmpeg it fails the whole test file with FileNotFoundError instead of skipping the one ffmpeg-dependent case. Every other ffmpeg-dependent test in this suite (test_audio_tracks.py:15, test_composition.py:35, test_render_treatment.py:54-56, test_sources.py:29, test_mix_audio.py:19, test_cards.py:111) guards with `shutil.which(...)` and `pytest.skip`/`skipif`. Add the same guard here.</comment>

<file context>
@@ -0,0 +1,284 @@
+
+# generated canvas filters execute successfully with actual ffmpeg input
+@pytest.mark.parametrize("fit", ["contain", "cover", "blur"])
+def test_canvas_filters_encode(fit, tmp_path):
+    import subprocess
+
</file context>
Fix with cubic

Comment thread helpers/mix_audio.py
Comment on lines +95 to +98
if not math.isfinite(float(base_db)) or any(
not math.isfinite(float(p[1])) for p in points
):
raise ValueError("gain must be finite")

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When a CLI manifest contains a malformed gain_points entry, this code indexes p[1] before checking its shape. build() then exits with IndexError/TypeError instead of a clear validation error; validate each point as a two-item numeric pair first.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At helpers/mix_audio.py, line 95:

<comment>When a CLI manifest contains a malformed `gain_points` entry, this code indexes `p[1]` before checking its shape. `build()` then exits with `IndexError`/`TypeError` instead of a clear validation error; validate each point as a two-item numeric pair first.</comment>

<file context>
@@ -0,0 +1,303 @@
+def gain_envelope(count, points, base_db=0):
+    if type(count) is not int or count <= 0:
+        raise ValueError("gain envelope needs a positive integer sample count")
+    if not math.isfinite(float(base_db)) or any(
+        not math.isfinite(float(p[1])) for p in points
+    ):
</file context>
Suggested change
if not math.isfinite(float(base_db)) or any(
not math.isfinite(float(p[1])) for p in points
):
raise ValueError("gain must be finite")
try:
base_finite = math.isfinite(float(base_db))
malformed = not isinstance(points, (list, tuple)) or any(
not isinstance(p, (list, tuple))
or len(p) != 2
or type(p[0]) is not int
or not math.isfinite(float(p[1]))
for p in points
)
except (TypeError, ValueError, IndexError):
base_finite = False
malformed = True
if not base_finite or malformed:
raise ValueError("gain must be finite and points must be two-item numeric pairs")
Fix with cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant