diff --git a/movie_editor/backend/builder.py b/movie_editor/backend/builder.py index 66858a4d..69c1ac2d 100644 --- a/movie_editor/backend/builder.py +++ b/movie_editor/backend/builder.py @@ -20,6 +20,7 @@ Output is a ComfyUI /prompt graph: {node_id: {class_type, inputs}}. from __future__ import annotations import json +import re import uuid from typing import Any, Optional @@ -993,6 +994,35 @@ def build(object_info: dict, models_config: dict, params: dict, media: dict | No report["blocking"].append(msg) report["unsatisfied"].append(msg) + # 6. media-OR: when a node has a video reference AND its same-index image reference + # both wired (e.g. you left an image-reference workflow's socket filled and then also + # filled the matching video-reference one), the video wins and the image socket is + # dropped — sending both is redundant at best and an unintended double reference at + # worst. Opt-out per node (slot.media_or === False); on by default. Runs last, after + # every wiring phase, so it only sees sockets that actually ended up filled. + # Dropping ref_image_N alone would leave a gap if ref_image_(N+1) survives — ordinary + # multi-reference use, not an edge case, for a list whose whole point is more than one + # entry — and V3 autogrow expansion needs contiguous indices from 0. So each affected + # list's survivors get re-indexed afterward to close any gap the drop left behind. + reindex_groups: set[tuple[str, str, str]] = set() + for s in slots: + if s.get("media_or") is False: + continue + sid = slot_node_id.get(s["id"]) + if not sid or sid not in graph: + continue + node_inputs = graph[sid]["inputs"] + for image_name, video_name in _media_or_pairs(slot_def.get(s["id"])): + if isinstance(node_inputs.get(image_name), list) and isinstance(node_inputs.get(video_name), list): + del node_inputs[image_name] + report["wired"].append( + f"{s.get('node_class')}.{image_name}: dropped — {video_name} is also " + "filled and takes priority (video over image reference).") + parent, child = image_name.split(".", 1) + reindex_groups.add((sid, parent, re.sub(r"\d+$", "", child))) + for sid, parent, prefix in reindex_groups: + _reindex_autogrow_group(graph[sid]["inputs"], parent, prefix) + return graph, report @@ -1165,6 +1195,37 @@ def _canonical_input(node_def: Optional[dict], name: str) -> str: return name +def _media_or_pairs(node_def: Optional[dict]) -> list[tuple[str, str]]: + """Same-index autogrow sockets that name the same reference slot once as IMAGE and + once as VIDEO (MiniMax H3's ref_images.ref_image_0 / ref_videos.ref_video_0, and any + node following the same convention) — found generically by swapping "image" for + "video" in the dotted socket id, no per-node list to maintain.""" + names = {ci["name"] for ci in connection_inputs(node_def or {}) if ci.get("autogrow")} + pairs = [] + for name in names: + video_name = re.sub("image", "video", name, flags=re.IGNORECASE) + if video_name != name and video_name in names: + pairs.append((name, video_name)) + return pairs + + +def _reindex_autogrow_group(node_inputs: dict, parent: str, prefix: str) -> None: + """Renumber `.` entries still present in `node_inputs` to be + contiguous from 0, preserving relative order — used after media-OR removes one entry + from the middle of an autogrow list, which V3 expansion otherwise can't parse.""" + entries = [] + for key in node_inputs: + if not key.startswith(parent + "."): + continue + suffix = key[len(parent) + 1:][len(prefix):] + if key[len(parent) + 1:].startswith(prefix) and suffix.isdigit(): + entries.append((int(suffix), key)) + entries.sort() + for new_idx, (old_idx, key) in enumerate(entries): + if new_idx != old_idx: + node_inputs[f"{parent}.{prefix}{new_idx}"] = node_inputs.pop(key) + + def _resolve_target(target: str, port_to_core, slot_node_id) -> Optional[tuple[str, str]]: if target.startswith("port:"): return port_to_core.get(target[5:]) diff --git a/movie_editor/frontend/models.js b/movie_editor/frontend/models.js index 27845036..afcc970d 100644 --- a/movie_editor/frontend/models.js +++ b/movie_editor/frontend/models.js @@ -173,6 +173,20 @@ } } + // Same-index autogrow sockets naming the same reference slot once as IMAGE and once as + // VIDEO (MiniMax H3's ref_images.ref_image_0 / ref_videos.ref_video_0, and anything else + // following the convention) — mirrors backend/builder.py's _media_or_pairs so the toggle + // only appears where the backend will actually act on it. + function mediaOrPairs(connectionInputs) { + const names = new Set((connectionInputs || []).filter((ci) => ci.autogrow).map((ci) => ci.name)); + const pairs = []; + names.forEach((name) => { + const videoName = name.replace(/image/gi, "video"); + if (videoName !== name && names.has(videoName)) pairs.push([name, videoName]); + }); + return pairs; + } + function allowedSources(slot, ci) { const all = sources(slot, ci.type); if (!pipelineLocked()) return all; @@ -880,6 +894,16 @@ }); if (advBox) sbox.append(advBox); card.append(sbox); + + if (mediaOrPairs(cand.connection_inputs).length) { + const orRow = el("label", "field node-media-or-field"); + const cb = el("input"); cb.type = "checkbox"; cb.style.width = "auto"; + cb.checked = slot.media_or !== false; + cb.onchange = async () => { slot.media_or = cb.checked ? undefined : false; await persist(); }; + orRow.append(cb); + orRow.append(el("span", null, "When both an image and a video reference are filled at the same slot, send only the video")); + card.append(orRow); + } } if (cand && cand.inputs.length) { @@ -1900,6 +1924,7 @@ async function saveLinkSelection() { if (linkSel.length < 1) return; + if (!settleForegroundEdit()) return; if (linkTarget) return addLinkSelection(); const def = "size " + ((config.links || []).length + 1); const nm = prompt("Link name:", def); if (nm == null) return; @@ -2183,6 +2208,24 @@ _setView(v); } + // Link picking happens ON a node's own page (via beginEdit()'s deferSave buffer), which + // means an unrelated edit to that node's own fields can be sitting buffered when the + // user hits "Save link"/"Add". Previously that edit got persisted silently along with + // the link — the same dirty-change decision setView() already confirms before leaving a + // page, applied here since saving a link is also a page-leaving action. Discards the + // stray edit (not the link, which the caller creates afterward) on confirm; returns + // false to abort the save if the user declines. + function settleForegroundEdit() { + if (!deferDirty) return true; + if (!confirm( + "This node has unsaved changes that aren't part of the link.\n\n" + + "Discard them and save the link? Cancel to go back and save or discard them first." + )) return false; + restoreBaseline(); + deferDirty = false; + return true; + } + const mnItem = (opts) => window.SettingsWindow.navItem(opts); function sideBar(v) { diff --git a/movie_editor/tests/test_builder.py b/movie_editor/tests/test_builder.py index 4ebd10f7..e08dbaeb 100644 --- a/movie_editor/tests/test_builder.py +++ b/movie_editor/tests/test_builder.py @@ -649,6 +649,9 @@ REF_OI["MiniMaxH3ReferenceToVideo"] = { "ref_images": ["COMFY_AUTOGROW_V3", { "template": {"input": {"optional": {"ref_image": ["IMAGE", {}]}}, "prefix": "ref_image", "min": 0, "max": 4}}], + "ref_videos": ["COMFY_AUTOGROW_V3", { + "template": {"input": {"optional": {"ref_video": ["VIDEO", {}]}}, + "prefix": "ref_video", "min": 0, "max": 4}}], }}, "output": ["CONDITIONING"], "output_name": ["conditioning"]} @@ -761,6 +764,71 @@ REF_OI["LoadVideo"] = {"input": {"required": {"file": [["a.mp4"]]}}, "output": [ "output_name": ["VIDEO"]} +def test_media_or_drops_the_image_when_its_paired_video_is_also_filled(): + """Same reference index wired to both an IMAGE and a VIDEO autogrow socket: sending + both is redundant at best, so the video wins and the image socket is dropped.""" + models = {"slots": [ + {"id": "li", "node_class": "LoadImage", "inputs": {}, "wires": {}}, + {"id": "lv", "node_class": "LoadVideo", "inputs": {}, "wires": {}}, + {"id": "r", "node_class": "MiniMaxH3ReferenceToVideo", "inputs": {"prompt": "hi"}, + "wires": {}, + "input_sources": {"ref_images.ref_image0": "out:li:IMAGE", "ref_videos.ref_video0": "out:lv:VIDEO"}}, + ]} + graph, report = builder.build(REF_OI, models, PARAMS, media={"filename": "scene.png"}) + ins = graph["slot_r"]["inputs"] + assert "ref_images.ref_image0" not in ins + assert ins["ref_videos.ref_video0"] == ["slot_lv", 0] + assert any("ref_images.ref_image0" in m and "dropped" in m for m in report["wired"]) + + +def test_media_or_leaves_a_lone_image_reference_alone(): + """No paired video wired at that index — the image reference is untouched.""" + models = {"slots": [ + {"id": "li", "node_class": "LoadImage", "inputs": {}, "wires": {}}, + {"id": "r", "node_class": "MiniMaxH3ReferenceToVideo", "inputs": {"prompt": "hi"}, + "wires": {}, "input_sources": {"ref_images.ref_image0": "out:li:IMAGE"}}, + ]} + graph, _ = builder.build(REF_OI, models, PARAMS, media={"filename": "scene.png"}) + assert graph["slot_r"]["inputs"]["ref_images.ref_image0"] == ["slot_li", 0] + + +def test_media_or_reindexes_survivors_to_close_the_gap(): + """Two image refs (0 and 1, ordinary multi-reference use) plus one video ref at index + 0: dropping ref_image0 must not leave ref_image1 stranded at a non-zero index — V3 + autogrow expansion wants the list contiguous from 0.""" + models = {"slots": [ + {"id": "li0", "node_class": "LoadImage", "inputs": {}, "wires": {}}, + {"id": "li1", "node_class": "LoadImage", "inputs": {}, "wires": {}}, + {"id": "lv", "node_class": "LoadVideo", "inputs": {}, "wires": {}}, + {"id": "r", "node_class": "MiniMaxH3ReferenceToVideo", "inputs": {"prompt": "hi"}, + "wires": {}, "input_sources": { + "ref_images.ref_image0": "out:li0:IMAGE", + "ref_images.ref_image1": "out:li1:IMAGE", + "ref_videos.ref_video0": "out:lv:VIDEO"}}, + ]} + graph, _ = builder.build(REF_OI, models, PARAMS, media={"filename": "scene.png"}) + ins = graph["slot_r"]["inputs"] + assert "ref_images.ref_image1" not in ins # no gap left behind + assert ins["ref_images.ref_image0"] == ["slot_li1", 0] # survivor re-indexed down + assert ins["ref_videos.ref_video0"] == ["slot_lv", 0] # untouched, its own list has no gap + + +def test_media_or_can_be_switched_off_per_node(): + """slot.media_or === False opts a node out — both sockets stay, letting the node see + (and presumably reject or otherwise handle) both references itself.""" + models = {"slots": [ + {"id": "li", "node_class": "LoadImage", "inputs": {}, "wires": {}}, + {"id": "lv", "node_class": "LoadVideo", "inputs": {}, "wires": {}}, + {"id": "r", "node_class": "MiniMaxH3ReferenceToVideo", "inputs": {"prompt": "hi"}, + "wires": {}, "media_or": False, + "input_sources": {"ref_images.ref_image0": "out:li:IMAGE", "ref_videos.ref_video0": "out:lv:VIDEO"}}, + ]} + graph, _ = builder.build(REF_OI, models, PARAMS, media={"filename": "scene.png"}) + ins = graph["slot_r"]["inputs"] + assert ins["ref_images.ref_image0"] == ["slot_li", 0] + assert ins["ref_videos.ref_video0"] == ["slot_lv", 0] + + def _ref_params(*refs): p = dict(PARAMS) p["references"] = [dict(r, index=i + 1) for i, r in enumerate(refs)]