diff --git a/RenderPipeline2.md b/RenderPipeline2.md index 207c098..36e42db 100644 --- a/RenderPipeline2.md +++ b/RenderPipeline2.md @@ -1037,6 +1037,15 @@ Runs on the worker immediately after a successful render (Export = VALIDATING). Full-sequence checks (count, gaps, names, sizes) run on every file; header checks sample first/middle/last + every 25th frame (fast, catches systemic issues; per-frame full decode is not worth minutes per shot — tunable via `SystemConfig: validation.sampleEvery`). +**Two coordinate systems, never conflated.** A render carries frame numbers in two unrelated systems, and every check states which one it uses: + +| | Meaning | Example | +|---|---|---| +| `frameStart`/`frameEnd` | The **comp's timeline**, handed to aerender as `-s`/`-e`. A comp anchored to its plate's timecode starts at the absolute frame that timecode represents. | `1247792`–`1247896` | +| `outputFrameStart` | The number the **first output file** carries, set by the output module template. House convention: every sequence starts at 1001. | `1001` | + +`file_count`, `missing_frames` and `duplicate_frames` are measured on the *output* numbering; `first_frame`/`last_frame` compare against `outputFrameStart` and are SKIPPED when a manifest predates the field, since the count and contiguity checks still hold. Comparing file numbers against comp frames fails every correctly-rendered shot by ~1.2 million frames, and the timecode reference must be anchored on the render's own first frame for the same reason. + ### 8.1a Timecode: the primary check (studio decision) Timecode is the check this engine exists for. It is validated **against the source plate**, never against the API, because the API and the plate do not agree by design. diff --git a/RenderWorker/VFXReviewWorker.Tests/ValidationStageTests.cs b/RenderWorker/VFXReviewWorker.Tests/ValidationStageTests.cs new file mode 100644 index 0000000..a9ac51f --- /dev/null +++ b/RenderWorker/VFXReviewWorker.Tests/ValidationStageTests.cs @@ -0,0 +1,134 @@ +using Microsoft.Extensions.Logging.Abstractions; +using VFXReviewWorker; +using Xunit; + +namespace VFXReviewWorker.Tests; + +/// +/// Regression cover for the two coordinate systems a render lives in, taken +/// from a real failure on UNG_107_051_010 v004: +/// +/// comp frames 1247792-1247896 (anchored to the plate's timecode 14:26:31:08) +/// file numbers 1001-1105 (the output module's starting number) +/// +/// Checking file numbers against comp frames failed every render, and using +/// the comp range as the timecode anchor produced a million-frame offset. +/// +public class ValidationStageTests : IDisposable +{ + private readonly string _dir = Path.Combine(Path.GetTempPath(), $"vfxr_val_{Guid.NewGuid():N}"); + + public ValidationStageTests() => Directory.CreateDirectory(_dir); + + public void Dispose() + { + try { Directory.Delete(_dir, recursive: true); } catch { } + } + + /// Writes a sequence numbered from 1001 with a plate-derived timecode. + private void WriteSequence(int count, string startTimecode, int firstFrame = 1001) + { + var start = Timecode.Parse(startTimecode)!.Value.ToFrames(24); + for (var i = 0; i < count; i++) + { + var tc = Timecode.FromFrames(start + i, 24); + var path = Path.Combine(_dir, $"UNG_107_051_010_cmp_TT_v004.{firstFrame + i:D5}.exr"); + File.Move(ExrFixture.Write(tc.ToString(), 3840, 2160, new[] { ("B", 1), ("G", 1), ("R", 1) }, 24, 4), path); + } + } + + private static RenderManifest Manifest(string dir, string? apiTimecode, int? outputFrameStart) => new() + { + ShotCode = "UNG_107_051_010", + OutputDir = dir, + OutputPattern = "UNG_107_051_010_cmp_TT_v004.[#####].exr", + // The comp is anchored to the plate timecode, so its frames are absolute. + FrameStart = 1247792, + FrameEnd = 1247896, + OutputFrameStart = outputFrameStart, + Fps = 24, + Width = 3840, + Height = 2160, + Expected = apiTimecode is null ? null : new ManifestExpected { TimecodeStart = apiTimecode }, + }; + + private ValidationStage NewStage() => + new(new WorkerOptions { ServerUrl = "n/a", ApiKey = "n/a" }, NullLogger.Instance); + + private static ValidationCheck Check(ValidationOutcome o, string name) => + o.Checks.Single(c => c.CheckName == name); + + [Fact] + public void FileNumberingIsCheckedAgainstOutputStartNotCompFrames() + { + WriteSequence(105, "14:26:31:08"); + var outcome = NewStage().Run(Manifest(_dir, "14:26:31:16", 1001), _dir, new ServerConfig()); + + Assert.Equal("PASS", Check(outcome, "file_count").Status); + Assert.Equal("PASS", Check(outcome, "first_frame").Status); + Assert.Equal("PASS", Check(outcome, "last_frame").Status); + Assert.Equal("PASS", Check(outcome, "missing_frames").Status); + Assert.Equal("1105", Check(outcome, "last_frame").Actual); + } + + [Fact] + public void ApiTimecodeIsAnchoredOnTheRenderNotTheCompRange() + { + // Shot.timecodeStart is the edit in-point; the plate carries 8 frames of + // head handle, so the first rendered frame is 8 frames earlier. + WriteSequence(105, "14:26:31:08"); + var outcome = NewStage().Run(Manifest(_dir, "14:26:31:16", 1001), _dir, new ServerConfig()); + + var tc = Check(outcome, "timecode_plate_match"); + // WARN, never PASS: the API is a fallback reference (§8.1a). + Assert.Equal("WARN", tc.Status); + Assert.Equal("14:26:31:08", tc.Expected); + Assert.Equal("14:26:31:08", tc.Actual); + Assert.DoesNotContain("frames)", tc.Message ?? ""); + } + + [Fact] + public void RealTimecodeDefectStillFailsAgainstAPlate() + { + // Same shot, but rendered 8 frames late — the handle-offset defect. + WriteSequence(105, "14:26:31:16"); + var plate = Path.Combine(_dir, "plate", "UNG_107_051_010_BG01_TT_v001.01001.exr"); + Directory.CreateDirectory(Path.GetDirectoryName(plate)!); + File.Move(ExrFixture.Write("14:26:31:08", 3840, 2160, new[] { ("B", 1), ("G", 1), ("R", 1) }, 24, 4), plate); + + var manifest = Manifest(_dir, "14:26:31:16", 1001); + manifest.Source = new ManifestSource + { + TimingPlate = new PlateRef { Path = plate, FrameStart = 1001 }, + HandleFrames = 8, + }; + + var tc = Check(NewStage().Run(manifest, _dir, new ServerConfig()), "timecode_plate_match"); + Assert.Equal("FAIL", tc.Status); + Assert.Contains("+8 frames", tc.Message); + Assert.Contains("8-frame handle", tc.Message); + } + + [Fact] + public void GapsAreStillCaughtWithinTheRun() + { + WriteSequence(105, "14:26:31:08"); + File.Delete(Path.Combine(_dir, "UNG_107_051_010_cmp_TT_v004.01050.exr")); + + var outcome = NewStage().Run(Manifest(_dir, "14:26:31:16", 1001), _dir, new ServerConfig()); + Assert.Equal("FAIL", Check(outcome, "file_count").Status); + Assert.Equal("FAIL", Check(outcome, "missing_frames").Status); + Assert.Contains("1050", Check(outcome, "missing_frames").Message); + } + + [Fact] + public void OldManifestsWithoutOutputNumberingSkipRatherThanFail() + { + WriteSequence(105, "14:26:31:08"); + var outcome = NewStage().Run(Manifest(_dir, "14:26:31:16", null), _dir, new ServerConfig()); + + Assert.Equal("SKIPPED", Check(outcome, "first_frame").Status); + Assert.Equal("PASS", Check(outcome, "missing_frames").Status); + Assert.Contains("1001-1105", Check(outcome, "first_frame").Message); + } +} diff --git a/RenderWorker/VFXReviewWorker/Dtos.cs b/RenderWorker/VFXReviewWorker/Dtos.cs index 100bb4c..04d9337 100644 --- a/RenderWorker/VFXReviewWorker/Dtos.cs +++ b/RenderWorker/VFXReviewWorker/Dtos.cs @@ -163,8 +163,16 @@ public sealed class RenderManifest public string OutputPattern { get; set; } = ""; public string? OutputModuleTemplate { get; set; } public string? RenderSettingsTemplate { get; set; } + /// Comp timeline range handed to aerender (-s/-e). public int FrameStart { get; set; } public int FrameEnd { get; set; } + /// + /// The number the *first output file* carries — the output module's + /// starting number (house convention 1001), which is deliberately not the + /// comp's frame range: a comp anchored to the plate's timecode starts in + /// the millions. Null on manifests predating this field. + /// + public int? OutputFrameStart { get; set; } public double Fps { get; set; } = 24; public int Width { get; set; } public int Height { get; set; } diff --git a/RenderWorker/VFXReviewWorker/ValidationStage.cs b/RenderWorker/VFXReviewWorker/ValidationStage.cs index 9ef16a9..f124177 100644 --- a/RenderWorker/VFXReviewWorker/ValidationStage.cs +++ b/RenderWorker/VFXReviewWorker/ValidationStage.cs @@ -102,15 +102,28 @@ public sealed class ValidationStage if (files.Count == 0) return files; - // missing_frames / duplicate_frames / first_frame / last_frame + // Output files are numbered by the output module's starting number + // (house convention: every sequence starts at 1001), which is a + // different coordinate system from the comp's frame range — a comp + // anchored to the plate's timecode starts at 1247792. Checking file + // numbers against comp frames compares the two and fails every render, + // so the sequence is checked for the right *count* and *contiguity*, + // and its absolute start only where the manifest states it. + var expectedFirst = manifest.OutputFrameStart; + var seen = new HashSet(); var duplicates = new List(); foreach (var f in frameNumbers) { if (!seen.Add(f)) duplicates.Add(f); } + + // Gaps are found relative to the run the render actually produced: a + // hole in the middle is a real defect regardless of where it starts. + var runStart = expectedFirst ?? frameNumbers[0]; + var runEnd = runStart + expectedCount - 1; var missing = new List(); - for (var f = manifest.FrameStart; f <= manifest.FrameEnd; f++) + for (var f = runStart; f <= runEnd; f++) { if (!seen.Contains(f)) missing.Add(f); } @@ -118,7 +131,8 @@ public sealed class ValidationStage checks.Add(missing.Count == 0 ? ValidationCheck.Pass("missing_frames", "none", "none") : ValidationCheck.Fail("missing_frames", "none", $"{missing.Count} missing", - "Missing frames: " + Summarise(missing), new { missing = missing.Take(500) })); + $"Missing frames in {runStart}-{runEnd}: " + Summarise(missing), + new { missing = missing.Take(500), runStart, runEnd })); checks.Add(duplicates.Count == 0 ? ValidationCheck.Pass("duplicate_frames", "none", "none") @@ -126,16 +140,27 @@ public sealed class ValidationStage "Duplicate frame numbers (differing padding?): " + Summarise(duplicates), new { duplicates = duplicates.Take(100) })); - checks.Add(frameNumbers[0] == manifest.FrameStart - ? ValidationCheck.Pass("first_frame", manifest.FrameStart.ToString(), frameNumbers[0].ToString()) - : ValidationCheck.Fail("first_frame", manifest.FrameStart.ToString(), frameNumbers[0].ToString(), - $"Sequence starts at {frameNumbers[0]}, expected {manifest.FrameStart}")); + if (expectedFirst is int first) + { + checks.Add(frameNumbers[0] == first + ? ValidationCheck.Pass("first_frame", first.ToString(), frameNumbers[0].ToString()) + : ValidationCheck.Fail("first_frame", first.ToString(), frameNumbers[0].ToString(), + $"Sequence starts at {frameNumbers[0]}, expected {first}")); - var last = frameNumbers[^1]; - checks.Add(last == manifest.FrameEnd - ? ValidationCheck.Pass("last_frame", manifest.FrameEnd.ToString(), last.ToString()) - : ValidationCheck.Fail("last_frame", manifest.FrameEnd.ToString(), last.ToString(), - $"Sequence ends at {last}, expected {manifest.FrameEnd}")); + var expectedLast = first + expectedCount - 1; + checks.Add(frameNumbers[^1] == expectedLast + ? ValidationCheck.Pass("last_frame", expectedLast.ToString(), frameNumbers[^1].ToString()) + : ValidationCheck.Fail("last_frame", expectedLast.ToString(), frameNumbers[^1].ToString(), + $"Sequence ends at {frameNumbers[^1]}, expected {expectedLast}")); + } + else + { + // Old manifests carry no output numbering; the count and contiguity + // checks above still hold, so the run is reported rather than + // measured against a number that was never sent. + checks.Add(ValidationCheck.Skipped("first_frame", + $"manifest carries no output frame numbering — sequence runs {frameNumbers[0]}-{frameNumbers[^1]}")); + } // zero_byte — every file, it is only a directory stat var zeroByte = new List(); @@ -315,7 +340,7 @@ public sealed class ValidationStage } // timecode_plate_match — reference precedence, rank 1 → 3. - var reference = ResolveTimecodeReference(m, config); + var reference = ResolveTimecodeReference(m, config, renderFirstFrame); if (reference is null) { checks.Add(ValidationCheck.Warn("timecode_plate_match", "plate timecode", renderTc.ToString(), @@ -371,7 +396,7 @@ public sealed class ValidationStage /// Reference precedence (§8.1a): manifest timing plate → plate found under a /// configured search root → API timecode minus handles (advisory only). /// - private TimecodeReference? ResolveTimecodeReference(RenderManifest m, ServerConfig config) + private TimecodeReference? ResolveTimecodeReference(RenderManifest m, ServerConfig config, int renderFirstFrame) { var mapper = new PathMapper(_options.PathMappings); var handleFrames = m.Source?.HandleFrames ?? config.ValidationHandleFrames; @@ -395,12 +420,19 @@ public sealed class ValidationStage if (derived is not null) return derived with { HandleFrames = handleFrames }; // Rank 3 — the API's editorial in-point, less handles. Advisory only. + // + // Shot.timecodeStart is the *edit* in-point; the plate is delivered with + // handleFrames of head handle, so the first delivered frame sits that + // much earlier. The anchor is the render's own first frame, which is in + // the same numbering as the files on disk — not the manifest's frame + // range, which is the comp's timeline and a different coordinate system + // entirely (mixing the two produced a nonsense million-frame offset). var apiTc = Timecode.Parse(m.Expected?.TimecodeStart); if (apiTc is not null) { var fps = m.Fps > 0 ? m.Fps : 24; var plateStart = Timecode.FromFrames(apiTc.Value.ToFrames(fps) - handleFrames, fps); - return new TimecodeReference("api", null, plateStart, m.FrameStart, handleFrames); + return new TimecodeReference("api", null, plateStart, renderFirstFrame, handleFrames); } return null; diff --git a/VFXReviewConnector.jsx b/VFXReviewConnector.jsx index 1589773..31392fb 100644 --- a/VFXReviewConnector.jsx +++ b/VFXReviewConnector.jsx @@ -18,7 +18,12 @@ var CONFIG = { // Format: "//SAN/=V:/_EXPORTS/" — several separated by ";". // Usually unnecessary: see resolveServerPath, which derives the local path // from Export Root when the layouts mirror each other. - pathMappings: "" + pathMappings: "", + // The number the first frame of every rendered sequence carries, set by the + // output module template. House convention is 1001, and it is deliberately + // independent of the comp's start frame — comps are anchored to the plate's + // timecode, so their frame numbers run into the millions. + sequenceStart: 1001 }; var SETTINGS_GROUP = "VFXReviewConnector"; @@ -27,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.1.1"; +var PANEL_VERSION = "2.1.2"; (function VFXReviewConnector(thisObj) { var uiState = { @@ -3077,6 +3082,11 @@ var PANEL_VERSION = "2.1.1"; var fBanner = addField(dlg, "Banner Path:", CONFIG.bannerImagePath); var fEmail = addField(dlg, "Artist Email:", CONFIG.artistEmail); var fHandles= addField(dlg, "Handle Frames:", String(CONFIG.handleFrames)); + var fSeqStart = addField(dlg, "Sequence Start:", String(CONFIG.sequenceStart)); + fSeqStart.helpTip = + "The frame number the first file of every rendered sequence carries,\n" + + "set by the output module template (house convention: 1001).\n" + + "Not the comp's start frame — comps are anchored to the plate's timecode."; var fMappings = addField(dlg, "Path Mappings:", CONFIG.pathMappings); fMappings.helpTip = "Optional. Rewrites paths the server supplies (render folders are\n" + @@ -3104,6 +3114,7 @@ var PANEL_VERSION = "2.1.1"; CONFIG.artistEmail = trimText(fEmail.text); CONFIG.handleFrames = parseInt(fHandles.text, 10) || 8; CONFIG.pathMappings = trimText(fMappings.text); + CONFIG.sequenceStart = parseInt(fSeqStart.text, 10) || 1001; saveConfig(); populateProjectDropdown(); } @@ -3730,6 +3741,9 @@ var PANEL_VERSION = "2.1.1"; outputModuleTemplate: "EXR Sequence", frameStart: frameStart, frameEnd: frameEnd, + // What the rendered files are actually named, which the comp's frame + // range is not: the comp is anchored to the plate's timecode. + outputFrameStart: parseInt(CONFIG.sequenceStart, 10) || 1001, fps: Math.round(comp.frameRate * 1000) / 1000, width: comp.width, height: comp.height, diff --git a/lib/render-pipeline/manifest.ts b/lib/render-pipeline/manifest.ts index dcfe141..8e40073 100644 --- a/lib/render-pipeline/manifest.ts +++ b/lib/render-pipeline/manifest.ts @@ -37,8 +37,16 @@ export const renderManifestSchema = z outputPattern: z.string().min(1), outputModuleTemplate: z.string().optional(), renderSettingsTemplate: z.string().optional(), + /** Comp timeline range handed to aerender. */ frameStart: z.number().int(), frameEnd: z.number().int(), + /** + * The number the first *output file* carries — the output module's starting + * number (house convention 1001). Deliberately separate from frameStart: a + * comp anchored to its plate's timecode starts in the millions, so file + * numbering and comp frames are different coordinate systems. + */ + outputFrameStart: z.number().int().optional(), fps: z.number().positive(), width: z.number().int().positive(), height: z.number().int().positive(),