From 991abe072cd9cdaece4e55060989e88d27606819 Mon Sep 17 00:00:00 2001 From: twotalesanimation <80506065+twotalesanimation@users.noreply.github.com> Date: Fri, 14 Aug 2026 08:56:18 +0200 Subject: [PATCH] fix(validation): anchor the timecode check on a delivered plate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UNG_110_030_010 v002 failed with a 461614-frame offset on a render whose timecode was correct. Two causes, both in how the plate reference was recorded. The comp held three same-length elements and the timing plate was picked by duration alone, so a PRERENDER won on layer order. Using a pre-render as the timecode reference is circular anyway: it was rendered from this comp, so it can only ever agree with it — including when both are wrong. Only footage under the Footage Root is now eligible; among those, longest still wins. The offset itself came from the plate's frameStart. AE writes renders as "..._PRERENDER_01001.exr" while delivered plates use "....01001.exr", and the parser only handled the dot form, so the number fell back to the comp's frame range — 462615 against a render numbered from 1001. Both separators now parse, and an unparseable name omits the field rather than filling it with a number from the wrong coordinate system. Verified against the real manifest: the BG01 plate is now chosen over both pre-renders, and all three filenames parse to 1001. Panel 2.2.1, worker 1.2.1. Co-Authored-By: Claude Opus 5 --- RenderPipeline2.md | 3 +- .../VFXReviewWorker.Tests/ExrHeaderTests.cs | 6 ++ .../VFXReviewWorker/VFXReviewWorker.csproj | 2 +- .../VFXReviewWorker/ValidationStage.cs | 14 ++++- VFXReviewConnector.jsx | 57 ++++++++++++++----- 5 files changed, 65 insertions(+), 17 deletions(-) diff --git a/RenderPipeline2.md b/RenderPipeline2.md index 36e42db..0e5aa04 100644 --- a/RenderPipeline2.md +++ b/RenderPipeline2.md @@ -603,7 +603,8 @@ The server validates the manifest with Zod, cross-checks `shotId`/`shotCode`/fra **`source` (required from manifestVersion 2)** — the plate provenance the timecode check depends on (§8.1a). The panel resolves these from the *comp's own footage layers*, not by re-deriving paths, so the manifest records what was actually comped: -- `timingPlate` — the element the comp's start time was anchored to (the panel's `primaryFootage`: longest duration, first on tie). A shot's elements routinely carry unrelated camera timecodes, so "the plate" must be named explicitly rather than guessed by the worker. +- `timingPlate` — the delivered plate the render's timecode is checked against. A shot's elements routinely carry unrelated camera timecodes, so "the plate" must be named explicitly rather than guessed by the worker. **Only footage under the Footage Root qualifies**: a comp usually also holds pre-renders and CG passes of identical length, and anchoring on one of those is circular — it was rendered from this comp, so it can only ever agree with it, including when both are wrong. Among delivered plates, longest duration wins. +- `frameStart`/`frameEnd` on a plate are its **own file numbering**, and are omitted when the filename carries no frame number. They are never filled from the comp's frame range: the render's first frame is in output numbering, so a comp-frame anchor skews the comparison by the gap between the two systems. - `startSource` — verbatim from the panel's existing start-frame resolution: `"footage metadata"` (probed from the plate, trustworthy) or `"API timecode …"` (fallback, suspect). This is known at queue time, so an export built from the API fallback can be flagged **before** it burns render hours. - `plates[]` — every element imported into the comp, for the record and for cross-element diagnostics. diff --git a/RenderWorker/VFXReviewWorker.Tests/ExrHeaderTests.cs b/RenderWorker/VFXReviewWorker.Tests/ExrHeaderTests.cs index abefa23..11e9b0b 100644 --- a/RenderWorker/VFXReviewWorker.Tests/ExrHeaderTests.cs +++ b/RenderWorker/VFXReviewWorker.Tests/ExrHeaderTests.cs @@ -100,7 +100,13 @@ public class ExrHeaderTests [Theory] [InlineData("UNG_102_019_010_cmp_TT_v005.01001.exr", 1001)] [InlineData("UNG_108_030_010_cmp_TT_v003.01204.exr", 1204)] + // AE-written renders separate the frame with an underscore, and turn up as + // layers in a shot comp alongside dot-separated delivered plates. + [InlineData("UNG_110_030_010_PRERENDER_01001.exr", 1001)] + [InlineData("UNG_110_030_010_render_1001.exr", 1001)] [InlineData("no_frame_number.exr", null)] + // A version suffix is not a frame number. + [InlineData("UNG_110_030_010_BG01_TT_v001.exr", null)] public void ParsesFrameNumbersFromFilenames(string name, int? expected) { Assert.Equal(expected, SequencePattern.FrameFromName(name)); diff --git a/RenderWorker/VFXReviewWorker/VFXReviewWorker.csproj b/RenderWorker/VFXReviewWorker/VFXReviewWorker.csproj index 8bb313f..76fb9d7 100644 --- a/RenderWorker/VFXReviewWorker/VFXReviewWorker.csproj +++ b/RenderWorker/VFXReviewWorker/VFXReviewWorker.csproj @@ -15,7 +15,7 @@ every change that gets published. publish.ps1 appends the git SHA via SourceRevisionId, giving "1.1.0+feb1c7b". --> - 1.2.0 + 1.2.1 diff --git a/RenderWorker/VFXReviewWorker/ValidationStage.cs b/RenderWorker/VFXReviewWorker/ValidationStage.cs index f124177..2e41db2 100644 --- a/RenderWorker/VFXReviewWorker/ValidationStage.cs +++ b/RenderWorker/VFXReviewWorker/ValidationStage.cs @@ -409,7 +409,12 @@ public sealed class ValidationStage var header = File.Exists(local) ? ExrHeader.TryRead(local) : null; if (header?.Timecode is not null) { - var frame = SequencePattern.FrameFromName(Path.GetFileName(local)) ?? timing.FrameStart ?? m.FrameStart; + // The plate's own file numbering, never the comp's frame range: + // the render's first frame is in output numbering, so anchoring + // on a comp frame skews the comparison by the gap between them. + var frame = SequencePattern.FrameFromName(Path.GetFileName(local)) + ?? timing.FrameStart + ?? renderFirstFrame; return new TimecodeReference("manifest", local, header.Timecode.Value, frame, handleFrames); } _log.LogWarning("Timing plate from manifest is unreadable: {Path}", local); @@ -589,7 +594,12 @@ public sealed class SequencePattern public string Extension { get; private init; } = ".exr"; public int Padding { get; private init; } - private static readonly Regex FrameInName = new(@"\.(\d{2,9})\.[A-Za-z0-9]+$", RegexOptions.Compiled); + // Delivered plates separate the frame number with a dot + // ("..._BG01_TT_v001.01001.exr"); AE-written renders use an underscore + // ("..._PRERENDER_01001.exr"). Both appear as layers in a shot comp, so + // both have to parse — a plate whose number cannot be read gets anchored on + // a fallback, and a wrong anchor silently skews the timecode check. + private static readonly Regex FrameInName = new(@"[._](\d{2,9})\.[A-Za-z0-9]+$", RegexOptions.Compiled); public static SequencePattern FromOutputPattern(string outputPattern) { diff --git a/VFXReviewConnector.jsx b/VFXReviewConnector.jsx index 03d98bc..534e4ed 100644 --- a/VFXReviewConnector.jsx +++ b/VFXReviewConnector.jsx @@ -32,7 +32,7 @@ var SETTINGS_KEY = "cfg"; // Bump on every change deployed to a workstation. The panel reports this to // the server on load, so Pipeline → Machines shows which version each machine // is actually running and flags the ones left behind. -var PANEL_VERSION = "2.2.0"; +var PANEL_VERSION = "2.2.1"; (function VFXReviewConnector(thisObj) { var uiState = { @@ -3701,9 +3701,32 @@ var PANEL_VERSION = "2.2.0"; return plates; } - // The element the comp's timing came from: longest duration, first on tie — - // the same rule buildShotFromCode uses to pick primaryFootage. + // The element the comp's timing came from. + // + // Delivered plates win over anything else in the comp: a shot routinely + // contains pre-renders and CG passes of exactly the same length, and using + // one of those as the timecode reference is circular — it was rendered from + // this comp, so it can only ever agree with it, including when both are + // wrong. Only footage living under the configured Footage Root is camera + // material. Among equals, longest duration wins, as buildShotFromCode does + // when it picks primaryFootage. function pickTimingPlate(plates) { + var root = normalizeSlashes(trimText(CONFIG.footageRoot)).toLowerCase(); + var delivered = []; + var i; + + if (root) { + for (i = 0; i < plates.length; i += 1) { + if (normalizeSlashes(plates[i].path).toLowerCase().indexOf(root) === 0) { + delivered.push(plates[i]); + } + } + } + + return pickLongest(delivered.length ? delivered : plates); + } + + function pickLongest(plates) { var best = null; var i; @@ -3712,23 +3735,30 @@ var PANEL_VERSION = "2.2.0"; best = plates[i]; } } - return best; } - function toManifestPlate(plate, frameStart) { + function toManifestPlate(plate) { var ref = { element: plate.element, path: plate.path }; var parsed = parseFrameNumberFromPath(plate.path); - var start = parsed !== null ? parsed : frameStart; - ref.frameStart = start; - ref.frameEnd = start + Math.max(0, plate.frames - 1); + // Only ever the plate's *own* file numbering. When the name carries no + // frame number the field is left out entirely rather than filled with + // the comp's frame range — those are different coordinate systems, and + // a wrong number here silently skews the timecode comparison by the + // difference between them. + if (parsed !== null) { + ref.frameStart = parsed; + ref.frameEnd = parsed + Math.max(0, plate.frames - 1); + } return ref; } - // "UNG_102_019_010_BG01_TT_v001.01001.exr" -> 1001 + // "..._BG01_TT_v001.01001.exr" -> 1001, and "..._PRERENDER_01001.exr" -> 1001: + // renders written by AE separate the frame number with an underscore rather + // than a dot, and both turn up as layers in a shot comp. function parseFrameNumberFromPath(path) { - var match = String(path).match(/\.(\d{2,9})\.[A-Za-z0-9]+$/); + var match = String(path).match(/[._](\d{2,9})\.[A-Za-z0-9]+$/); if (!match) { return null; @@ -3791,12 +3821,12 @@ var PANEL_VERSION = "2.2.0"; timingPlate = pickTimingPlate(plates); if (timingPlate) { manifest.source = { - timingPlate: toManifestPlate(timingPlate, frameStart), + timingPlate: toManifestPlate(timingPlate), plates: [], handleFrames: CONFIG.handleFrames }; for (i = 0; i < plates.length; i += 1) { - manifest.source.plates.push(toManifestPlate(plates[i], frameStart)); + manifest.source.plates.push(toManifestPlate(plates[i])); } // Only claim a start source when this session actually resolved one // for this shot — an AEP opened from disk has no such history, and a @@ -3805,7 +3835,8 @@ var PANEL_VERSION = "2.2.0"; manifest.source.startSource = buildState.lastStartSource; manifest.source.startFrames = buildState.lastStartFrames; } - diagLog("manifest plates: " + plates.length + ", timing plate " + timingPlate.element); + diagLog("manifest plates: " + plates.length + ", timing plate " + + timingPlate.element + " (" + timingPlate.path + ")"); } else { diagLog("no file-backed footage found in " + comp.name + " — the render's timecode cannot be checked against a plate");