fix(validation): separate output file numbering from comp frame numbers
Deploy / deploy (push) Successful in 6m13s

A render carries frame numbers in two unrelated systems. The comp is
anchored to its plate's timecode, so its timeline runs 1247792-1247896;
the output module names files from 1001. Validation compared the file
numbers against the comp range and failed every correctly-rendered shot
by ~1.2 million frames — first_frame, last_frame and missing_frames all
reporting a defect that was not there.

The manifest now carries outputFrameStart (the output module's starting
number, house convention 1001) alongside the comp range. Count and
contiguity are measured on the output numbering; first/last compare
against outputFrameStart and are SKIPPED on older manifests, where the
count and gap checks still hold on their own.

The same conflation broke the API timecode fallback: it anchored on the
comp's frameStart while the render's first frame is 1001, producing a
+1246791 frame "offset". Shot.timecodeStart is the edit in-point and the
plate carries handleFrames of head handle, so the reference is the API
value less handles, anchored on the render's own first frame.

Verified on the shot that reported it: UNG_107_051_010 v004 now passes
count, contiguity and a plate-anchored timecode match. Regression tests
cover both coordinate systems, including that a genuine +8 handle offset
still fails.

Panel 2.1.2.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
twotalesanimation
2026-08-13 23:26:39 +02:00
parent 4929db45a2
commit 37f73a2bd7
6 changed files with 222 additions and 17 deletions
+9
View File
@@ -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.
@@ -0,0 +1,134 @@
using Microsoft.Extensions.Logging.Abstractions;
using VFXReviewWorker;
using Xunit;
namespace VFXReviewWorker.Tests;
/// <summary>
/// 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.
/// </summary>
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 { }
}
/// <summary>Writes a sequence numbered from 1001 with a plate-derived timecode.</summary>
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<ValidationStage>.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);
}
}
+8
View File
@@ -163,8 +163,16 @@ public sealed class RenderManifest
public string OutputPattern { get; set; } = "";
public string? OutputModuleTemplate { get; set; }
public string? RenderSettingsTemplate { get; set; }
/// <summary>Comp timeline range handed to aerender (-s/-e).</summary>
public int FrameStart { get; set; }
public int FrameEnd { get; set; }
/// <summary>
/// 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.
/// </summary>
public int? OutputFrameStart { get; set; }
public double Fps { get; set; } = 24;
public int Width { get; set; }
public int Height { get; set; }
+47 -15
View File
@@ -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<int>();
var duplicates = new List<int>();
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<int>();
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<string>();
@@ -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).
/// </summary>
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;
+16 -2
View File
@@ -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,
+8
View File
@@ -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(),