From 483db65eb34ffeeb9b4e8b1f9c43018bf92e8cf7 Mon Sep 17 00:00:00 2001 From: twotalesanimation <80506065+twotalesanimation@users.noreply.github.com> Date: Fri, 14 Aug 2026 13:10:10 +0200 Subject: [PATCH] fix(connector): build shots at the shot's frame rate, not a hardcoded 24 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shot.fps existed, the lookup API returned it, and one button honoured it — Set Start Timecode — while the shot builder ignored it entirely. Comps were created at 24 and every plate conformed to 24. DH2 is a 25 fps show. Its plates carry framesPerSecond 25/1, so comps built at 24 conform every plate to the wrong speed and compute every frame-from-timecode sum against the wrong rate. The builder now takes the rate from the shot record, which is the show's delivery rate and therefore the authority; a plate's own rate is whatever happens to be on disk. Import Renders follows the comp it imports into, and Import EXR follows the shot. Validation gains plate_fps, comparing the render against the timing plate. timecode_fps only compares the render to its manifest and both come from the comp, so a shot built at the wrong rate agrees with itself and passes; the plate is the independent witness. It is a WARN, never a FAIL: sampling DH2 found one plate at 50 fps against a 25 fps show, a deliberate over-crank, so a whole-number multiple is reported as exactly that and anything else as a probable rate mistake. Note this does not fix the data: all 175 DH2 shots still carry fps 24 from the create-time default and need correcting before their comps are rebuilt. Panel 2.4.0, worker 1.4.0. Co-Authored-By: Claude Opus 5 --- RenderPipeline2.md | 3 +- .../ValidationStageTests.cs | 68 +++++++++++++++++-- .../VFXReviewWorker/VFXReviewWorker.csproj | 2 +- .../VFXReviewWorker/ValidationStage.cs | 33 ++++++++- VFXReviewConnector.jsx | 56 ++++++++++----- VFXReviewConnector.md | 15 ++++ 6 files changed, 152 insertions(+), 25 deletions(-) diff --git a/RenderPipeline2.md b/RenderPipeline2.md index d862167..5d18ffa 100644 --- a/RenderPipeline2.md +++ b/RenderPipeline2.md @@ -1033,7 +1033,8 @@ Runs on the worker immediately after a successful render (Export = VALIDATING). | `colourspace` | EXR metadata (`chromaticities` / ACES container flag / colorspace attr written by AE OCIO) | ≠ `expected.colorspace` (working space is linear ACES2065-1 per 18.1-Q4); `colorPipeline` is required per project (18.1-Q12), so SKIPPED only on legacy projects predating pipeline enablement | | `timecode_plate_match` | **primary check — see §8.1a** | render `timeCode` ≠ the timing plate's `timeCode` at the corresponding frame | | `timecode_consistency` | EXR `timeCode` attr across sampled frames | non-monotonic (gap, repeat, or frozen value) | -| `timecode_fps` | EXR `framesPerSecond` rational + drop-frame flag on render vs timing plate vs manifest `fps` | any disagreement | +| `timecode_fps` | EXR `framesPerSecond` on the render vs manifest `fps` | any disagreement | +| `plate_fps` | EXR `framesPerSecond` on the render vs **the timing plate's** | WARN on any difference — never FAIL: a whole-number multiple is a legitimate over-crank (50 fps plate on a 25 fps show), and anything else is reported as a probable rate mistake | | `zero_byte / corrupt` | every file size > 0; oiiotool header-read on sampled frames (first, last, every Nth) | any zero-byte or unreadable file | 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`). diff --git a/RenderWorker/VFXReviewWorker.Tests/ValidationStageTests.cs b/RenderWorker/VFXReviewWorker.Tests/ValidationStageTests.cs index a9ac51f..473204d 100644 --- a/RenderWorker/VFXReviewWorker.Tests/ValidationStageTests.cs +++ b/RenderWorker/VFXReviewWorker.Tests/ValidationStageTests.cs @@ -26,14 +26,14 @@ public class ValidationStageTests : IDisposable } /// Writes a sequence numbered from 1001 with a plate-derived timecode. - private void WriteSequence(int count, string startTimecode, int firstFrame = 1001) + private void WriteSequence(int count, string startTimecode, int firstFrame = 1001, int fps = 24) { - var start = Timecode.Parse(startTimecode)!.Value.ToFrames(24); + var start = Timecode.Parse(startTimecode)!.Value.ToFrames(fps); for (var i = 0; i < count; i++) { - var tc = Timecode.FromFrames(start + i, 24); + var tc = Timecode.FromFrames(start + i, fps); 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); + File.Move(ExrFixture.Write(tc.ToString(), 3840, 2160, new[] { ("B", 1), ("G", 1), ("R", 1) }, fps, 4), path); } } @@ -109,6 +109,66 @@ public class ValidationStageTests : IDisposable Assert.Contains("8-frame handle", tc.Message); } + [Fact] + public void APlateAtADifferentRateFailsPlateFps() + { + // The DH2 case: a 25 fps show whose shot records defaulted to 24, so + // every comp was built at 24. The render agrees with its own manifest, + // so only the plate can reveal it. + WriteSequence(105, "14:26:31:08"); + 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) }, 25, 4), plate); + + var manifest = Manifest(_dir, "14:26:31:16", 1001); + manifest.Source = new ManifestSource { TimingPlate = new PlateRef { Path = plate, FrameStart = 1001 } }; + + var outcome = NewStage().Run(manifest, _dir, new ServerConfig()); + var fpsCheck = Check(outcome, "plate_fps"); + Assert.Equal("WARN", fpsCheck.Status); + Assert.Equal("25", fpsCheck.Expected); + Assert.Equal("24", fpsCheck.Actual); + Assert.Contains("wrong rate", fpsCheck.Message); + + // timecode_fps alone would not have caught it: the render and the + // manifest agree with each other at 24. + Assert.Equal("PASS", Check(outcome, "timecode_fps").Status); + } + + [Fact] + public void AnOverCrankedPlateIsRecognisedRatherThanReportedAsAMistake() + { + // A 50 fps plate on a 25 fps show is deliberate slow motion, conformed + // on purpose — real on DH2, and it must not read as a rate error. + WriteSequence(105, "14:26:31:08", fps: 25); + var plate = Path.Combine(_dir, "plate", "DH2_PL01.01001.exr"); + Directory.CreateDirectory(Path.GetDirectoryName(plate)!); + File.Move(ExrFixture.Write("14:26:31:08", 3840, 2160, new[] { ("B", 1), ("G", 1), ("R", 1) }, 50, 4), plate); + + var manifest = Manifest(_dir, null, 1001); + manifest.Fps = 25; + manifest.Source = new ManifestSource { TimingPlate = new PlateRef { Path = plate, FrameStart = 1001 } }; + + var check = Check(NewStage().Run(manifest, _dir, new ServerConfig()), "plate_fps"); + Assert.Equal("WARN", check.Status); + Assert.Contains("over-crank", check.Message); + Assert.DoesNotContain("wrong rate", check.Message); + } + + [Fact] + public void MatchingRatesPassPlateFps() + { + WriteSequence(105, "14:26:31:08"); + 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 } }; + + Assert.Equal("PASS", Check(NewStage().Run(manifest, _dir, new ServerConfig()), "plate_fps").Status); + } + [Fact] public void GapsAreStillCaughtWithinTheRun() { diff --git a/RenderWorker/VFXReviewWorker/VFXReviewWorker.csproj b/RenderWorker/VFXReviewWorker/VFXReviewWorker.csproj index 05120df..f10fbce 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.3.0 + 1.4.0 diff --git a/RenderWorker/VFXReviewWorker/ValidationStage.cs b/RenderWorker/VFXReviewWorker/ValidationStage.cs index 2e41db2..24f1f33 100644 --- a/RenderWorker/VFXReviewWorker/ValidationStage.cs +++ b/RenderWorker/VFXReviewWorker/ValidationStage.cs @@ -350,6 +350,31 @@ public sealed class ValidationStage return; } + // plate_fps — the render's rate against the plate's own. + // + // timecode_fps only compares the render to the manifest, and both come + // from the comp: a shot built at the wrong rate agrees with itself and + // passes. The plate is the independent witness. A 25 fps show whose + // shot records defaulted to 24 renders every shot at 24, and this is + // what names that rather than leaving it to surface as an off-by-N + // timecode error. + if (reference.PlateFps is double plateFps && first.H.FramesPerSecond is double renderFps) + { + var ratio = renderFps > 0 ? plateFps / renderFps : 0; + var overcrank = ratio > 1.5 && Math.Abs(ratio - Math.Round(ratio)) < 0.01; + + checks.Add(Math.Abs(plateFps - renderFps) < 0.01 + ? ValidationCheck.Pass("plate_fps", plateFps.ToString("0.###"), renderFps.ToString("0.###")) + // Never a FAIL: a plate deliberately shot at a multiple of the + // show rate is a legitimate over-crank, conformed on purpose. + // Anything else is a rate mistake, and the wording says which. + : ValidationCheck.Warn("plate_fps", plateFps.ToString("0.###"), renderFps.ToString("0.###"), + overcrank + ? $"The plate is {plateFps:0.###} fps against a {renderFps:0.###} fps render — a {Math.Round(ratio)}x over-crank, conformed as expected." + : $"The plate is {plateFps:0.###} fps but this rendered at {renderFps:0.###} — not a whole-number over-crank, so the comp was probably built at the wrong rate (check the shot's fps in the database).", + new { platePath = reference.PlatePath, reference = reference.Kind, ratio })); + } + var expectedFrames = reference.PlateTimecode.ToFrames(fps) + (renderFirstFrame - reference.PlateFirstFrame); var expected = Timecode.FromFrames(expectedFrames, fps); var actualFrames = renderTc.ToFrames(fps); @@ -415,7 +440,8 @@ public sealed class ValidationStage var frame = SequencePattern.FrameFromName(Path.GetFileName(local)) ?? timing.FrameStart ?? renderFirstFrame; - return new TimecodeReference("manifest", local, header.Timecode.Value, frame, handleFrames); + return new TimecodeReference("manifest", local, header.Timecode.Value, frame, handleFrames, + header.FramesPerSecond); } _log.LogWarning("Timing plate from manifest is unreadable: {Path}", local); } @@ -507,7 +533,7 @@ public sealed class ValidationStage if (header?.Timecode is null) return null; var frame = SequencePattern.FrameFromName(Path.GetFileName(firstFile)) ?? m.FrameStart; - return new TimecodeReference("derived", firstFile, header.Timecode.Value, frame, 0); + return new TimecodeReference("derived", firstFile, header.Timecode.Value, frame, 0, header.FramesPerSecond); } private static void AddCandidate(List<(string Dir, int Count)> candidates, string dir) @@ -554,7 +580,8 @@ public sealed class ValidationStage } public sealed record TimecodeReference( - string Kind, string? PlatePath, Timecode PlateTimecode, int PlateFirstFrame, int HandleFrames); + string Kind, string? PlatePath, Timecode PlateTimecode, int PlateFirstFrame, int HandleFrames, + double? PlateFps = null); public sealed record ValidationOutcome( List Checks, int FileCount, long TotalBytes, string? Checksum) diff --git a/VFXReviewConnector.jsx b/VFXReviewConnector.jsx index 9a7e420..5ebc175 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.3.0"; +var PANEL_VERSION = "2.4.0"; (function VFXReviewConnector(thisObj) { var uiState = { @@ -1218,7 +1218,7 @@ var PANEL_VERSION = "2.3.0"; return matches; } - function importEXRSequence(folder) { + function importEXRSequence(folder, fps) { var files = folder.getFiles("*.exr"); var importOptions; var footage; @@ -1231,10 +1231,28 @@ var PANEL_VERSION = "2.3.0"; importOptions = new ImportOptions(files[0]); importOptions.sequence = true; footage = app.project.importFile(importOptions); - footage.mainSource.conformFrameRate = 24; + // Conform to the show's rate, not a hardcoded 24: conforming a 25 fps + // plate to 24 changes its duration and every frame-from-timecode sum + // that follows. + footage.mainSource.conformFrameRate = resolveFps(fps); return footage; } + /** + * The frame rate to build at. The shot record is the show's delivery rate + * and therefore the authority; a plate's own rate is what happens to be on + * disk, and where they differ the plate is the thing that is wrong. + */ + function resolveFps(fps) { + var value = Number(fps); + + return (value && value > 0) ? value : 24; + } + + function getShotFps(shot) { + return resolveFps(shot && shot.fps); + } + function addOCIOToLayer(layer) { var ocio; var outputProp; @@ -1751,6 +1769,7 @@ var PANEL_VERSION = "2.3.0"; var folderNames; var emptyFolderNotes = []; var importFailure; + var fps = 24; diagLog("build: shot=" + shotCode + " group=" + (groupCode || "(none)") + " episodic=" + isEpisodicProject()); @@ -1769,9 +1788,10 @@ var PANEL_VERSION = "2.3.0"; ["Looked up under project " + CURRENT_PROJECT.code + " — a shot belonging to another project will not be found."])); return null; } + fps = getShotFps(apiData.shot); diagLog("shot found: version " + (apiData.shot.shotVersion || "?") + ", episode " + (apiData.shot.episode || "-") + ", scene " + (apiData.shot.scene || "-") + - ", tc " + (apiData.shot.timecodeStart || "-")); + ", tc " + (apiData.shot.timecodeStart || "-") + ", " + fps + " fps"); if (existingShotComp) { diagLog("comp " + shotCode + " already exists — refreshing its overlay only"); @@ -1817,7 +1837,7 @@ var PANEL_VERSION = "2.3.0"; footage = null; importFailure = ""; try { - footage = importEXRSequence(sequenceFolders[i]); + footage = importEXRSequence(sequenceFolders[i], fps); } catch (importError) { importFailure = describeError(importError); diagLog("import threw for " + sequenceFolders[i].fsName + ": " + importFailure); @@ -1825,7 +1845,7 @@ var PANEL_VERSION = "2.3.0"; if (footage) { diagLog("imported " + sequenceFolders[i].name + " (" + footage.width + "x" + footage.height + - ", " + Math.round(footage.duration * 24) + " frames)"); + ", " + Math.round(footage.duration * fps) + " frames at " + fps + " fps)"); footage.name = sequenceFolders[i].name; if (footageFolder) { footage.parentFolder = footageFolder; @@ -1856,13 +1876,13 @@ var PANEL_VERSION = "2.3.0"; try { // Both comps start at the plate's native start timecode (as AE would // when creating a comp from the footage), not at frame 0. - startInfo = getFootageStartFrames(primaryFootage, apiData.shot, 24); + startInfo = getFootageStartFrames(primaryFootage, apiData.shot, fps); buildState.lastStartSource = startInfo.source; buildState.lastStartFrames = startInfo.frames; buildState.lastStartShotCode = shotCode; diagLog("start frame " + startInfo.frames + " from " + startInfo.source); - footageComp = app.project.items.addComp(shotCode + "_FOOTAGE", width, height, 1, maxDuration, 24); + footageComp = app.project.items.addComp(shotCode + "_FOOTAGE", width, height, 1, maxDuration, fps); setCompStartFrame(footageComp, startInfo.frames); if (precompFolder) { footageComp.parentFolder = precompFolder; @@ -1873,7 +1893,7 @@ var PANEL_VERSION = "2.3.0"; addOCIOToLayer(footageLayer); } - shotComp = app.project.items.addComp(shotCode, width, height, 1, maxDuration, 24); + shotComp = app.project.items.addComp(shotCode, width, height, 1, maxDuration, fps); setCompStartFrame(shotComp, startInfo.frames); if (episodeProjectFolder) { shotComp.parentFolder = episodeProjectFolder; @@ -1900,8 +1920,8 @@ var PANEL_VERSION = "2.3.0"; if (!updateOverlayForComp(shotComp, shotCode, apiData)) { diagLog("overlay not applied (non-fatal): UNG_VFX_OVERLAY comp missing, or its DATE/SHOT NAME Essential Properties are not exposed"); } - diagLog("built comp " + shotComp.name + " (" + width + "x" + height + ", " + - Math.round(maxDuration * 24) + " frames, " + footageItems.length + " plate layer(s))"); + diagLog("built comp " + shotComp.name + " (" + width + "x" + height + " @ " + fps + " fps, " + + Math.round(maxDuration * fps) + " frames, " + footageItems.length + " plate layer(s))"); return shotComp; } @@ -2380,7 +2400,7 @@ var PANEL_VERSION = "2.3.0"; importOptions = new ImportOptions(files[0]); importOptions.sequence = true; footage = app.project.importFile(importOptions); - footage.mainSource.conformFrameRate = 24; + footage.mainSource.conformFrameRate = getShotFps(shot); } catch (importErr) { updateStatus(comps.length, "Import failed for " + shot.shotCode + ": " + importErr.message); skipped += 1; @@ -2416,6 +2436,7 @@ var PANEL_VERSION = "2.3.0"; var footage; var layer; var i; + var renderFps; if (comps.length < 1) { updateStatus(0, "Please select or open a comp first."); @@ -2424,6 +2445,9 @@ var PANEL_VERSION = "2.3.0"; comp = comps[0]; shotCode = getShotCodeFromComp(comp); + // The comp being imported into is already at the show's rate; conforming + // renders to anything else would resample them. + renderFps = resolveFps(comp.frameRate); rendersRoot = new Folder("X:/shared_projects_2026/UNGO_VFX/production/renders/" + shotCode); if (!rendersRoot.exists) { @@ -2439,10 +2463,10 @@ var PANEL_VERSION = "2.3.0"; subFolders = rendersRoot.getFiles(); for (i = 0; i < subFolders.length; i += 1) { if (subFolders[i] instanceof Folder) { - footage = importEXRSequence(subFolders[i]); + footage = importEXRSequence(subFolders[i], renderFps); if (footage) { footage.name = subFolders[i].name; - footage.mainSource.conformFrameRate = 24; + footage.mainSource.conformFrameRate = renderFps; if (rendersProjectFolder) { footage.parentFolder = rendersProjectFolder; } @@ -2453,10 +2477,10 @@ var PANEL_VERSION = "2.3.0"; // Fall back to sequences directly in the root renders folder if (importedFootage.length < 1) { - footage = importEXRSequence(rendersRoot); + footage = importEXRSequence(rendersRoot, renderFps); if (footage) { footage.name = shotCode + "_render"; - footage.mainSource.conformFrameRate = 24; + footage.mainSource.conformFrameRate = renderFps; if (rendersProjectFolder) { footage.parentFolder = rendersProjectFolder; } diff --git a/VFXReviewConnector.md b/VFXReviewConnector.md index 93ab933..2bc7472 100644 --- a/VFXReviewConnector.md +++ b/VFXReviewConnector.md @@ -103,6 +103,21 @@ Source EXR plates are read from **`/{SHOW_ID}/`**. Configure `foot └── {EPISODE_CODE}_*.mov # matched by episode prefix ``` +### Frame rate + +Comps are built at the **shot record's `fps`**, and plates are conformed to it. +That value is the show's delivery rate and the authority; a plate's own rate is +whatever happens to be on disk. + +`fps` defaults to 24 when a shot is created, so **check it after importing a +show that is not 24** — every shot carries its own value, and a wrong one +produces comps at the wrong rate, plates conformed to the wrong speed, and +timecode-to-frame sums that are quietly off. + +A plate at a whole-number multiple of the show rate (50 fps on a 25 fps show) is +a deliberate over-crank and is conformed as intended. Validation reports the +difference either way, saying which of the two it looks like. + **Scenes are not folders.** Standard projects have no level between the show folder and the shot folder — the scene picker filters the shot list only.