From 59a2927a1552347413f8df969450eb4bfc1d08d8 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Thu, 6 Aug 2026 19:44:44 +0200 Subject: [PATCH] fix(AR-004): derive the scene join depth instead of pinning it at 256 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `kSceneJoinDepth` was a constant 256, used for two different channels. Both uses were wrong, in different ways. **It is a span of film, not a count of slots.** The join needs the sampled branch to trail the dense one by however long TransNetV2 takes to be able to answer: its input queue (128) plus the window it must fill (100), over native frame rate — about 9.5 s at 24 fps, which is the conservative floor since a slower source makes the same frame count span more film. The slots needed to hold that depend on `sample_fps`, so the constant meant ~256 s of lag at 1 fps — 27x what the join needs — and nothing recomputed it when `sample_fps` changed. The one number AR-010's correctness rests on drifted with an unrelated knob. It is now derived, giving a constant 19 s of lag at any sample rate: 19 slots at 1 fps, 95 at 5. **The decimator's buffer was on the wrong side of the decimator.** Its *input* carries the full-rate stream, so 256 slots held ~256 full decoded frames of which, at 1 fps against 24 fps native, 23 in every 24 existed only to be discarded by the predicate a moment later. Holding ~1.5 GB of images for frames the very next node throws away is the worst available use of the budget. A filter is a pass-through, not a reservoir: the lag belongs after decimation, where a slot buys 1/sample_fps seconds instead of 1/native_fps. Its input is now sized only to keep it fed. Together, at 1080p and 1 fps, those two channels go from ~3.0 GB to ~206 MB while the join keeps 2x margin over its requirement. Every message embeds `Frame source`, so a slot on either branch holds a full decoded image — which is visible now that the byte counter is honest (5e46f52), and was not before. **Not verified against a running pipeline.** The check that matters is `SceneBoundaries::outran()` — sampled frames that arrived before the detector had scored them, which must stay 0 — and that needs a real clip through `--scene-detect`. I could not launch it here. The arithmetic is stated above so it can be checked by inspection, and the margin is deliberately 2x rather than tight, but a run should confirm outran() is still 0 before this is trusted on real content. TRACES: AR-004, AR-010 | SR-002 --- src/main.cpp | 75 +++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 66 insertions(+), 9 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 6303edc..7be3ef0 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -71,6 +71,7 @@ #include "nodes/face_tracker_node.hpp" #include "nodes/identity_matcher_node.hpp" #include "nodes/frame_annotation_node.hpp" +#include "inference/scene_detector.hpp" // ISceneDetector::kWindow, for the join-depth derivation #include "nodes/scene_detector_node.hpp" #include "scene_boundaries.hpp" #include "nodes/scene_boundary_annotator_node.hpp" @@ -84,8 +85,10 @@ #include // cv::setNumThreads (SAE_CV_THREADS) +#include #include #include +#include #include #include #include @@ -101,12 +104,66 @@ // ── CLI parsing ─────────────────────────────────────────────────────────────── /// TRACES: AR-010, AR-004 | SR-002 -/// How deeply the sampled branch is buffered behind the dense one. TransNetV2 -/// needs kWindow (100) dense frames before it can score any of them, so the face -/// branch must lag by at least that much or it asks about frames nobody has -/// looked at yet. Backpressure turns depth into lag: the fanout blocks on the -/// slower branch rather than dropping, so the detector simply runs ahead. -static constexpr std::size_t kSceneJoinDepth = 256; +/// Depth of the dense branch's own input queue. Part of how far behind the +/// fanout head TransNetV2 can be, and therefore an input to the join depth. +static constexpr std::size_t kSceneInputDepth = 128; + +/// TRACES: AR-010, AR-004 | SR-002 +/// How far the sampled branch must trail the dense one, in seconds of film. +/// +/// TransNetV2 needs kWindow (100) dense frames before it can score any of +/// them, and its input queue can hold kSceneInputDepth more, so in the worst +/// case it has scored only up to (kSceneInputDepth + kWindow) frames behind +/// whatever the fanout has just delivered. The face branch must be at least +/// that far behind, or `scene_annotate` asks about frames nobody has looked at +/// yet. Backpressure turns depth into lag: the fanout blocks on the slower +/// branch rather than dropping, so the dense branch simply runs ahead. +/// +/// Divided by a *lower bound* on native frame rate, because a slower source +/// makes the same frame count span more film — 24 fps is the floor for the +/// material this runs on, so it is the conservative choice. +static constexpr double kMinNativeFps = 24.0; +static constexpr double kSceneJoinLagSec = + (kSceneInputDepth + ISceneDetector::kWindow) / kMinNativeFps; // ~9.5 s + +/// Margin over that minimum, for jitter in TransNetV2's inference time. +static constexpr double kSceneJoinSafety = 2.0; + +/// TRACES: AR-004 | SR-002 +/// Slots the sampled branch needs to hold `kSceneJoinLagSec` of film. +/// +/// This used to be a constant 256, which is the whole bug: the requirement is a +/// span of *film*, and the slots needed to hold it depend on `sample_fps`. +/// Pinned at 256 it was ~256 s of lag at 1 fps — 27x what the join needs — and +/// nothing recomputed it if `sample_fps` changed, so the one number the join's +/// correctness rests on drifted silently with an unrelated knob. +/// +/// It is also the largest single memory item in the pipeline. Every message +/// embeds `Frame source`, so a slot on this branch holds a full decoded image: +/// 256 of them is ~1.5 GB at 1080p, against ~110 MB for the derived depth at +/// 1 fps. See AR-004 — capacity is counted in items, and only the byte figure +/// (now correct, see types.hpp) shows what a slot really costs. +static std::size_t scene_join_depth(float sample_fps) { + const double slots = kSceneJoinSafety * kSceneJoinLagSec * sample_fps; + // Floor of 16: below that the queue stops absorbing ordinary jitter and + // starts throttling the fanout, which would slow the dense branch it + // exists to let run ahead. + return std::max(16, static_cast(std::ceil(slots))); +} + +/// TRACES: AR-004 | SR-002 +/// The decimator's input, on the *full-rate* stream. +/// +/// This was also kSceneJoinDepth, which put a 256-slot buffer of full-rate +/// frames in front of the decimator — and at 1 fps against 24 fps native, 23 of +/// every 24 of those frames exist only to be discarded a moment later. Holding +/// ~1.5 GB of decoded images for frames the very next node throws away is the +/// worst available use of the memory budget. +/// +/// A filter is a pass-through, not a reservoir: the lag belongs *after* +/// decimation, where a slot buys `1/sample_fps` seconds of film instead of +/// `1/native_fps`. Sized only to keep the decimator fed. +static constexpr std::size_t kDecimatorInputDepth = 16; /// Set when the scene branch is built, so shutdown can report whether the join /// actually worked. @@ -529,7 +586,7 @@ int main(int argc, char** argv) { auto boundaries = std::make_shared(); scene_fn.set_boundaries(boundaries); kpn::ObjectNode, kpn::out<>, "scene_detector", 0> - scene_node(scene_fn, 128); + scene_node(scene_fn, kSceneInputDepth); // Decimator: keep frames on the sample_fps cadence, drop the rest. // eof always passes so downstream shuts down cleanly. Stateful — one @@ -544,7 +601,7 @@ int main(int argc, char** argv) { return true; } return false; - }, kSceneJoinDepth); + }, kDecimatorInputDepth); /// TRACES: AR-010 | SR-002 // Stamp is_scene_boundary from the detector's published verdict. tol is @@ -559,7 +616,7 @@ int main(int argc, char** argv) { // otherwise look exactly like "no boundary here". SceneBoundaryAnnotatorFunc annotate_fn{boundaries, 0.5 / cfg.sample_fps}; kpn::ObjectNode, kpn::out<"frame">, - "scene_annotate", 0> annotate(annotate_fn, kSceneJoinDepth); + "scene_annotate", 0> annotate(annotate_fn, scene_join_depth(cfg.sample_fps)); // Reported at shutdown: without this the join is unverifiable, and an // annotator that never fired looks identical to footage with no