diff --git a/src/benchmark.hpp b/src/benchmark.hpp index 02a60ba..a749342 100644 --- a/src/benchmark.hpp +++ b/src/benchmark.hpp @@ -173,7 +173,7 @@ struct NodeCost { double queue_wait_ms{0.0}; bool is_bottleneck{false}; - /// TRACES: VR-015, AR-004 | PR-004 + /// TRACES: VR-015 | AR-004 | PR-004 /// Live scheduling state, so a wedged run says *why* it is wedged rather /// than only that it is. With `queued=0, wake=1` a wake was recorded and /// never consumed; with `queued=0, wake=0` and a full input, no wake was @@ -469,7 +469,7 @@ public: print_impl(os, final_, film_sec); } - /// TRACES: VR-015, AR-004 | PR-004 + /// TRACES: VR-015 | AR-004 | PR-004 /// Dump the report from a LIVE snapshot, mid-run, without stopping anything. /// /// A report that only exists at shutdown is no use against the failure this @@ -519,7 +519,7 @@ private: << "\n"; } - /// TRACES: VR-015, AR-004 | PR-004 + /// TRACES: VR-015 | AR-004 | PR-004 // Fires only when the scheduling state is actually wrong, so a healthy // run stays quiet and a wedged one names the fault — instead of leaving // it to be reconstructed under a debugger that suppresses the bug. diff --git a/src/gallery/track_gallery.hpp b/src/gallery/track_gallery.hpp index ee28aec..ccc387d 100644 --- a/src/gallery/track_gallery.hpp +++ b/src/gallery/track_gallery.hpp @@ -133,25 +133,52 @@ struct TrackGallery { TrackState& ts = tracks_[track_id]; - // Vote toward ownership: only accepted frames name an actor, and a track - // that flip-flops between actors is ambiguous, so we tally per actor and - // pick the plurality winner at confirmation time. - if (accepted && best_actor >= 0) { - ts.actor_votes[best_actor]++; - ts.accepted_frames++; - } + /// TRACES: AR-019 | SR-005 + // accepted_frames is an EVIDENCE FLOOR, not an identity decision: it + // asks "has this track been recognised often enough to be worth + // promoting", never "who is it". Who it is comes from the registry. + // + // There used to be a per-actor tally here too, and promote() fell back + // to its plurality winner. That made two answers to "who is this track" + // able to coexist, and the local one ignored the Bayesian accumulation + // entirely -- weighting thirty near-identical looks the same as thirty + // distinct ones, which is exactly what AR-025's discounting exists to + // stop. Since promotion only fired on the local count, the fallback was + // reachable in the live pipeline and not merely in tests: three + // accepted frames arrive well before a posterior crosses ownership. + if (accepted && best_actor >= 0) ts.accepted_frames++; insert_into_buffer(ts, emb, best_gal_sim, crop); - // Confirm and promote as soon as the anchor threshold is met, once. - if (!ts.promoted && ts.accepted_frames >= min_anchor_frames_) + // Confirm and promote once BOTH hold: the registry owns this track, and + // enough frames have been accepted to be worth the slots. Ownership is + // the necessary one -- without it there is no actor to promote into. + if (!ts.promoted && ts.registry_owner >= 0 && + ts.accepted_frames >= min_anchor_frames_) promote(track_id, ts); } - // Drop a track's buffer when the face_tracker expires it or on a scene cut, - // so stale/cross-cut embeddings can never be promoted later. Called by the - // matcher when it observes a cut or track disappearance. - void forget(int track_id) { tracks_.erase(track_id); } + /// TRACES: AR-019 | SR-005 + /// Drop the buffers of tracks the registry no longer has. + /// + /// `alive` is the registry's own liveness test, so this annotates the track + /// pool rather than duplicating it — the same shape as FaceTrackerFunc's + /// prune_boxes, and for the same reason: a second opinion about which + /// tracks exist is a second thing that can be wrong. + /// + /// This replaces a `forget(int)` that had NO callers, under a comment + /// asserting "called by the matcher when it observes a cut or track + /// disappearance". The cut half was true by another route (clear_tracks); + /// the disappearance half was not, so a track that died quietly kept its + /// buffer until the next cut cleared everything. + template + void prune_dead(const AlivePredicate& alive) { + if (!enabled_) return; + for (auto it = tracks_.begin(); it != tracks_.end(); ) { + if (alive(it->first)) ++it; + else it = tracks_.erase(it); + } + } /// TRACES: AR-019 | SR-005 /// The registry's verdict on who this track is. Authoritative: it comes from @@ -198,7 +225,6 @@ private: struct TrackState { std::vector buf; - std::map actor_votes; // actor_idx → accepted-frame count int accepted_frames{0}; bool promoted{false}; int registry_owner{-1}; ///< AR-019: authoritative @@ -271,8 +297,8 @@ private: void promote(int track_id, TrackState& ts) { ts.promoted = true; // idempotent: never promote a track twice - int actor = owning_actor(ts); - if (actor < 0) return; + const int actor = ts.registry_owner; + if (actor < 0) return; // unreachable: observe() gates on this // ── Safety gate: the band's lower bound, across the whole store ────── float worst = store_coherence(ts.buf); @@ -302,21 +328,6 @@ private: << " views; annex now " << annex_size() << "\n"; } - /// Prefer the registry's verdict; fall back to the local tally only when no - /// registry is attached (unit tests, replay harness). - static int owning_actor(const TrackState& ts) { - if (ts.registry_owner >= 0) return ts.registry_owner; - return plurality_actor(ts); - } - - static int plurality_actor(const TrackState& ts) { - int best = -1, best_votes = 0; - for (const auto& [ai, v] : ts.actor_votes) { - if (v > best_votes) { best_votes = v; best = ai; } - } - return best; - } - /// TRACES: AR-018, AR-024 | SR-005 /// The store's weakest pairwise P(same person) — the band's lower bound /// asked of every pair, not just of the best match at the door. diff --git a/src/main.cpp b/src/main.cpp index ed6f47d..a79d3c6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -105,7 +105,7 @@ static constexpr std::size_t kSceneJoinDepth = 256; /// actually worked. static std::shared_ptr scene_stats; -/// TRACES: VR-015, AR-004 | PR-004 +/// TRACES: VR-015 | AR-004 | PR-004 /// Set by SIGUSR1, serviced by the wait loop. `kill -USR1 ` on a running /// or WEDGED run prints the benchmark table immediately — channel occupancy /// names the stalled node (full input, empty output) without a debug build or a @@ -371,7 +371,7 @@ int main(int argc, char** argv) { !scene_done.load(std::memory_order_acquire)) && !node_crashed.load(std::memory_order_acquire)) { std::this_thread::sleep_for(std::chrono::milliseconds(100)); - /// TRACES: VR-015, AR-004 | PR-004 + /// TRACES: VR-015 | AR-004 | PR-004 if (g_dump_request.exchange(false, std::memory_order_relaxed)) bench.dump_live(std::cerr, film_sec.load(std::memory_order_acquire)); } diff --git a/src/nodes/identity_matcher_node.hpp b/src/nodes/identity_matcher_node.hpp index 08d531d..ff6ebf7 100644 --- a/src/nodes/identity_matcher_node.hpp +++ b/src/nodes/identity_matcher_node.hpp @@ -311,6 +311,16 @@ struct IdentityMatcherFunc { absorb_promotions(); + /// TRACES: AR-019 | SR-005 + // Drop buffers for tracks the registry has reaped. Without this a track + // that simply went off screen kept its diversity buffer until the next + // cut, so the store grew with the film rather than with what is on + // screen — and a buffer that outlives its track is evidence about a + // person nobody is looking at any more. + if (registry_) + track_gallery_.prune_dead( + [this](int id) { return registry_->is_live(id); }); + return {std::move(tf.source), std::move(actors)}; } diff --git a/src/nodes/result_sink_node.hpp b/src/nodes/result_sink_node.hpp index fc1a059..06ffacf 100644 --- a/src/nodes/result_sink_node.hpp +++ b/src/nodes/result_sink_node.hpp @@ -152,6 +152,7 @@ private: double start{0.0}; double end{0.0}; float belief{0.f}; ///< the posterior that justified the claim (AR-017) + Route route{Route::live}; ///< how it was identified (AR-017) }; struct ActorWindow { std::string name, imdb_id, tmdb_id, jellyfin_id; @@ -180,7 +181,7 @@ private: aw.jellyfin_id = it->second.jellyfin_id; } } - aw.scenes.push_back({c.first_seen, c.last_seen, c.belief}); + aw.scenes.push_back({c.first_seen, c.last_seen, c.belief, c.route}); } std::vector result; @@ -204,7 +205,7 @@ private: windows.push_back({{"start", w.start}, {"end", w.end}, {"belief", w.belief}, - {"route", "live"}}); + {"route", route_name(w.route)}}); json ja; ja["name"] = aw.name; ja["imdb_id"] = aw.imdb_id; diff --git a/src/nodes/scene_detector_node.hpp b/src/nodes/scene_detector_node.hpp index bb9305c..e9369a2 100644 --- a/src/nodes/scene_detector_node.hpp +++ b/src/nodes/scene_detector_node.hpp @@ -149,8 +149,15 @@ private: // final verdict. The face branch consults this for frames it has not // reached yet, and the watermark is what lets it tell "no boundary // here" from "not scored yet". - if (shared_ && hi > lo) - shared_->publish(fresh, times_[hi - 1]); + /// TRACES: AR-011 | SR-002 + // Hand the join the same dedup window scenes.json uses, derived from the + // observed cadence rather than assumed. Set on every window because the + // median refines as intervals accumulate; it converges within the first + // window and costs a double assignment thereafter. + if (shared_) { + shared_->set_merge_window(dedup_window_sec(intervals_)); + if (hi > lo) shared_->publish(fresh, times_[hi - 1]); + } } // At EOF the tail (< kWindow frames) never formed a full window. Pad it out @@ -183,7 +190,10 @@ private: // the last full window — reach the join with no verdict and are treated // as boundary-free without evidence, which is precisely the ambiguity // the watermark exists to prevent. - if (shared_ && n > 0) shared_->publish(fresh, times_[n - 1]); + if (shared_ && n > 0) { + shared_->set_merge_window(dedup_window_sec(intervals_)); + shared_->publish(fresh, times_[n - 1]); + } } void write_output() { diff --git a/src/scene_boundaries.hpp b/src/scene_boundaries.hpp index c2f8002..5f879c1 100644 --- a/src/scene_boundaries.hpp +++ b/src/scene_boundaries.hpp @@ -27,15 +27,35 @@ class SceneBoundaries { public: - /// Peaks closer than this are one boundary. Matches the dedup scenes.json - /// applies, so the two views agree. - static constexpr double kMergeSec = 0.04; + /// TRACES: AR-011 | SR-002 + /// Peaks closer than this are one boundary. + /// + /// Supplied by the detector, derived from the cadence it was actually fed + /// (SceneDetectorFunc::dedup_window_sec), NOT assumed. It used to be a hard + /// 0.04 here, and AR-011 is recorded as having replaced that literal -- + /// which it did, but only for scenes.json. This path, the one that feeds + /// is_scene_boundary into the tracker, kept the constant while the comment + /// above it claimed "matches the dedup scenes.json applies, so the two + /// views agree". They did not agree. 0.04 s is one frame at 25 fps and + /// wider than a frame at 30, so two cuts on consecutive frames merged into + /// one and the loss was invisible: the pipeline simply saw fewer + /// boundaries. + /// + /// Zero until the detector sets it, which makes the pre-cadence state a + /// no-op dedup rather than a wrong one -- adjacent peaks stay separate + /// until there is evidence about how far apart frames are, and is_boundary + /// absorbs duplicates in its tolerance anyway. + void set_merge_window(double sec) { + std::lock_guard g(mu_); + merge_sec_ = sec; + } /// Called by the scene detector as each window is scored. `through` is the /// timestamp up to which its verdict is now final. void publish(const std::vector& ts, double through) { { std::lock_guard g(mu_); + const double merge = merge_sec_; // Dedup on insert, matching what scenes.json does at write time. A run // of adjacent high-scoring frames is one boundary, not several, and // leaving them raw made this view report 357 where the file said 13 — @@ -45,7 +65,7 @@ public: bounds_.insert(bounds_.end(), ts.begin(), ts.end()); std::sort(bounds_.begin(), bounds_.end()); bounds_.erase(std::unique(bounds_.begin(), bounds_.end(), - [](double a, double b) { return b - a < kMergeSec; }), + [merge](double a, double b) { return b - a < merge; }), bounds_.end()); scored_through_ = std::max(scored_through_, through); } @@ -120,6 +140,7 @@ private: mutable std::mutex mu_; mutable std::condition_variable cv_; bool finished_{false}; + double merge_sec_{0.0}; ///< set by the detector; see set_merge_window std::vector bounds_; double scored_through_{-1.0}; mutable std::size_t outran_{0}; diff --git a/src/track_registry.hpp b/src/track_registry.hpp index b0891e6..0304e0c 100644 --- a/src/track_registry.hpp +++ b/src/track_registry.hpp @@ -44,12 +44,39 @@ // A finished presence claim, emitted exactly once when a track is reaped or // flushed. Immutable by construction: it carries everything needed to justify // itself (AR-017), with no back-reference into registry state. +/// TRACES: AR-017 | IR-002 | SR-002, SR-003 +/// How an actor came to be attached to a track. +/// +/// AR-017 requires every presence claim to carry its identification route, and +/// IR-002 publishes it per window. Until now the sink wrote the string "live" +/// unconditionally, so the field existed but could not distinguish anything -- +/// and AR-017's own verification asks for "deferred and pooled routes +/// distinguishable". +/// +/// Only `live` occurs today. `deferred` is what AR-020's pass will set when it +/// resolves a track that failed during streaming and was identified against the +/// final expanded gallery; the value exists now so that pass has somewhere to +/// write rather than a serialisation change to make. +enum class Route { + live, ///< identified while streaming, from accumulated per-frame evidence + deferred, ///< resolved after EOF against the expanded gallery (AR-020) +}; + +inline const char* route_name(Route r) { + switch (r) { + case Route::deferred: return "deferred"; + case Route::live: break; + } + return "live"; +} + struct DeadTrack { int track_id{-1}; double first_seen{0.0}; double last_seen{0.0}; ///< always the last sighting, never the death time int actor_idx{-1}; ///< -1 when the track was never owned float belief{0.0f}; ///< accumulated posterior for actor_idx + Route route{Route::live}; ///< how the actor was attached (AR-017) int observations{0}; ///< evidence updates that landed on this track float effective_obs{0.f}; ///< sum of discounted weights — the evidence that counted }; @@ -215,6 +242,15 @@ public: // ── Diagnostics ────────────────────────────────────────────────────────── // These measure how often tracking is silently wrong, which nothing in the // pipeline currently reveals. + /// Whether the registry still holds this track. The authority on which + /// tracks exist, so annotating structures elsewhere (spatial boxes in the + /// tracker, diversity buffers in the expansion store) can prune against it + /// rather than keeping a second opinion. + bool is_live(int track_id) const { + std::lock_guard g(mu_); + return tracks_.count(track_id) != 0; + } + int dropped_votes() const { std::lock_guard g(mu_); return dropped_votes_; } int belief_swaps() const { std::lock_guard g(mu_); return belief_swaps_; } int actor_conflicts() const { std::lock_guard g(mu_); return actor_conflicts_; } @@ -340,17 +376,6 @@ private: for (int i = 0; i < 512; ++i) t.mean[i] = static_cast(t.mean[i] / norm); } - static float logistic(float z) { - return z >= 0 ? 1.f / (1.f + std::exp(-z)) - : std::exp(z) / (1.f + std::exp(z)); - } - - static float logit(float p) { - const float eps = 1e-6f; - p = std::min(1.f - eps, std::max(eps, p)); - return std::log(p / (1.f - p)); - } - Config cfg_; EvidenceDiscounter discounter_; mutable std::mutex mu_; diff --git a/tests/test_track_gallery.cpp b/tests/test_track_gallery.cpp index 309ffc9..b68da91 100644 --- a/tests/test_track_gallery.cpp +++ b/tests/test_track_gallery.cpp @@ -210,6 +210,9 @@ TEST_CASE("band thresholds probability, not cosine", "[track_gallery][AR-018][AR TEST_CASE("a two-person track never poisons the annex", "[track_gallery][AR-018]") { TrackGallery tg(expand_cfg()); tg.set_calibration(identity_cal); + // AR-019: promotion needs the registry's verdict; the local + // accepted-frame plurality that used to supply it is gone. + tg.set_owner(3, 0); // Two orthogonal identities under one track ID — a track-ID collision. // The band refuses the outsider at the door, so the store never becomes // two-person in the first place. @@ -246,6 +249,9 @@ TEST_CASE("a track that drifts through the band is refused at promotion", TEST_CASE("confirmed track promotes its store", "[track_gallery][AR-019]") { TrackGallery tg(expand_cfg()); tg.set_calibration(identity_cal); + // AR-019: promotion needs the registry's verdict; the local + // accepted-frame plurality that used to supply it is gone. + tg.set_owner(7, 0); REQUIRE(tg.enabled()); // A track owned by actor 0: every frame accepted, every view mutually @@ -281,21 +287,29 @@ TEST_CASE("unconfirmed track (too few accepts) does not promote", "[track_galler CHECK(tg.annex_size() == 0); } -TEST_CASE("plurality actor wins a mixed-vote track", "[track_gallery][AR-019]") { +TEST_CASE("an unowned track never promotes, however many frames it accepts", + "[track_gallery][AR-019]") { TrackGallery tg(expand_cfg()); tg.set_calibration(identity_cal); - // No registry attached (unit-test path): actor 5 accepted twice, actor 6 - // once → plurality is 5. - tg.observe(8, spoke(1, kSpokeCos), 5, 0.30f, true, kNoCrop); - tg.observe(8, spoke(2, kSpokeCos), 5, 0.30f, true, kNoCrop); - tg.observe(8, spoke(3, kSpokeCos), 6, 0.30f, true, kNoCrop); - REQUIRE(tg.annex_size() > 0); - for (int actor : tg.annex_actors()) CHECK(actor == 5); + // This case used to assert the opposite: it drove a mixed-vote track with + // no registry owner and expected the local plurality winner (actor 5) to + // take the promotion. That fallback is gone. AR-019 says ownership comes + // from the registry and not from a second local tally, and the tally could + // not see the AR-025 discounting -- so it weighted thirty near-identical + // looks like thirty distinct ones. + // + // Ten accepted frames, no set_owner, nothing promoted. + for (int k = 1; k <= 10; ++k) + tg.observe(8, spoke(k, kSpokeCos), 5, 0.30f, true, kNoCrop); + CHECK(tg.annex_size() == 0); } TEST_CASE("promotion is idempotent across a long track", "[track_gallery][AR-019]") { TrackGallery tg(expand_cfg()); tg.set_calibration(identity_cal); + // AR-019: promotion needs the registry's verdict; the local + // accepted-frame plurality that used to supply it is gone. + tg.set_owner(9, 0); for (int k = 1; k <= 3; ++k) tg.observe(9, spoke(k, kSpokeCos), 0, 0.30f, true, kNoCrop); const int after_confirm = tg.annex_size(); @@ -328,6 +342,9 @@ TEST_CASE("eviction keeps the gallery-far views", "[track_gallery][AR-018]") { cfg.expand_min_anchor_frames = 4; TrackGallery tg(cfg); tg.set_calibration(identity_cal); + // AR-019: promotion needs the registry's verdict; the local + // accepted-frame plurality that used to supply it is gone. + tg.set_owner(6, 0); tg.observe(6, spoke(1, kSpokeCos), 0, 0.80f, true, kNoCrop); // well recognised tg.observe(6, spoke(2, kSpokeCos), 0, 0.40f, true, kNoCrop); @@ -354,6 +371,10 @@ TEST_CASE("promotions drain exactly once, in matrix order", "[track_gallery][AR-026]") { TrackGallery tg(expand_cfg()); tg.set_calibration(identity_cal); + // AR-019: promotion needs the registry's verdict; the local + // accepted-frame plurality that used to supply it is gone. + tg.set_owner(7, 0); + tg.set_owner(8, 4); std::vector emb; std::vector actor;