From bf73835e3b361bdeb6e703d0adaf8b6fa19bf1a5 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Fri, 14 Aug 2026 02:24:53 +0300 Subject: [PATCH] fix: wiring survives a project load; Frames says when it reaches nothing; anchor survives upscale_2x MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three unrelated reports. Wires lost or "(not allowed)" after loading a project. /api/pipeline-ports read the family from the GLOBAL models.json, so it described whichever project was saved last. Open one on the other family and its saved wires point at ports that family has no rule for. It takes ?pid= now, and the panel asks for the config before the rules that depend on it. Reconcile also dropped a port wire when core_overrides named a deleted slot — it cleared every other slot's wire to that port, then failed to add the replacement. Guided mode also filtered harder than the builder validates. A node added through "Any node…" has role `custom`, which no rule table mentions, so a custom VAE loader could not reach the pipeline at all. A role with no rule for an output type now reaches any port of that type; a role that HAS one keeps it, and internal links stay closed. Both sides read the same list, since the panel hiding what the builder accepts is what produced "(not allowed)". Frames / scene silently missed shots. Any timeline resize, trim or split moves a scene off project length for good, and nothing said so — typing 305 and rendering 450 read as the edit being ignored. The Project panel now says how many shots it doesn't reach and offers to put them back. The panel's fingerprint had to learn about scene state, or the warning outlived the fix. second_pass_op=upscale_2x no longer drops the i2v pin. The pinned frames in the upscaled latent ARE the anchor — carried through the same upsampler as the rest of the clip — so only the mask needed scaling to the new grid. Pass 2 at 4x the pixels was the run most likely to drift off the reference, and it was the one running unpinned. The pin now holds the upscaled anchor rather than the encoded source image, and the report says so. Guide keyframes are still dropped: those are token indices, not pixels. Co-Authored-By: Claude Opus 5 --- movie_editor/backend/pipeline_wiring.py | 57 ++++++++++++------ movie_editor/frontend/api.js | 2 +- movie_editor/frontend/engine_settings.js | 2 +- movie_editor/frontend/inspector.js | 18 ++++++ movie_editor/frontend/models.js | 27 ++++++--- movie_editor/frontend/store.js | 24 +++++++- movie_editor/frontend/styles.css | 1 + movie_editor/frontend/view_bus.js | 4 ++ movie_editor/server.py | 10 +++- movie_editor/tests/test_pipeline_wiring.py | 41 +++++++++++++ movie_editor/tests/test_server.py | 19 ++++++ samplers.py | 70 +++++++++++++++++++--- tests/test_second_pass.py | 32 +++++++++- 13 files changed, 265 insertions(+), 42 deletions(-) diff --git a/movie_editor/backend/pipeline_wiring.py b/movie_editor/backend/pipeline_wiring.py index 2b059a6e..b85c5f08 100644 --- a/movie_editor/backend/pipeline_wiring.py +++ b/movie_editor/backend/pipeline_wiring.py @@ -200,6 +200,36 @@ def _chain_terminals(family: str) -> dict: return out +def _type_fallback_ports(family: str) -> dict[str, list[str]]: + """Ports an output of a given type may reach when its own role has no rule for that type. + + Guided wiring exists to keep the core's INTERNAL links fixed, not to decide which node is + allowed to fill an open socket. Restricting to role rules did the latter: a node added + through "Any node…" has role ``custom``, which no table mentions, and the chain terminals + cover only MODEL/CLIP/LATENT/IMAGE — so a custom VAE loader could not be wired into the + pipeline at all, and an already-saved wire read back as "(not allowed)". + + So the fallback is every port some role can reach with this type, minus the hidden + internal ones. Roles that DO have a rule keep it: audio_encoder's LATENT still has to + land on Concat · audio_latent and nowhere else. + """ + out: dict[str, list[str]] = {} + + def add(t, port): + lst = out.setdefault(t, []) + if port not in lst: + lst.append(port) + + for rules in _role_targets(family).values(): + for t, _name, port in rules: + add(t, port) + for t, ports in _chain_terminals(family).items(): + for port in ports: + add(t, port) + hidden = _hidden_ports(family) + return {t: [p for p in ports if p not in hidden] for t, ports in out.items()} + + def _default_wires(family: str) -> dict: out = {k: dict(v) for k, v in DEFAULT_WIRES_BY_ROLE.items()} out.update({k: dict(v) for k, v in (FAMILY_WIRING.get(family, {}).get("default_wires") or {}).items()}) @@ -252,7 +282,6 @@ def wiring_locked(models: Any) -> bool: def allowed_port_ids(role: str, out_type: str, out_name: Optional[str] = None, family: str = DEFAULT_FAMILY) -> list[str]: role_targets = _role_targets(family) - terminals = _chain_terminals(family) out: list[str] = [] role_rules = role_targets.get(role or "", []) for t, name, port in role_rules: @@ -261,11 +290,12 @@ def allowed_port_ids(role: str, out_type: str, out_name: Optional[str] = None, if name is not None and out_name is not None and name != out_name: continue out.append(port) - # Generic chain terminals only when the role has no explicit rule for this type - # (e.g. audio_encoder LATENT must stay on Concat · audio_latent, not Studio · latent). + # Only when the role has no rule for this type at all (an unknown or custom role, or a + # role that simply says nothing about this output). A role that DOES have one keeps it: + # audio_encoder LATENT stays on Concat · audio_latent, not Studio · latent. has_explicit = any(t == out_type for t, _, _ in role_rules) if not has_explicit: - for port in terminals.get(out_type, []): + for port in _type_fallback_ports(family).get(out_type, []): if port not in out: out.append(port) return out @@ -305,20 +335,9 @@ def validate_port_wire( f"{out_name} ({out_type}) cannot wire to {port_label(port_id, family)} in guided mode " f"(internal core link). Enable Full control to override." ) + # allowed_port_ids falls back to every port of this type for a role with no rule, which + # is what a custom or legacy slot has — no special case needed here any more. allowed = allowed_port_ids(role, out_type, out_name, family=family) - if not allowed and role in ("custom", "", None): - # Legacy slots without a role: allow canonical built-in ports for this output type. - for rules in _role_targets(family).values(): - for t, name, port in rules: - if t != out_type: - continue - if name is not None and name != out_name: - continue - if port not in allowed: - allowed.append(port) - for port in _chain_terminals(family).get(out_type, []): - if port not in allowed: - allowed.append(port) if not allowed: return ( f"{out_name or out_type} from role '{role}' cannot wire into the built-in pipeline " @@ -385,6 +404,10 @@ def wiring_rules_payload(family: str = DEFAULT_FAMILY) -> dict[str, Any]: "family": family, "role_targets": role_targets, "type_chain_terminals": _chain_terminals(family), + # What a role with no rule for this output type may reach. The panel has to filter + # destinations exactly as validate_port_wire does, or it hides a wire the builder + # would have accepted — which is how a saved wire came back "(not allowed)". + "type_fallback_ports": _type_fallback_ports(family), "guided_hidden_ports": sorted(_hidden_ports(family)), "default_wires": _default_wires(family), "default_input_sources": DEFAULT_INPUT_SOURCES_BY_ROLE, diff --git a/movie_editor/frontend/api.js b/movie_editor/frontend/api.js index 18cb63e3..29937b8a 100644 --- a/movie_editor/frontend/api.js +++ b/movie_editor/frontend/api.js @@ -116,9 +116,9 @@ nodeCandidates: (role, refresh) => j("GET", API(`/node-candidates/${role}${refresh ? "?refresh=true" : ""}`)), allNodes: () => j("GET", API("/all-nodes")), nodeSpec: (cls) => j("GET", API(`/node/${encodeURIComponent(cls)}`)), - pipelinePorts: () => j("GET", API("/pipeline-ports")), // pid is optional but should be passed whenever a project is open: without it the answer // describes the GLOBAL default, which is not necessarily this project's family. + pipelinePorts: (pid) => j("GET", API("/pipeline-ports" + (pid ? "?pid=" + encodeURIComponent(pid) : ""))), pipelineDeps: (pid) => j("GET", API("/pipeline-deps" + (pid ? "?pid=" + encodeURIComponent(pid) : ""))), pipelineDepsInstall: (packIds) => j("POST", API("/pipeline-deps/install"), { pack_ids: packIds }), pipelineDepsInstallManager: () => j("POST", API("/pipeline-deps/install"), { install_manager: true }), diff --git a/movie_editor/frontend/engine_settings.js b/movie_editor/frontend/engine_settings.js index f046b3e9..54e265c3 100644 --- a/movie_editor/frontend/engine_settings.js +++ b/movie_editor/frontend/engine_settings.js @@ -309,7 +309,7 @@ detail: "The anchor is generated at full strength, then this many frames are dropped from the FRONT of the finished clip, so an i2v render reads as t2v without weakening the anchor. Nothing is regrown: the scene comes out shorter and the audio is cropped to match. 8 removes only the anchor and is usually too little; 48 worked on a 768x768x305@30 chain. Skipped on continuation scenes and scenes carrying guides." }, { name: "second_pass_op", label: "Between-pass operation", kind: "combo", choices: ["none", "sharpen", "upscale_2x"], default: "none", hint: "'sharpen' adds detail almost free; 'upscale_2x' doubles the output resolution at 3-5x the cost of pass 2.", - detail: "Both need the LTX 2.3 latent upsampler (~1 GB, found or downloaded once); without it pass 2 still runs and the report says the operation was skipped. 'sharpen' resamples straight back to size — no video-model calls, and it cannot fix wrong structure. 'upscale_2x' keeps the 2x, so pass 2 costs 3-5x and drops the i2v pin. Video only." }, + detail: "Both need the LTX 2.3 latent upsampler (~1 GB, found or downloaded once); without it pass 2 still runs and the report says the operation was skipped. 'sharpen' resamples straight back to size — no video-model calls, and it cannot fix wrong structure. 'upscale_2x' keeps the 2x, so pass 2 costs 3-5x; the i2v anchor is upscaled with the clip and stays pinned, but guide keyframes are dropped for pass 2. Video only." }, { name: "context_windows", label: "Context windows (long scenes)", kind: "bool", default: false, hint: "Renders very long scenes as overlapping windows instead of one pass. Slower short, faster past ~300 frames.", detail: "Denoises a scene longer than the model's comfortable window as overlapping windows — core's own mechanism, audio-aware on LTX. Engages only past the window length below. About 1.45x the per-frame work at the defaults, roughly break-even near 200 frames and a win past ~300. Needs ComfyUI v0.29.0 or newer." }, diff --git a/movie_editor/frontend/inspector.js b/movie_editor/frontend/inspector.js index e9fefe75..ee5301d1 100644 --- a/movie_editor/frontend/inspector.js +++ b/movie_editor/frontend/inspector.js @@ -473,6 +473,24 @@ } row1.append(fpsField); body.append(row1); + + // A scene leaves "project" length the moment it is resized, trimmed or split, and from + // then on this field does not reach it. Nothing said so, so typing a new number and + // watching the render come out the old length read as the editor ignoring the edit. + const overridden = S.scenesOverridingProjectFrames ? S.scenesOverridingProjectFrames() : []; + const gen = (p.scenes || []).filter((s) => s && !S.isVideoClip?.(s)); + if (overridden.length) { + const warn = el("div", "insp-hint warn"); + warn.textContent = overridden.length >= gen.length + ? `Frames doesn't reach any shot — all ${overridden.length} have their own length from the timeline.` + : `Frames doesn't reach ${overridden.length} of ${gen.length} shots — they have their own length from the timeline.`; + const fix = el("button", "btn ghost tiny", "Use project length everywhere"); + fix.title = "Put every generated shot back on the project's Frames value. Video clips keep " + + "their own length. Undoable."; + fix.onclick = () => S.useProjectFramesEverywhere(); + warn.append(fix); + body.append(warn); + } const row2 = el("div", "fields-row"); row2.append(numberField("Width", p.width != null ? p.width : 768, (v) => S.patchProjectQuiet({ width: v }), "pj-w")); row2.append(numberField("Height", p.height != null ? p.height : 512, (v) => S.patchProjectQuiet({ height: v }), "pj-h")); diff --git a/movie_editor/frontend/models.js b/movie_editor/frontend/models.js index 33a8277d..d95e4c80 100644 --- a/movie_editor/frontend/models.js +++ b/movie_editor/frontend/models.js @@ -59,13 +59,17 @@ if (!pipelineLocked()) return all; const role = slot.role || "custom"; const rules = wiringRules.role_targets?.[role] || []; - const chainPorts = wiringRules.type_chain_terminals?.[out.type] || []; const hidden = new Set(wiringRules.guided_hidden_ports || []); - const allowedPorts = [...rules + // Must match pipeline_wiring.allowed_port_ids exactly: a role's own rules when it has any + // for this type, otherwise every port of the type. Filtering harder than the builder + // validates is what made saved wires read back as "(not allowed)". + const explicit = rules .filter((r) => r.type === out.type && (r.output_name == null || r.output_name === out.name)) - .map((r) => "port:" + r.port), - ...chainPorts.map((p) => "port:" + p), - ].filter((p, i, a) => a.indexOf(p) === i); + .map((r) => "port:" + r.port); + const fallback = rules.some((r) => r.type === out.type) + ? [] + : (wiringRules.type_fallback_ports?.[out.type] || []).map((p) => "port:" + p); + const allowedPorts = [...explicit, ...fallback].filter((p, i, a) => a.indexOf(p) === i); return all.filter((d) => { if (!d.value.startsWith("port:")) return true; const id = d.value.slice(5); @@ -113,6 +117,10 @@ const src = config.core_overrides?.[cid]?.[inp]; const parsed = _parseOutSource(src); if (!parsed) continue; + // An override naming a slot that no longer exists is stale — _addWire below would + // no-op on it while _clearPortWires still stripped everyone ELSE's wire to that port, + // so the surviving loader lost its wire on load. Drop the override, keep the wire. + if (!slotById(parsed.slotId)) { delete config.core_overrides[cid][inp]; continue; } _clearPortWires(portId, parsed.slotId, parsed.out); _addWire(parsed.slotId, parsed.out, "port:" + portId); } @@ -1523,7 +1531,7 @@ // the family decides which core nodes exist and which ports loaders may wire // into, so both have to be re-fetched before the panel redraws try { - const pp = await API.pipelinePorts(); + const pp = await API.pipelinePorts(window.Store?.get().project?.id); ports = pp.ports || ports; requirements = pp.requirements || requirements; wiringRules = pp.wiring || wiringRules; @@ -1781,14 +1789,17 @@ async function loadAll() { await ensureRoles(); + const pid = window.Store?.get().project?.id; + // Config FIRST: the ports and wiring rules depend on the project's model family, so + // asking for them before we know it answered for whatever family was saved globally. + try { config = await API.getModels(pid); } catch (_) { config = { slots: [] }; } try { - const pp = await API.pipelinePorts(); + const pp = await API.pipelinePorts(pid); ports = pp.ports || []; coreProducers = pp.core_producers || []; requirements = pp.requirements || []; wiringRules = pp.wiring || {}; } catch (_) { ports = []; coreProducers = []; requirements = []; wiringRules = {}; } - try { config = await API.getModels(window.Store?.get().project?.id); } catch (_) { config = { slots: [] }; } reconcileOpenPortWiring(); try { coreNodes = (await API.coreGraph(window.Store?.get().project?.id)).nodes || []; } catch (_) { coreNodes = []; } await prewarmSpecs(); diff --git a/movie_editor/frontend/store.js b/movie_editor/frontend/store.js index a900479e..6defbfc1 100644 --- a/movie_editor/frontend/store.js +++ b/movie_editor/frontend/store.js @@ -1838,6 +1838,28 @@ return sc.frames != null ? sc.frames : p.num_frames_per_scene; } + // Generative scenes whose own length overrides the project's Frames field. Any timeline + // resize, trim or split flips a scene out of "project" mode, and from then on editing the + // project number does nothing to it — silently, which reads as the field being ignored. + function scenesOverridingProjectFrames() { + if (!state.project) return []; + return (state.project.scenes || []).filter( + (s) => s && !isVideoClip(s) && (s.frames_mode || "project") !== "project"); + } + + // Put those scenes back on the project's length. Video clips keep theirs: their length is + // the source file's, not a preference. + function useProjectFramesEverywhere() { + if (!state.project) return; + const targets = scenesOverridingProjectFrames(); + if (!targets.length) return; + _historyRecord(); + targets.forEach((s) => { s.frames_mode = "project"; s.frames = null; }); + _afterTimelineStructureChange(); + notify(); + scheduleSave(); + } + function sceneEffFps(sc, project) { const p = project || state.project; if (!sc || !p) return p?.frame_rate || 25; @@ -4372,7 +4394,7 @@ bringOverlayToFront, sendOverlayToBack, bringOverlayForward, sendOverlayBackward, addImageOverlay, addTextOverlay, updateOverlayTrack, removeOverlayTrack, removeSelectedOverlay, isOverlayAudioTrack, isSeparatedAudioTrack, - resizeScene, setSceneGapAfter, splitScene, autoMontage, hasPlayableRender, snapFrames, snapFramesFloor, snapFramesCeil, sceneEffFrames, sceneEffFps, setSourceTrim, trimSceneLeft, slipScene, + resizeScene, setSceneGapAfter, splitScene, autoMontage, hasPlayableRender, snapFrames, snapFramesFloor, snapFramesCeil, sceneEffFrames, sceneEffFps, scenesOverridingProjectFrames, useProjectFramesEverywhere, setSourceTrim, trimSceneLeft, slipScene, applyEnginePreset, ENGINE_PRESETS, undo, redo, refreshPreview, syncFromPreview, applyGlobalPromptQuiet, scheduleGlobalPromptApply, globalPromptApplyPending, buildGlobalPromptFromTimeline, syncGlobalPromptFromTimeline, generate: _clockedGenerate, generateMontage: _clockedMontage, generateSelected: _clockedSelected, genElapsed, selectedSceneCount, renderFinal, exportSelected, saveSelectedToMediaBin, clipSaveableToMediaBin, interrupt, loadModels, loadImageTargets, setModelInput, setModelBypass, setModelLink, clearNotice, projectVariables, setProjectVariables, promptTemplates, savePromptTemplate, deletePromptTemplate, applyPromptTemplate, diff --git a/movie_editor/frontend/styles.css b/movie_editor/frontend/styles.css index 5759a8ce..87074100 100644 --- a/movie_editor/frontend/styles.css +++ b/movie_editor/frontend/styles.css @@ -817,6 +817,7 @@ input[type=number] { font-family: var(--font-mono); } .insp-tag { font-size: var(--fs-subhead); line-height: var(--lh-subhead); color: var(--muted); font-weight: 600; } .insp-hint { font-size: 11px; color: var(--faint); margin: -4px 0 12px; } .insp-hint.warn { color: var(--accent-hi, var(--accent)); } +.insp-hint .btn { margin-left: 6px; vertical-align: baseline; } .insp-block { margin-bottom: 16px; } /* global prompt — first entry in the inspector list (scrolls with content) */ .insp-global { margin: 0 0 14px; padding: 10px 10px 8px; diff --git a/movie_editor/frontend/view_bus.js b/movie_editor/frontend/view_bus.js index 03cfe91b..750d9e2e 100644 --- a/movie_editor/frontend/view_bus.js +++ b/movie_editor/frontend/view_bus.js @@ -60,6 +60,10 @@ conditioning_slot: p.conditioning_slot, sampler_slot: p.sampler_slot, studio_inputs: p.studio_inputs, sampler_inputs: p.sampler_inputs, guide_settings: p.guide_settings, continuity_settings: p.continuity_settings, + // The Project panel warns when its Frames field reaches no shot, so it depends on + // scene state even with no scene selected — without this the warning stays on screen + // after "Use project length everywhere" has already cleared it. + ownLength: (p.scenes || []).filter((s) => (s.frames_mode || "project") !== "project").length, }, preview: st.preview ? { warning: st.preview.warning, parse_error: st.preview.parse_error, diff --git a/movie_editor/server.py b/movie_editor/server.py index 8e9fcdbd..80f10f52 100644 --- a/movie_editor/server.py +++ b/movie_editor/server.py @@ -2377,12 +2377,18 @@ if web is not None and PromptServer is not None: return web.json_response({"cancelled": True, "job_id": job_id}) @routes.get(UI_PREFIX + "/api/pipeline-ports") - async def _pipeline_ports(_req): + async def _pipeline_ports(req): try: oi = await bridge.object_info() except Exception: oi = None - fam = pipeline_wiring.family_of(nodes.load_models()) + # The family decides which ports exist and which wires are legal, and it lives on the + # PROJECT. Answering from the global default described whatever project was saved last, + # so opening one on the other family read its saved wires back as "(not allowed)" and + # let reconcile drop them against stale overrides. + pid = req.query.get("pid") + fam = pipeline_wiring.family_of( + _project_models(projects.get(pid)) if pid else nodes.load_models()) return web.json_response({ "family": fam, "ports": nodes.pipeline_ports(oi, fam), diff --git a/movie_editor/tests/test_pipeline_wiring.py b/movie_editor/tests/test_pipeline_wiring.py index 9c2550ec..379b38d7 100644 --- a/movie_editor/tests/test_pipeline_wiring.py +++ b/movie_editor/tests/test_pipeline_wiring.py @@ -172,3 +172,44 @@ def test_core_graph_marks_locked_inputs(): assert model_in["locked"] is False assert latent_in["locked"] is False assert pos_in["locked"] is True + + +# ── a role with no rules ────────────────────────────────────────────────────── +# "Any node…" gives a slot role `custom`, which no rule table mentions. Guided mode is +# there to keep the core's INTERNAL links fixed, not to decide which node may fill an open +# socket — restricting to role rules meant a custom VAE loader could not be wired into the +# pipeline at all, and an already-saved wire read back as "(not allowed)". + +def test_custom_role_may_reach_a_port_of_its_type(): + ok = pipeline_wiring.validate_port_wire( + role="custom", out_type="VAE", out_name="VAE", + target="port:FunPackLTXAVSceneChainSampler.vae", models={"slots": []}, + ) + assert ok is None + + +def test_custom_role_still_cannot_reach_an_internal_link(): + err = pipeline_wiring.validate_port_wire( + role="custom", out_type="LATENT", out_name="LATENT", + target="port:LTXVConcatAVLatent.video_latent", models={"slots": []}, + ) + assert err is not None and "Studio · latent" in err + + +def test_a_role_with_a_rule_keeps_it(): + """The fallback must not widen a role that HAS an opinion about this type.""" + assert pipeline_wiring.allowed_port_ids("audio_encoder", "LATENT", family="ltxav") == \ + ["LTXVConcatAVLatent.audio_latent"] + + +def test_the_panel_filters_exactly_as_the_builder_validates(): + """models.js allowedDestinations() reads type_fallback_ports for a role with no rule. + A panel that hides a wire the builder would accept is how "(not allowed)" appeared.""" + for family in ("ltxav", "minimax_h3"): + payload = pipeline_wiring.wiring_rules_payload(family) + for out_type, ports in payload["type_fallback_ports"].items(): + assert ports == pipeline_wiring.allowed_port_ids( + "custom", out_type, family=family) + # and nothing internal leaks into it + for ports in payload["type_fallback_ports"].values(): + assert not set(ports) & set(payload["guided_hidden_ports"]) diff --git a/movie_editor/tests/test_server.py b/movie_editor/tests/test_server.py index df9e8468..3b06934c 100644 --- a/movie_editor/tests/test_server.py +++ b/movie_editor/tests/test_server.py @@ -563,3 +563,22 @@ def test_h3_references_round_trip_on_the_project(): assert Project.from_dict(p.to_dict()).h3_references == refs # absent on every project that predates the feature, never None assert Project.from_dict({}).h3_references == [] + + +def test_the_ports_endpoint_answers_for_the_project_not_the_global_default(monkeypatch): + """/api/pipeline-ports resolves its family the same way, from ?pid=. + + Without that it described whatever project was saved last, so opening one on the other + family read its saved wires back as "(not allowed)" and let the panel's reconcile drop + them against overrides for a core that isn't there. + """ + from movie_editor.backend import nodes, pipeline_wiring + + monkeypatch.setattr(nodes, "load_models", lambda: {"slots": [], "model_family": "ltxav"}) + p = _project(scenes=[{"id": "s1", "text": "a"}]) + p.models = {"slots": [{"id": "a", "role": "unet"}], "model_family": "minimax_h3"} + + assert pipeline_wiring.family_of(_project_models(p)) == "minimax_h3" + assert pipeline_wiring.family_of(nodes.load_models()) == "ltxav" + # No pid — an app with no project open — still answers with the global default. + assert pipeline_wiring.family_of(_project_models(None)) == "ltxav" diff --git a/samplers.py b/samplers.py index 8e2c98b8..6d8edadf 100644 --- a/samplers.py +++ b/samplers.py @@ -2863,7 +2863,7 @@ class FunPackLTXAVSceneChainSampler: }), "second_pass_op": (["none", "sharpen", "upscale_2x"], { "default": "none", - "tooltip": "OPTIONAL latent-space operation applied between the two passes — 'none' by default, nothing runs unless you pick one. 'sharpen': one forward of Lightricks' trained 2x latent upsampler, resampled straight back to the original size. No video-model calls at all, so it costs a fraction of a step; pass 2 then re-denoises the sharpened latent, which is what makes it stick. It adds detail consistent with what is already there and CANNOT fix structure that is wrong (an extra finger stays an extra finger, just sharper) — the same limit segmented detailing's sharpen mode documents. 'upscale_2x': the same upsampler, but the result is KEPT at 2x, so pass 2 runs at four times the pixels and the scene decodes at double resolution. That is 3-5x the sampling cost of the second half and it drops the i2v pin (the anchor and its mask are the old size, and rescaling them would be inventing an anchor) — the scene report says so when it happens. MULTI-SCENE works with it: a scene finishes at 2x while every later scene is still built from the latent template at the original size, so everything that crosses a scene boundary (carried overlap frames, the anchor's continuation, the soft join, JoyAI memory, per-scene guide sources) is brought back to the template's grid on the way. Each scene still samples and OUTPUTS at 2x — only the carried material is resampled, and only downwards, which is the direction that survives it: those frames exist to say 'continue from here', which a resample preserves far better than invented detail would. Both ops use the same upsampler file as segmented detailing (detail_upsampler, 'auto' downloads the official one on first use). Video stream only; audio is never reshaped.", + "tooltip": "OPTIONAL latent-space operation applied between the two passes — 'none' by default, nothing runs unless you pick one. 'sharpen': one forward of Lightricks' trained 2x latent upsampler, resampled straight back to the original size. No video-model calls at all, so it costs a fraction of a step; pass 2 then re-denoises the sharpened latent, which is what makes it stick. It adds detail consistent with what is already there and CANNOT fix structure that is wrong (an extra finger stays an extra finger, just sharper) — the same limit segmented detailing's sharpen mode documents. 'upscale_2x': the same upsampler, but the result is KEPT at 2x, so pass 2 runs at four times the pixels and the scene decodes at double resolution. That is 3-5x the sampling cost of the second half. The i2v pin SURVIVES it: the pinned frames are carried through the upsampler with everything else and the mask is scaled to the new grid, so pass 2 still holds the anchor — as the upscaled anchor rather than the encoded source image, which the scene report states. Guide keyframes do not survive, because they are token indices into the old grid; pass 1 uses them in full. MULTI-SCENE works with it: a scene finishes at 2x while every later scene is still built from the latent template at the original size, so everything that crosses a scene boundary (carried overlap frames, the anchor's continuation, the soft join, JoyAI memory, per-scene guide sources) is brought back to the template's grid on the way. Each scene still samples and OUTPUTS at 2x — only the carried material is resampled, and only downwards, which is the direction that survives it: those frames exist to say 'continue from here', which a resample preserves far better than invented detail would. Both ops use the same upsampler file as segmented detailing (detail_upsampler, 'auto' downloads the official one on first use). Video stream only; audio is never reshaped.", }), "h3_audio_clock": ("BOOLEAN", { "default": False, @@ -3786,6 +3786,37 @@ class FunPackLTXAVSceneChainSampler: except Exception: return False + def _rescale_mask_to(self, mask, height, width): + """A noise mask on `height`x`width`, or None if it cannot be built. + + Nearest-neighbour, and deliberately: this mask is a hard 0/1 pin, and interpolating + its edge would hand pass 2 half-pinned latent positions that are neither frozen nor + free. An i2v pin covers whole leading frames, so nearest is exact for it. + + Nested masks carry one entry per stream and only the video stream has spatial axes — + audio is passed through untouched, as everywhere else. + """ + if mask is None: + return None + try: + if self._is_nested(mask): + parts = list(mask.unbind()) + out = [self._rescale_mask_to(parts[0], height, width)] + [ + self._clone_value(m) for m in parts[1:]] + if out[0] is None: + return None + return comfy.nested_tensor.NestedTensor(out) + if not isinstance(mask, torch.Tensor) or mask.dim() < 4: + return None + if mask.shape[-2:] == (int(height), int(width)): + return self._clone_value(mask) + flat = mask.reshape(-1, 1, mask.shape[-2], mask.shape[-1]).float() + resized = torch.nn.functional.interpolate( + flat, size=(int(height), int(width)), mode="nearest") + return resized.reshape(*mask.shape[:-2], int(height), int(width)).to(mask.dtype) + except Exception: + return None + def _restore_pinned_prefix(self, state, chunk): """Put the i2v pin back on a latent that is about to be handed to a second pass. @@ -3793,20 +3824,34 @@ class FunPackLTXAVSceneChainSampler: would run UNPINNED and re-denoise the anchor frame, letting the reference drift. The pinned region's values are copied back from the original chunk as well, since pass 2 re-noises everything it is handed, including frames that are supposed to be frozen. + + When a resolution-changing second_pass_op (upscale_2x) has run, the chunk's anchor and + mask are the old size. The anchor is NOT re-derived from the source image: the pinned + frames already in `state` are the upsampler's own rendering of those very frames, so + they are kept as they are and only the mask is scaled to the new grid. That is the + same upscale every other frame in the clip just got, which is what makes it the anchor + and not an invention — but it does mean the pin at 2x holds the upsampled anchor + rather than the encoded source image. """ mask = chunk.get("noise_mask") pinned = self._anchor_pinned_frames(chunk) if mask is None or pinned <= 0: return state try: - # A resolution-changing second_pass_op (upscale_2x) leaves the chunk's anchor and - # mask the wrong size. Rescaling them here would be inventing an anchor; the - # caller reports the lost pin instead. - if self._latent_spatial_changed(state, chunk): - return state + upscaled = self._latent_spatial_changed(state, chunk) except Exception: return state result = self._clone_latent(state) + if upscaled: + try: + h, w = self._latent_tensors(result)[0].shape[-2:] + except Exception: + return state + scaled = self._rescale_mask_to(mask, int(h), int(w)) + if scaled is None: + return state + result["noise_mask"] = scaled + return result try: src = self._latent_tensors(chunk)[0] dst = self._latent_tensors(result) @@ -6928,14 +6973,21 @@ class FunPackLTXAVSceneChainSampler: # re-denoises the reference frame and the scene drifts away from the # image it was supposed to start from. _sp_before = _sp_state + _sp_pinned = self._anchor_pinned_frames(chunk) > 0 + _sp_resized = _sp_pinned and self._latent_spatial_changed(_sp_state, chunk) _sp_state = self._restore_pinned_prefix(_sp_state, chunk) - if _sp_state is _sp_before and self._anchor_pinned_frames(chunk) > 0: - # Only happens when the op changed the resolution — say so, because - # an unpinned pass 2 can drift off the reference image. + if _sp_state is _sp_before and _sp_pinned: + # The mask could not be rebuilt at the new size — an unpinned pass 2 + # can drift off the reference image, so say so. run_mechanisms.append( "second_pass NOTE: the i2v anchor could not stay pinned for pass 2 " "because second_pass_op changed the latent resolution — pass 2 runs " "unpinned and may drift from the reference image.") + elif _sp_resized: + run_mechanisms.append( + "second_pass NOTE: second_pass_op changed the resolution, so the i2v " + "anchor stays pinned as the UPSCALED anchor — the same upsampler pass " + "the rest of the clip got, not the encoded source image.") # Same resolution change, second casualty: guide keyframes are recorded as # TOKEN indices into the pass-1 grid, and 2x spatial means 4x the tokens per # latent frame, so those indices now address the wrong tokens entirely. diff --git a/tests/test_second_pass.py b/tests/test_second_pass.py index d5f143db..815829a3 100644 --- a/tests/test_second_pass.py +++ b/tests/test_second_pass.py @@ -184,12 +184,38 @@ def test_an_unknown_operation_is_reported_not_guessed(): assert out is lat and "unknown operation" in note -def test_the_pin_is_dropped_when_the_operation_changed_the_resolution(): - """upscale_2x leaves the chunk's anchor and mask the wrong size. Rescaling them would be - inventing an anchor, so the pin is dropped — and the caller reports it.""" +def test_the_pin_survives_an_operation_that_changed_the_resolution(): + """upscale_2x leaves the chunk's mask the wrong size, which used to drop the pin and let + pass 2 re-denoise the anchor at 4x the pixels — the run most likely to drift. + + The anchor is not re-derived from the source image: the pinned frames in the upscaled + state ARE the anchor, carried through the same upsampler as every other frame. Only the + mask has to be brought to the new grid. + """ clean = torch.zeros(1, 4, 4, 8, 8) mask = torch.ones_like(clean) mask[:, :, :1] = 0.0 chunk = {"samples": clean, "noise_mask": mask} + anchor = torch.full((1, 4, 1, 16, 16), 7.0) + upscaled = {"samples": torch.zeros(1, 4, 4, 16, 16)} + upscaled["samples"][:, :, :1] = anchor + + out = _node()._restore_pinned_prefix(upscaled, chunk) + assert out is not upscaled + m = out["noise_mask"] + assert m.shape[-2:] == (16, 16) + assert float(m[:, :, :1].max()) == 0.0 # anchor frame still pinned... + assert float(m[:, :, 1:].min()) == 1.0 # ...and nothing else is + # The upsampler's anchor is what stays pinned — untouched by the restore. + assert torch.equal(out["samples"][:, :, :1], anchor) + + +def test_the_pin_is_dropped_when_the_mask_cannot_be_rebuilt(): + """A mask with no spatial axes to scale is unusable at the new size. Dropping the pin and + saying so beats pinning against a grid that isn't there.""" + clean = torch.zeros(1, 4, 4, 8, 8) + mask = torch.ones(1, 4, 4) # 3-D: no H/W to rescale + mask[:, :1] = 0.0 + chunk = {"samples": clean, "noise_mask": mask} upscaled = {"samples": torch.zeros(1, 4, 4, 16, 16)} assert _node()._restore_pinned_prefix(upscaled, chunk) is upscaled