diff --git a/frigate/object_detection/base.py b/frigate/object_detection/base.py index bc7910e4d0..858c8a5e4d 100644 --- a/frigate/object_detection/base.py +++ b/frigate/object_detection/base.py @@ -83,7 +83,7 @@ class BaseLocalDetector(ObjectDetector): raw_detections = self.detect_raw(tensor_input) # type: ignore[attr-defined] for d in raw_detections: - if int(d[0]) < 0 or int(d[0]) >= len(self.labels): + if int(d[0]) not in self.labels: logger.warning(f"Raw Detect returned invalid label: {d}") continue if d[1] < threshold: @@ -395,6 +395,9 @@ class RemoteObjectDetector: self.labels = labels self.name = name self.fps = EventsPerSecond() + # class ids already warned about, so an incomplete labelmap logs once + # per id instead of once per frame + self.unnamed_class_ids: set[int] = set() self.detection_queue = detection_queue self.stop_event = stop_event self.shm = UntrackedSharedMemory(name=self.name, create=False) @@ -436,9 +439,21 @@ class RemoteObjectDetector: for d in self.out_np_shm: if d[1] < threshold: break - detections.append( - (self.labels[int(d[0])], float(d[1]), (d[2], d[3], d[4], d[5])) - ) + + class_id = int(d[0]) + label = self.labels.get(class_id) + + if label is None: + if class_id not in self.unnamed_class_ids: + self.unnamed_class_ids.add(class_id) + logger.warning( + "Detector returned class id %d for %s, which the labelmap does not name. Check that labelmap_path matches the model", + class_id, + self.name, + ) + continue + + detections.append((label, float(d[1]), (d[2], d[3], d[4], d[5]))) self.fps.update() return detections diff --git a/frigate/test/test_keyframe_analysis.py b/frigate/test/test_keyframe_analysis.py index 14dad5849d..4234ae112b 100644 --- a/frigate/test/test_keyframe_analysis.py +++ b/frigate/test/test_keyframe_analysis.py @@ -26,6 +26,26 @@ class TestClassifyKeyframeGaps(unittest.TestCase): self.assertEqual(result["severity"], "warning") self.assertEqual(result["max_gap"], 5.5) + def test_fixed_pattern_for_regular_gop(self): + # a 5s GOP with normal encoder jitter is sparse but not variable + pts = [0.0, 4.98, 10.01, 15.0] + result = classify_keyframe_gaps(pts, segment_time=10) + self.assertEqual(result["severity"], "warning") + self.assertEqual(result["pattern"], "fixed") + + def test_variable_pattern_for_smart_codec(self): + # keyframes bunched up then a long stretch without one + pts = [0.0, 1.0, 2.0, 8.0] + result = classify_keyframe_gaps(pts, segment_time=10) + self.assertEqual(result["severity"], "warning") + self.assertEqual(result["pattern"], "variable") + + def test_fixed_pattern_for_short_regular_gop(self): + pts = [0.0, 1.0, 2.0, 3.0] + result = classify_keyframe_gaps(pts, segment_time=10) + self.assertEqual(result["severity"], "ok") + self.assertEqual(result["pattern"], "fixed") + def test_error_when_gap_exceeds_segment_time(self): pts = [0.0, 12.0] # 12s gap > 10s segment result = classify_keyframe_gaps(pts, segment_time=10) @@ -40,6 +60,7 @@ class TestClassifyKeyframeGaps(unittest.TestCase): result = classify_keyframe_gaps([1.0], segment_time=10) self.assertEqual(result["severity"], "unknown") self.assertIsNone(result["max_gap"]) + self.assertIsNone(result["pattern"]) self.assertEqual(result["keyframe_count"], 1) def test_unknown_with_no_keyframes(self): diff --git a/frigate/test/test_object_detector.py b/frigate/test/test_object_detector.py index dc15b23516..e50723a0fa 100644 --- a/frigate/test/test_object_detector.py +++ b/frigate/test/test_object_detector.py @@ -1,7 +1,8 @@ import unittest -from unittest.mock import Mock, patch +from unittest.mock import MagicMock, Mock, patch import numpy as np +import zmq from pydantic import parse_obj_as import frigate.detectors as detectors @@ -108,13 +109,13 @@ class TestLocalObjectDetector(unittest.TestCase): ("label-2", 0.5, (8, 7, 6, 5)), ] TEST_LABEL_FILE = "/test_labels.txt" - mock_load_labels.return_value = [ - "label-1", - "label-2", - "label-3", - "label-4", - "label-5", - ] + mock_load_labels.return_value = { + 0: "label-1", + 1: "label-2", + 2: "label-3", + 3: "label-4", + 4: "label-5", + } test_cfg = parse_obj_as(DetectorConfig, {"type": "cpu", "model": {}}) test_cfg.model = ModelConfig() @@ -136,3 +137,69 @@ class TestLocalObjectDetector(unittest.TestCase): == np.zeros((1, 32, 32, 3)).shape ) assert test_result == TEST_DETECT_RESULT + + +class TestRemoteObjectDetector(unittest.TestCase): + """Cover the label lookup that turns raw class ids into detections.""" + + def _build_detector(self, labels, rows): + detector = frigate.object_detection.base.RemoteObjectDetector.__new__( + frigate.object_detection.base.RemoteObjectDetector + ) + detector.labels = labels + detector.name = "front_door" + detector.fps = MagicMock() + detector.stop_event = MagicMock() + detector.stop_event.is_set.return_value = False + detector.unnamed_class_ids = set() + detector.np_shm = np.zeros((1, 320, 320, 3), np.uint8) + detector.out_np_shm = np.array(rows, np.float32) + detector.detection_queue = MagicMock() + detector.detector_subscriber = MagicMock() + detector.detector_subscriber.socket.recv_string.side_effect = zmq.Again() + detector.detector_subscriber.check_for_update.return_value = "front_door" + return detector + + def test_maps_class_ids_to_labels(self): + rows = [[2, 0.9, 0.1, 0.2, 0.3, 0.4], [0, 0.8, 0.5, 0.6, 0.7, 0.8]] + [ + [0, 0, 0, 0, 0, 0] + ] * 18 + detector = self._build_detector({0: "person", 2: "car"}, rows) + + results = detector.detect(np.zeros((1, 320, 320, 3), np.uint8)) + + self.assertEqual([r[0] for r in results], ["car", "person"]) + + def test_skips_class_ids_the_labelmap_does_not_name(self): + # a labelmap that names fewer classes than the model emits + rows = [[7, 0.9, 0.1, 0.2, 0.3, 0.4], [0, 0.8, 0.5, 0.6, 0.7, 0.8]] + [ + [0, 0, 0, 0, 0, 0] + ] * 18 + detector = self._build_detector({0: "person"}, rows) + + results = detector.detect(np.zeros((1, 320, 320, 3), np.uint8)) + + self.assertEqual([r[0] for r in results], ["person"]) + self.assertEqual(detector.unnamed_class_ids, {7}) + + def test_warns_once_per_unnamed_class_id(self): + rows = [ + [7, 0.9, 0.1, 0.2, 0.3, 0.4], + [7, 0.8, 0.1, 0.2, 0.3, 0.4], + [9, 0.7, 0.1, 0.2, 0.3, 0.4], + ] + [[0, 0, 0, 0, 0, 0]] * 17 + detector = self._build_detector({0: "person"}, rows) + + with self.assertLogs("frigate.object_detection.base", level="WARNING") as logs: + detector.detect(np.zeros((1, 320, 320, 3), np.uint8)) + + self.assertEqual(len(logs.output), 2) + self.assertEqual(detector.unnamed_class_ids, {7, 9}) + + def test_empty_labelmap_drops_detections_instead_of_raising(self): + rows = [[0, 0.9, 0.1, 0.2, 0.3, 0.4]] + [[0, 0, 0, 0, 0, 0]] * 19 + detector = self._build_detector({}, rows) + + results = detector.detect(np.zeros((1, 320, 320, 3), np.uint8)) + + self.assertEqual(results, []) diff --git a/frigate/util/builtin.py b/frigate/util/builtin.py index c1a6e787cb..6bf5c6f380 100644 --- a/frigate/util/builtin.py +++ b/frigate/util/builtin.py @@ -152,12 +152,19 @@ def get_record_segment_time(config: "CameraConfig") -> int: def load_labels( - path: str | None, encoding="utf-8", prefill=91, indexed: bool | None = None + path: str | None, encoding="utf-8", prefill=0, indexed: bool | None = None ): """Loads labels from file (with or without index numbers). + + Only the indices the file defines are returned, so the result describes + exactly the classes a model can name. Callers must treat a missing index + as an unnamed class rather than assuming a contiguous range. + Args: path: path to label file. encoding: label file encoding. + prefill: pad indices below this with "unknown" before reading the file. + indexed: whether lines start with an index; auto-detected when None. Returns: Dictionary mapping indices to labels. """ diff --git a/frigate/util/services.py b/frigate/util/services.py index ef5cd15d17..3336c59ba1 100644 --- a/frigate/util/services.py +++ b/frigate/util/services.py @@ -1061,6 +1061,7 @@ def ffprobe_stream(ffmpeg, path: str, detailed: bool = False) -> sp.CompletedPro KEYFRAME_PROBE_WINDOW_SECONDS = 20 KEYFRAME_GAP_WARNING_SECONDS = 4.0 +KEYFRAME_GAP_JITTER_SECONDS = 0.5 def parse_keyframe_packets(output: str) -> tuple[list[float], float | None]: @@ -1100,6 +1101,10 @@ def classify_keyframe_gaps( - "error" when the longest gap exceeds the record segment length - "warning" when the longest gap exceeds the warning threshold - "ok" otherwise + + The "pattern" key separates the two causes so callers can give accurate + advice: "fixed" is a regular GOP that is simply too long, "variable" is + the irregular spacing a smart/+ codec produces. """ thresholds = { "warning": KEYFRAME_GAP_WARNING_SECONDS, @@ -1112,6 +1117,7 @@ def classify_keyframe_gaps( "max_gap": None, "mean_gap": None, "min_gap": None, + "pattern": None, "segment_time": segment_time, "severity": "unknown", "thresholds": thresholds, @@ -1119,6 +1125,7 @@ def classify_keyframe_gaps( gaps = [b - a for a, b in zip(keyframe_pts, keyframe_pts[1:])] max_gap = max(gaps) + min_gap = min(gaps) if max_gap > segment_time: severity = "error" @@ -1127,11 +1134,16 @@ def classify_keyframe_gaps( else: severity = "ok" + # allow for encoder jitter and probe rounding before calling a GOP variable + tolerance = max(KEYFRAME_GAP_JITTER_SECONDS, min_gap * 0.25) + pattern = "variable" if (max_gap - min_gap) > tolerance else "fixed" + return { "keyframe_count": len(keyframe_pts), "max_gap": round(max_gap, 2), "mean_gap": round(sum(gaps) / len(gaps), 2), - "min_gap": round(min(gaps), 2), + "min_gap": round(min_gap, 2), + "pattern": pattern, "segment_time": segment_time, "severity": severity, "thresholds": thresholds, diff --git a/web/public/locales/en/views/system.json b/web/public/locales/en/views/system.json index 2aaa15cd76..5f261eba4f 100644 --- a/web/public/locales/en/views/system.json +++ b/web/public/locales/en/views/system.json @@ -184,8 +184,10 @@ "gap": "Keyframe gap (min / avg / max):", "segmentLength": "Recording segment length:", "ok": "Keyframes every ~{{seconds}}s, good for recording and playback.", - "warning": "Sparse or variable keyframes (longest gap ~{{seconds}}s), likely a smart codec (H.264+/H.265+), this is not recommended.", - "error": "Keyframe gap (~{{seconds}}s) exceeds the recording segment length ({{segmentTime}}s). Some segments may have no keyframe, which breaks playback. Disable the smart/+ codec on the camera or shorten its keyframe interval.", + "warningFixed": "Keyframes are evenly spaced but sparse (every ~{{seconds}}s). Recording still works, but live playback and seeking start more slowly. Set the camera's I-frame (keyframe) interval to match its frame rate.", + "warningVariable": "Keyframe spacing is inconsistent ({{minSeconds}}s to {{maxSeconds}}s), which usually means a smart codec (H.264+/H.265+) is enabled. This is not recommended.", + "errorFixed": "Keyframes every ~{{seconds}}s is longer than the recording segment length ({{segmentTime}}s), so some segments have no keyframe and will not play back. Shorten the camera's I-frame (keyframe) interval to match its frame rate.", + "errorVariable": "Keyframe gaps reach ~{{seconds}}s, longer than the recording segment length ({{segmentTime}}s). Some segments will have no keyframe, which breaks playback. Disable the smart/+ codec on the camera or shorten its keyframe interval.", "unknown": "Couldn't determine keyframe spacing.", "recordDisabled": "Recording is disabled for this camera." } diff --git a/web/src/components/overlay/KeyframeAnalysisSection.tsx b/web/src/components/overlay/KeyframeAnalysisSection.tsx index 299c2468e4..5424aea531 100644 --- a/web/src/components/overlay/KeyframeAnalysisSection.tsx +++ b/web/src/components/overlay/KeyframeAnalysisSection.tsx @@ -89,17 +89,29 @@ export default function KeyframeAnalysisSection({ case "warning": summary = ( - {t("cameras.info.keyframes.warning", { seconds: analysis.max_gap })} + {analysis.pattern === "fixed" + ? t("cameras.info.keyframes.warningFixed", { + seconds: analysis.mean_gap, + }) + : t("cameras.info.keyframes.warningVariable", { + minSeconds: analysis.min_gap, + maxSeconds: analysis.max_gap, + })} ); break; case "error": summary = ( - {t("cameras.info.keyframes.error", { - seconds: analysis.max_gap, - segmentTime: analysis.segment_time, - })} + {analysis.pattern === "fixed" + ? t("cameras.info.keyframes.errorFixed", { + seconds: analysis.mean_gap, + segmentTime: analysis.segment_time, + }) + : t("cameras.info.keyframes.errorVariable", { + seconds: analysis.max_gap, + segmentTime: analysis.segment_time, + })} ); break; diff --git a/web/src/types/stats.ts b/web/src/types/stats.ts index c9f3f0032a..8354ed4ab6 100644 --- a/web/src/types/stats.ts +++ b/web/src/types/stats.ts @@ -161,6 +161,8 @@ export type KeyframeSeverity = | "unknown" | "record_disabled"; +export type KeyframeGapPattern = "fixed" | "variable"; + export type KeyframeAnalysis = { severity: KeyframeSeverity; stream_index?: number; @@ -168,6 +170,7 @@ export type KeyframeAnalysis = { max_gap?: number | null; mean_gap?: number | null; min_gap?: number | null; + pattern?: KeyframeGapPattern | null; duration_observed?: number | null; segment_time?: number; thresholds?: { warning: number; error: number };