From 36607133e5f70701f885663014c77db4596d1f41 Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Mon, 17 Aug 2026 08:09:10 -0500 Subject: [PATCH] Improve History's seek startup time and recordings query performance (#24011) * serve a segment startup ladder so seeks begin playing sooner nginx-vod was handed one 10s segment per recording file, so every playlist start had to download and decode a full segment before the first frame. Declare real keyframe data per clip and let nginx cut short leading segments from it. - add vod_bootstrap_segment_durations 1000/2000/4000 so each playlist starts with 1s/2s/4s segments before settling at 10s - emit real clip-relative keyFrameDurations (plus firstKeyFrameOffset when nonzero) from the recording keyframe index; rows without an index keep the whole-clip declaration, the only safe cut without keyframe knowledge - drop the manifest's segment_duration field, which was always inert: nginx-vod parses only camelCase segmentDuration - rebuild the player source at the seek target, quantized to a 10s grid, so the ladder applies to every seek and seek URLs stay repeatable for nginx's mapping and response caches - route the seek model, in-range checks, and the stale-report guard through the source window rather than the chunk range - bridge repositioning seeks (>2s from the last played timestamp) through the preview player and hold the release anchor one commit, so neither path paints a stale frame - clear a pending loading timer before replacing it; an orphaned timer escaped onPlaying's clearTimeout and flashed loading mid-playback * keep recordings queries on their indexes Several recordings queries degraded into full scans or large sorts on big databases: the planner ignored index order, or the query shape gave it nothing tight to seek on. Reshape them into bounded seeks and add the composite index the per-stream lookups need. - index recordings on (camera, stream_type, start_time DESC) and drop the (camera, stream_type) index it supersedes - walk the recordings summary day by day with EXISTS probes and per-camera MIN/MAX seeks, skipping ahead over empty gaps instead of bucketing every row for the requested cameras - run the summary endpoint on the event loop rather than the threadpool - bound the unavailable-recordings query by start_time per camera and merge the results in Python - bound the expire query's start_time so it seeks the retention window instead of scanning a camera's whole history - enumerate deleted cameras with one index seek each rather than a camera NOT IN (...) scan - compute bandwidth with segment_size filtered in a CASE projection; as a WHERE predicate it baited the planner into the (camera, segment_size) index plus a full sort of the camera's history - fall back to a 1000-segment window when the recent 100 are all zero-size, so an ingest glitch doesn't report zero bandwidth - limit the needs_refresh count instead of counting every segment - cover sub-only and sparse calendar days, midnight-spanning day attribution, multi-camera gap merging, deleted-camera expiry, and zero-size segment runs * fix mypy --- .../rootfs/usr/local/nginx/conf/nginx.conf | 6 + frigate/api/media.py | 15 +- frigate/api/record.py | 123 ++++++++++------ frigate/record/cleanup.py | 56 +++++--- frigate/storage.py | 69 +++++---- frigate/test/http_api/test_http_media.py | 133 ++++++++++++++++- frigate/test/test_record_cleanup_sub.py | 47 +++++- frigate/test/test_recording_coverage.py | 71 +++++++++ frigate/test/test_storage.py | 35 +++++ frigate/util/recording_coverage.py | 40 +++++- migrations/037_add_recordings_stream_index.py | 43 ++++++ .../player/dynamic/DynamicVideoPlayer.tsx | 136 ++++++++++++++---- 12 files changed, 654 insertions(+), 120 deletions(-) create mode 100644 migrations/037_add_recordings_stream_index.py diff --git a/docker/main/rootfs/usr/local/nginx/conf/nginx.conf b/docker/main/rootfs/usr/local/nginx/conf/nginx.conf index 399dba5736..262828f372 100644 --- a/docker/main/rootfs/usr/local/nginx/conf/nginx.conf +++ b/docker/main/rootfs/usr/local/nginx/conf/nginx.conf @@ -75,6 +75,12 @@ http { vod_align_segments_to_key_frames on; vod_manifest_segment_durations_mode accurate; vod_ignore_edit_list on; + # short leading segments at each playlist start; sources start at + # the seek target, so the ladder applies to every seek. Only + # effective when clips declare real keyFrameDurations + vod_bootstrap_segment_durations 1000; + vod_bootstrap_segment_durations 2000; + vod_bootstrap_segment_durations 4000; vod_segment_duration 10000; # MPEG-TS settings (not used when fMP4 is enabled, kept for reference) diff --git a/frigate/api/media.py b/frigate/api/media.py index 95b7f924a7..3547681d4d 100644 --- a/frigate/api/media.py +++ b/frigate/api/media.py @@ -598,7 +598,15 @@ def _build_vod_clip( clip: dict[str, Any] = {"type": "source", "path": row.path} if plan.clip_from_ms is not None: clip["clipFrom"] = plan.clip_from_ms - clip["keyFrameDurations"] = [plan.duration_ms] + if plan.key_frame_durations is not None: + # real gaps enable keyframe-aligned sub-file segments (bootstrap + # ladder); the whole-clip fallback keeps one segment per file, + # the only safe cut without an index + if plan.first_key_frame_offset_ms > 0: + clip["firstKeyFrameOffset"] = plan.first_key_frame_offset_ms + clip["keyFrameDurations"] = plan.key_frame_durations + else: + clip["keyFrameDurations"] = [plan.duration_ms] logger.debug( "VOD: added clip %s duration_ms=%s clipFrom=%s", row.path, @@ -740,14 +748,15 @@ async def _vod_response( NGINX_VOD_MAX_CLIPS, ) + # segmentation comes from the vod_* nginx directives plus per-clip + # keyFrameDurations; a segment_duration field here was always ignored + # (nginx-vod parses only camelCase segmentDuration) hour_ago = datetime.now() - timedelta(hours=1) content = { "cache": hour_ago.timestamp() > start_ts, "discontinuity": force_discontinuity or use_discontinuity, "consistentSequenceMediaInfo": True, "durations": durations, - # aligns segments to recording file boundaries - "segment_duration": max(durations), "sequences": [{"clips": clips}], } if use_discontinuity: diff --git a/frigate/api/record.py b/frigate/api/record.py index 451dd2bf1d..9a59e0ccb0 100644 --- a/frigate/api/record.py +++ b/frigate/api/record.py @@ -71,7 +71,7 @@ def get_recordings_storage_usage(request: Request): @router.get("/recordings/summary", dependencies=[Depends(allow_any_authenticated())]) -def all_recordings_summary( +async def all_recordings_summary( request: Request, params: MediaRecordingsSummaryQueryParams = Depends(), allowed_cameras: list[str] = Depends(get_allowed_cameras_for_filter), @@ -88,18 +88,23 @@ def all_recordings_summary( else: camera_list = allowed_cameras - time_range_query = ( - Recordings.select( - fn.MIN(Recordings.start_time).alias("min_time"), - fn.MAX(Recordings.start_time).alias("max_time"), + min_time: float | None = None + max_time: float | None = None + for camera in camera_list: + cam_min = ( + Recordings.select(fn.MIN(Recordings.start_time)) + .where(Recordings.camera == camera) + .scalar() ) - .where(Recordings.camera << camera_list) - .dicts() - .get() - ) - - min_time = time_range_query.get("min_time") - max_time = time_range_query.get("max_time") + if cam_min is None: + continue + cam_max = ( + Recordings.select(fn.MAX(Recordings.start_time)) + .where(Recordings.camera == camera) + .scalar() + ) + min_time = cam_min if min_time is None else min(min_time, cam_min) + max_time = cam_max if max_time is None else max(max_time, cam_max) if min_time is None or max_time is None: return JSONResponse(content={}) @@ -109,22 +114,60 @@ def all_recordings_summary( days: dict[str, bool] = {} for period_start, period_end, period_offset in dst_periods: - day_expr = ((Recordings.start_time + period_offset) / 86400).cast("int") + first_start = max(min_time, period_start - MAX_SEGMENT_DURATION) + first_day = int((first_start + period_offset) // 86400) + last_day = int((min(max_time, period_end) + period_offset) // 86400) - period_query = ( - Recordings.select(day_expr.alias("day_idx")) - .where( - (Recordings.camera << camera_list) - & (Recordings.end_time >= period_start) - & (Recordings.start_time <= period_end) + day_idx = first_day + while day_idx <= last_day: + day_str = (dt.date(1970, 1, 1) + dt.timedelta(days=day_idx)).isoformat() + day_start = day_idx * 86400 - period_offset + day_end = day_start + 86400 + + if day_str in days: + day_idx += 1 + continue + + if day_end <= period_end: + upper = Recordings.start_time < day_end + else: + upper = Recordings.start_time <= period_end + + has_recordings = ( + Recordings.select(Recordings.id) + .where( + (Recordings.camera << camera_list) + & (Recordings.end_time >= period_start) + & (Recordings.start_time >= day_start) + & upper + ) + .exists() ) - .distinct() - .namedtuples() - ) + if has_recordings: + days[day_str] = True + day_idx += 1 + continue - for g in period_query: - day_str = (dt.date(1970, 1, 1) + dt.timedelta(days=g.day_idx)).isoformat() - days[day_str] = True + # empty day + next_start: float | None = None + for camera in camera_list: + cam_next = ( + Recordings.select(fn.MIN(Recordings.start_time)) + .where( + Recordings.camera == camera, + Recordings.start_time >= day_end, + Recordings.start_time <= period_end, + ) + .scalar() + ) + if cam_next is not None and ( + next_start is None or cam_next < next_start + ): + next_start = cam_next + + if next_start is None: + break + day_idx = max(day_idx + 1, int((next_start + period_offset) // 86400)) return JSONResponse(content=dict(sorted(days.items()))) @@ -373,22 +416,22 @@ async def no_recordings( ) scale = params.scale - clauses = [ - (Recordings.end_time >= after) & (Recordings.start_time <= before), - (Recordings.camera << camera_list), - ] + recordings: list[tuple[float, float]] = [] + for camera in camera_list: + recordings.extend( + Recordings.select(Recordings.start_time, Recordings.end_time) + .where( + Recordings.camera == camera, + Recordings.start_time >= after - MAX_SEGMENT_DURATION, + Recordings.end_time >= after, + Recordings.start_time <= before, + ) + .tuples() + .iterator() + ) - # Get recording start times - data: list[Recordings] = ( - Recordings.select(Recordings.start_time, Recordings.end_time) - .where(reduce(operator.and_, clauses)) - .order_by(Recordings.start_time.asc()) - .dicts() - .iterator() - ) - - # Convert recordings to list of (start, end) tuples, ordered by start_time - recordings = [(r["start_time"], r["end_time"]) for r in data] + # the merge pass below expects a single start-ordered timeline + recordings.sort() # Merge overlapping/adjacent recordings into covered intervals. The query # orders by start_time, so a single pass merges them diff --git a/frigate/record/cleanup.py b/frigate/record/cleanup.py index 1874e0ed33..03e0ceb2f7 100644 --- a/frigate/record/cleanup.py +++ b/frigate/record/cleanup.py @@ -167,6 +167,10 @@ class RecordingCleanup(threading.Thread): .where( (Recordings.camera == config.name) & (Recordings.stream_type == stream_type) + & ( + Recordings.start_time + < max(continuous_expire_date, motion_expire_date) + ) & ( ( (Recordings.end_time < continuous_expire_date) @@ -333,27 +337,45 @@ class RecordingCleanup(threading.Thread): expire_before = ( datetime.datetime.now() - datetime.timedelta(days=expire_days) ).timestamp() - no_camera_recordings = ( - Recordings.select( - Recordings.id, - Recordings.path, - ) - .where( - Recordings.camera.not_in(list(self.config.cameras.keys())), # type: ignore[call-arg, arg-type, misc] - Recordings.end_time < expire_before, - ) - .namedtuples() - .iterator() - ) + + # enumerate the distinct cameras with one index seek each + db_cameras: list[str] = [] + last_camera: str | None = None + while True: + query = Recordings.select(Recordings.camera) + if last_camera is not None: + query = query.where(Recordings.camera > last_camera) + next_camera = query.order_by(Recordings.camera.asc()).limit(1).scalar() + if next_camera is None: + break + db_cameras.append(next_camera) + last_camera = next_camera maybe_empty_dirs = set() deleted_recordings = set() - for recording in no_camera_recordings: - recording_path = Path(recording.path) - recording_path.unlink(missing_ok=True) - deleted_recordings.add(recording.id) - maybe_empty_dirs.add(recording_path.parent) + for camera in db_cameras: + if camera in self.config.cameras: + continue + + no_camera_recordings = ( + Recordings.select( + Recordings.id, + Recordings.path, + ) + .where( + Recordings.camera == camera, + Recordings.end_time < expire_before, + ) + .namedtuples() + .iterator() + ) + + for recording in no_camera_recordings: + recording_path = Path(recording.path) + recording_path.unlink(missing_ok=True) + deleted_recordings.add(recording.id) + maybe_empty_dirs.add(recording_path.parent) logger.debug(f"Expiring {len(deleted_recordings)} recordings") # delete up to 100,000 at a time diff --git a/frigate/storage.py b/frigate/storage.py index 5e7c46957e..6baf5b5f55 100644 --- a/frigate/storage.py +++ b/frigate/storage.py @@ -6,7 +6,7 @@ import threading from multiprocessing.synchronize import Event as MpEvent from pathlib import Path -from peewee import SQL, fn +from peewee import SQL, Case, fn from frigate.config import FrigateConfig from frigate.const import ( @@ -35,6 +35,35 @@ class StorageMaintainer(threading.Thread): self.stop_event = stop_event self.camera_storage_stats: dict[str, dict] = {} + def _recent_stream_bandwidth( + self, camera: str, stream_type: str, window: int + ) -> float | None: + """Average MB/s over a stream's most recent rows, or None if no sample. + + Zero-size rows are excluded inside the projection, not the WHERE + clause: a segment_size predicate baits the planner into the + (camera, segment_size) index plus a full sort of the camera's + history instead of the time-ordered index. + """ + recent = ( + Recordings.select( + Case( + None, + [(Recordings.segment_size > 0, bandwidth_equation)], + None, + ).alias("bw") + ) + .where( + Recordings.camera == camera, + Recordings.stream_type == stream_type, + ) + .order_by(Recordings.start_time.desc()) + .limit(window) + .alias("recent") + ) + avg: float | None = Recordings.select(fn.AVG(SQL("bw"))).from_(recent).scalar() + return avg + def calculate_camera_bandwidth(self) -> None: """Calculate an average MB/hr for each camera.""" for camera in self.config.cameras.keys(): @@ -47,9 +76,10 @@ class StorageMaintainer(threading.Thread): if self.camera_storage_stats.get(camera, {}).get("needs_refresh", True): self.camera_storage_stats[camera] = { "needs_refresh": ( - Recordings.select(fn.COUNT("*")) + Recordings.select(Recordings.id) .where(Recordings.camera == camera, Recordings.segment_size > 0) - .scalar() + .limit(50) + .count() < 50 ) } @@ -58,31 +88,18 @@ class StorageMaintainer(threading.Thread): # type and sum the rates; mixing streams would average small # sub segments against large main segments and underestimate # the true write rate - bandwidth = 0 + bandwidth = 0.0 for stream_type in (STREAM_TYPE_MAIN, STREAM_TYPE_SUB): - try: - # Subquery to get last 100 segments, then average their bandwidth - last_100 = ( - Recordings.select(bandwidth_equation.alias("bw")) - .where( - Recordings.camera == camera, - Recordings.segment_size > 0, - Recordings.stream_type == stream_type, - ) - .order_by(Recordings.start_time.desc()) - .limit(100) - .alias("recent") + avg_bw = self._recent_stream_bandwidth(camera, stream_type, 100) + if avg_bw is None: + # the recent window can be all zero-size ingest + # glitches; look further back before concluding + # the stream writes nothing + avg_bw = self._recent_stream_bandwidth( + camera, stream_type, 1000 ) - - bandwidth += round( - Recordings.select(fn.AVG(SQL("bw"))) - .from_(last_100) - .scalar() - * 3600, - 2, - ) - except TypeError: - pass + if avg_bw is not None: + bandwidth += round(avg_bw * 3600, 2) bandwidth = round(bandwidth, 2) diff --git a/frigate/test/http_api/test_http_media.py b/frigate/test/http_api/test_http_media.py index 400ef7098f..0d385978a2 100644 --- a/frigate/test/http_api/test_http_media.py +++ b/frigate/test/http_api/test_http_media.py @@ -119,7 +119,6 @@ class TestHttpMedia(BaseTestHttp): ] == expected_clips expected_durations = [clip[2] for clip in expected_clips] assert vod["durations"] == expected_durations - assert vod["segment_duration"] == max(expected_durations) def test_recordings_summary_across_dst_spring_forward(self): """ @@ -481,6 +480,129 @@ class TestHttpMedia(BaseTestHttp): assert "2024-03-10" in summary assert summary["2024-03-10"] is True + def test_recordings_summary_includes_sub_only_days(self): + """ + A day covered only by sub-stream rows still gets a day marker. + + Retention can expire main rows while keeping sub history, so the + calendar must not filter by stream type. + """ + march_9_utc = datetime(2024, 3, 9, 12, 0, 0, tzinfo=UTC).timestamp() + march_10_utc = datetime(2024, 3, 10, 12, 0, 0, tzinfo=UTC).timestamp() + + with AuthTestClient(self.app) as client: + self._insert_recording( + "main_march_9", march_9_utc, march_9_utc + 3600, stream_type="main" + ) + self._insert_recording( + "sub_march_10", march_10_utc, march_10_utc + 3600, stream_type="sub" + ) + + response = client.get( + "/recordings/summary", params={"timezone": "utc", "cameras": "all"} + ) + + assert response.status_code == 200 + summary = response.json() + assert len(summary) == 2 + assert summary["2024-03-09"] is True + assert summary["2024-03-10"] is True + + def test_recordings_summary_sparse_days_across_large_gap(self): + """ + Only recorded days are reported when a large empty gap separates them. + """ + early = datetime(2023, 1, 5, 12, 0, 0, tzinfo=UTC).timestamp() + late = datetime(2024, 3, 10, 12, 0, 0, tzinfo=UTC).timestamp() + + with AuthTestClient(self.app) as client: + self._insert_recording("early_day", early, early + 3600) + self._insert_recording("late_day", late, late + 3600) + + response = client.get( + "/recordings/summary", params={"timezone": "utc", "cameras": "all"} + ) + + assert response.status_code == 200 + summary = response.json() + assert summary == {"2023-01-05": True, "2024-03-10": True} + + def test_recordings_unavailable_merges_cameras(self): + """ + Gaps are computed against the union of all requested cameras' coverage. + """ + + async def allow_both_cameras(request: Request): + return ["front_door", "back_door"] + + self.app.dependency_overrides[get_allowed_cameras_for_filter] = ( + allow_both_cameras + ) + + with AuthTestClient(self.app) as client: + for id, camera, start, end in [ + ("front_a", "front_door", 1000, 1100), + ("front_b", "front_door", 1200, 1300), + ("back_a", "back_door", 1100, 1160), + ]: + Recordings.insert( + id=id, + path=f"/media/recordings/{id}.mp4", + camera=camera, + start_time=start, + end_time=end, + duration=end - start, + motion=0, + objects=0, + ).execute() + + response = client.get( + "/recordings/unavailable", + params={ + "after": 1000, + "before": 1300, + "scale": 10, + "cameras": "front_door,back_door", + }, + ) + assert response.status_code == 200 + assert response.json() == [{"start_time": 1160, "end_time": 1200}] + + # single camera: back_door alone leaves both edges uncovered + response = client.get( + "/recordings/unavailable", + params={ + "after": 1000, + "before": 1300, + "scale": 10, + "cameras": "back_door", + }, + ) + assert response.status_code == 200 + assert response.json() == [ + {"start_time": 1000, "end_time": 1100}, + {"start_time": 1160, "end_time": 1300}, + ] + + def test_recordings_summary_day_attribution_by_start_time(self): + """ + A recording spanning midnight marks only its start day. + """ + # starts 23:30 March 9, ends 00:30 March 10 (UTC) + start = datetime(2024, 3, 9, 23, 30, 0, tzinfo=UTC).timestamp() + + with AuthTestClient(self.app) as client: + self._insert_recording("midnight_span", start, start + 3600) + + response = client.get( + "/recordings/summary", params={"timezone": "utc", "cameras": "all"} + ) + + assert response.status_code == 200 + summary = response.json() + assert len(summary) == 1 + assert summary["2024-03-09"] is True + def _insert_recording( self, id: str, @@ -585,7 +707,6 @@ class TestHttpMedia(BaseTestHttp): "/media/recordings/main_1.mp4", "/media/recordings/main_2.mp4", ] - assert body["segment_duration"] == 10000 def test_vod_merges_intervals_split_by_other_stream_boundaries(self): """A recording spanning several coverage intervals stays one clip. @@ -790,7 +911,6 @@ class TestHttpMedia(BaseTestHttp): assert "initialClipIndex" not in body assert body["consistentSequenceMediaInfo"] is True assert body["durations"] == [10000, 10000] - assert body["segment_duration"] == 10000 assert len(body["sequences"]) == 1 clips = body["sequences"][0]["clips"] assert [c["path"] for c in clips] == [ @@ -853,8 +973,8 @@ class TestHttpMedia(BaseTestHttp): A long sub-only stretch means the merged sequence mixes muxed main files and audio-less sub files. Track-PRESENCE mixing across discontinuities is unproven in MSE, so every clip is - stripped to video tracks rather than the tracks being carried - through the discontinuity the stream mix already forces. + stripped to video tracks (the cross-stream hand-off still serves + the range in discontinuity mode). """ with AuthTestClient(self.app) as client: self._insert_recording("main_1", 1000, 1010, "main", has_audio=True) @@ -1240,7 +1360,8 @@ class TestHttpMedia(BaseTestHttp): """Uniformly-unknown rows (the pre-feature case) are never stripped. Legacy rows have has_audio NULL and audio_rate NULL; they share a - single signature, so no audio policy fires and audio plays. + single signature, so audio plays (the cross-stream hand-off still + serves the range in discontinuity mode). """ with AuthTestClient(self.app) as client: self._insert_recording("main_1", 1000, 1010, "main") diff --git a/frigate/test/test_record_cleanup_sub.py b/frigate/test/test_record_cleanup_sub.py index 0ae8caa270..edbae0f8e3 100644 --- a/frigate/test/test_record_cleanup_sub.py +++ b/frigate/test/test_record_cleanup_sub.py @@ -49,14 +49,19 @@ class TestRecordingCleanupSubRetention(unittest.TestCase): return RecordingCleanup(config, MagicMock()) def _insert_recording( - self, id: str, stream_type: str, age_days: float, motion: int = 0 + self, + id: str, + stream_type: str, + age_days: float, + motion: int = 0, + camera: str = "front_door", ) -> None: end_time = ( datetime.datetime.now() - datetime.timedelta(days=age_days) ).timestamp() Recordings.create( id=id, - camera="front_door", + camera=camera, path=f"/media/frigate/recordings/{id}.mp4", start_time=end_time - 10, end_time=end_time, @@ -208,3 +213,41 @@ class TestRecordingCleanupSubRetention(unittest.TestCase): assert Recordings.get_or_none(Recordings.id == "s_old") is None assert Recordings.get_or_none(Recordings.id == "s_new") is not None + + def test_deleted_camera_recordings_expire(self): + # rows for a camera no longer in the config expire by the GLOBAL + # record retention window; newer orphan rows and configured-camera + # rows are untouched by the deleted-cameras sweep + config = FrigateConfig( + **{ + "mqtt": {"host": "mqtt"}, + "record": {"continuous": {"days": 7}}, + "cameras": { + "front_door": { + "ffmpeg": { + "inputs": [ + { + "path": "rtsp://10.0.0.1:554/video", + "roles": ["detect", "record"], + }, + ] + }, + "record": {"enabled": True, "continuous": {"days": 7}}, + } + }, + } + ) + cleanup = RecordingCleanup(config, MagicMock()) + self._insert_recording("gone_old", "main", 10, camera="removed_cam") + self._insert_recording("gone_new", "main", 5, camera="removed_cam") + self._insert_recording("gone_blank", "main", 10, camera="") + self._insert_recording("kept", "main", 5) + + cleanup.expire_recordings() + + assert Recordings.get_or_none(Recordings.id == "gone_old") is None + assert Recordings.get_or_none(Recordings.id == "gone_new") is not None + # empty-string camera names sort before every real name and must + # still be enumerated by the sweep + assert Recordings.get_or_none(Recordings.id == "gone_blank") is None + assert Recordings.get_or_none(Recordings.id == "kept") is not None diff --git a/frigate/test/test_recording_coverage.py b/frigate/test/test_recording_coverage.py index 1ade707b50..00ce79865b 100644 --- a/frigate/test/test_recording_coverage.py +++ b/frigate/test/test_recording_coverage.py @@ -303,3 +303,74 @@ class TestVodManifestPolicy(CoverageDbTestCase): mapping = self._mapping(1000.0, 1020.0, stream="sub") assert mapping["discontinuity"] is False assert "initialClipIndex" not in mapping + + +class TestClipKeyFrames(unittest.TestCase): + """Keyframe data emitted for nginx-vod segmentation.""" + + def _row(self, keyframes): + return SimpleNamespace( + path="/tmp/kf.mp4", + start_time=1000.0, + end_time=1010.0, + duration=10.0, + keyframes=keyframes, + ) + + def test_whole_file_clip_emits_keyframe_gaps(self): + keyframes = [0, 1000, 2000, 3000, 4000, 5000, 6000, 7000, 8000, 9000] + plan = plan_clip(self._row(keyframes), 1000.0, 1010.0) + assert plan.clip_from_ms is None + assert plan.key_frame_durations == [1000] * 9 + assert plan.first_key_frame_offset_ms == 0 + + def test_mid_file_clip_offsets_are_clip_relative(self): + # start 3.5s in: clipFrom snaps back to the 3000ms keyframe, so + # the emitted offsets must be relative to that snapped start + keyframes = [0, 1000, 2000, 3000, 4000, 5000, 6000, 7000, 8000, 9000] + plan = plan_clip(self._row(keyframes), 1003.5, 1010.0) + assert plan.clip_from_ms == 3000 + assert plan.duration_ms == 7000 + assert plan.key_frame_durations == [1000] * 6 + assert plan.first_key_frame_offset_ms == 0 + + def test_no_index_reports_none(self): + plan = plan_clip(self._row(None), 1000.0, 1010.0) + assert plan.key_frame_durations is None + + def test_single_keyframe_in_range_reports_none(self): + # one cut point cannot split anything; the manifest falls back + # to a whole-clip declaration + plan = plan_clip(self._row([0]), 1000.0, 1010.0) + assert plan.key_frame_durations is None + + def test_keyframe_on_clip_end_excluded(self): + # a keyframe landing exactly on the clip end would only declare + # a zero-length tail segment + plan = plan_clip(self._row([0, 5000, 10000]), 1000.0, 1010.0) + assert plan.key_frame_durations == [5000] + + +class TestVodManifestKeyFrames(CoverageDbTestCase): + """Mapping emission of real vs placeholder keyFrameDurations.""" + + def _clips(self, start, end): + response = asyncio.run(_vod_response("front_door", start, end)) + return json.loads(response.body)["sequences"][0]["clips"] + + def test_indexed_row_emits_real_gaps(self): + self._insert( + "m1", + 1000.0, + 1010.0, + "main", + keyframes=[0, 2000, 4000, 6000, 8000], + ) + clips = self._clips(1000.0, 1010.0) + assert clips[0]["keyFrameDurations"] == [2000, 2000, 2000, 2000] + assert "firstKeyFrameOffset" not in clips[0] + + def test_unindexed_row_keeps_whole_clip_placeholder(self): + self._insert("m1", 1000.0, 1010.0, "main") + clips = self._clips(1000.0, 1010.0) + assert clips[0]["keyFrameDurations"] == [10000] diff --git a/frigate/test/test_storage.py b/frigate/test/test_storage.py index 4ae5715cab..846b7d2ab7 100644 --- a/frigate/test/test_storage.py +++ b/frigate/test/test_storage.py @@ -139,6 +139,41 @@ class TestHttp(unittest.TestCase): "front_door": {"bandwidth": 0, "needs_refresh": True}, } + def test_segment_calculations_with_recent_zero_segments(self): + """A run of recent zero-size segments must not zero out the bandwidth. + + Older nonzero segments still describe the camera's real write rate. + """ + config = FrigateConfig(**self.minimal_config) + storage = StorageMaintainer(config, MagicMock()) + + time_keep = datetime.datetime.now().timestamp() + for i in range(10): + _insert_mock_recording( + f"nonzero_{i}.frontdoor", + os.path.join(self.test_dir, f"nonzero_{i}.tmp"), + time_keep + i * 10, + time_keep + i * 10 + 10, + camera="front_door", + seg_size=4, + seg_dur=10, + ) + for i in range(100): + _insert_mock_recording( + f"zero_{i}.frontdoor", + os.path.join(self.test_dir, f"zero_{i}.tmp"), + time_keep + 1000 + i * 10, + time_keep + 1000 + i * 10 + 10, + camera="front_door", + seg_size=0, + seg_dur=10, + ) + + storage.calculate_camera_bandwidth() + assert storage.camera_storage_stats == { + "front_door": {"bandwidth": 1440, "needs_refresh": True}, + } + def test_storage_cleanup(self): """Ensure that all recordings are cleaned up when necessary.""" config = FrigateConfig(**self.minimal_config) diff --git a/frigate/util/recording_coverage.py b/frigate/util/recording_coverage.py index f34f8a95e6..d8bbd55ad6 100644 --- a/frigate/util/recording_coverage.py +++ b/frigate/util/recording_coverage.py @@ -310,11 +310,18 @@ class ClipPlan: clip_from_ms is the keyframe-snapped clipFrom, or None when the whole file is served. skipped means the clip is omitted from the manifest. + + key_frame_durations / first_key_frame_offset_ms carry clip-relative + keyframe data for nginx-vod sub-file segmentation; None means no + usable index, and the manifest declares one whole-clip segment (the + only safe cut without keyframe knowledge). """ clip_from_ms: int | None duration_ms: int skipped: bool + key_frame_durations: list[int] | None = None + first_key_frame_offset_ms: int = 0 def plan_clip(row: Any, start: float, end: float) -> ClipPlan: @@ -378,7 +385,38 @@ def plan_clip(row: Any, start: float, end: float) -> ClipPlan: logger.warning(f"Recording clip is missing or empty: {row.path}") return ClipPlan(None, 0, True) - return ClipPlan(clip_from, duration, False) + return ClipPlan( + clip_from, + duration, + False, + *_clip_key_frames(row.keyframes, clip_from, duration), + ) + + +def _clip_key_frames( + keyframes: Any, clip_from: int | None, duration: int +) -> tuple[list[int] | None, int]: + """Clip-relative keyframe gaps and first offset for a served range. + + nginx-vod validates firstKeyFrameOffset against the clip duration, + so offsets are clip-relative. A keyframe exactly on the clip end is + excluded (it would declare a zero-length tail), and fewer than two + keyframes returns (None, 0): one cut point cannot split anything. + """ + if not keyframes: + return None, 0 + + clip_start = clip_from if clip_from is not None else 0 + relative = [ + int(k) - clip_start + for k in keyframes + if clip_start <= k < clip_start + duration + ] + + if len(relative) < 2: + return None, 0 + + return [b - a for a, b in zip(relative, relative[1:])], relative[0] def realized_timeline( diff --git a/migrations/037_add_recordings_stream_index.py b/migrations/037_add_recordings_stream_index.py new file mode 100644 index 0000000000..4cb8f59ae2 --- /dev/null +++ b/migrations/037_add_recordings_stream_index.py @@ -0,0 +1,43 @@ +"""Peewee migrations -- 037_add_recordings_stream_index.py. + +Some examples (model - class or model name):: + + > Model = migrator.orm['model_name'] # Return model in current state by name + + > migrator.sql(sql) # Run custom SQL + > migrator.python(func, *args, **kwargs) # Run python code + > migrator.create_model(Model) # Create a model (could be used as decorator) + > migrator.remove_model(model, cascade=True) # Remove a model + > migrator.add_fields(model, **fields) # Add fields to a model + > migrator.change_fields(model, **fields) # Change fields + > migrator.remove_fields(model, *field_names, cascade=True) + > migrator.rename_field(model, old_field_name, new_field_name) + > migrator.rename_table(model, new_table_name) + > migrator.add_index(model, *col_names, unique=False) + > migrator.drop_index(model, *col_names) + > migrator.add_not_null(model, *field_names) + > migrator.drop_not_null(model, *field_names) + > migrator.add_default(model, field_name, default) + +""" + +import peewee as pw + +SQL = pw.SQL + + +def migrate(migrator, database, fake=False, **kwargs): + # time-ordered per-stream lookups (bandwidth estimation, pinned-stream + # queries) need this ordering; without it, a per-stream ORDER BY + # start_time query on a camera with no rows for that stream walks the + # camera's entire history via per-row table lookups + migrator.sql( + 'CREATE INDEX IF NOT EXISTS "recordings_camera_stream_type_start_time" ON "recordings" ("camera", "stream_type", "start_time" DESC)' + ) + # the 036 index is a strict prefix of the one above, so keeping it + # would only add write amplification on the hottest table + migrator.sql('DROP INDEX IF EXISTS "recordings_camera_stream_type"') + + +def rollback(migrator, database, fake=False, **kwargs): + pass diff --git a/web/src/components/player/dynamic/DynamicVideoPlayer.tsx b/web/src/components/player/dynamic/DynamicVideoPlayer.tsx index a046235b47..62b95b313f 100644 --- a/web/src/components/player/dynamic/DynamicVideoPlayer.tsx +++ b/web/src/components/player/dynamic/DynamicVideoPlayer.tsx @@ -45,6 +45,16 @@ import { useUserPersistence } from "@/hooks/use-user-persistence"; // a longer buffer cheap and it rides out connection variance better const SUB_STREAM_BUFFER_LENGTH_S = 30; +// seeks rebuild the source starting at the seek target so the vod +// bootstrap segment ladder applies to every seek; quantizing keeps +// seek URLs repeatable for nginx's mapping/response caches +const SOURCE_START_GRID_S = 10; + +// seeks beyond this bridge the source load with the preview player (the +// held video frame is pre-seek content); continuations (quality switch, +// natural chunk advance) keep the held frame +const REPOSITION_PREVIEW_THRESHOLD_S = 2; + /** * Dynamically switches between video playback and scrubbing preview player. */ @@ -157,6 +167,11 @@ export default function DynamicVideoPlayer({ useEffect(() => { if (!isScrubbing) { + // never overwrite a pending timer: an orphaned one escapes + // onPlaying's clearTimeout and flashes loading mid-playback + if (loadingTimeoutRef.current) { + clearTimeout(loadingTimeoutRef.current); + } loadingTimeoutRef.current = setTimeout(() => setIsLoading(true), 1000); } @@ -179,6 +194,85 @@ export default function DynamicVideoPlayer({ // playlist, so the playback effect skips its loading indicator const modelOnlyUpdateRef = useRef(false); + // re-anchors quality-switch rebuilds and classifies rebuilds as + // repositioning vs continuation + const lastPlayedTimestampRef = useRef(undefined); + + // start of the current source window within the chunk; undefined plays + // from the chunk start. Follows explicit seeks (startTimestamp), + // cleared when a chunk change leaves the seek target behind + const [sourceAfter, setSourceAfter] = useState(undefined); + + // adjusted during render: an effect lands one commit late, briefly + // painting the stale video frame between drag preview and load bridge + const nextSourceAfter = + startTimestamp !== undefined && + startTimestamp > timeRange.after && + startTimestamp < timeRange.before + ? Math.max( + timeRange.after, + Math.floor(startTimestamp / SOURCE_START_GRID_S) * + SOURCE_START_GRID_S, + ) + : undefined; + + if (nextSourceAfter !== sourceAfter) { + setSourceAfter(nextSourceAfter); + + const lastPlayed = lastPlayedTimestampRef.current; + if ( + !isLoading && + startTimestamp !== undefined && + lastPlayed !== undefined && + Math.abs(startTimestamp - lastPlayed) > REPOSITION_PREVIEW_THRESHOLD_S + ) { + setIsLoading(true); + } + } + + // the release anchor lands one commit after isScrubbing flips false + // (the view writes playbackStart from an effect), which would flash + // the stale frame; hold one commit. The clearing effect runs before + // the parent's release effect, so clear and anchor batch into one render + const [releaseHold, setReleaseHold] = useState(false); + const [wasScrubbing, setWasScrubbing] = useState(isScrubbing); + + if (isScrubbing !== wasScrubbing) { + setWasScrubbing(isScrubbing); + if (!isScrubbing) { + setReleaseHold(true); + } + } + + useEffect(() => { + if (releaseHold) { + setReleaseHold(false); + } + }, [releaseHold]); + + const recordingParams = useMemo( + () => ({ + before: timeRange.before, + // clamp: during the adjust-render pass this memo can evaluate with + // the previous sourceAfter against a new timeRange + after: + sourceAfter !== undefined && + sourceAfter > timeRange.after && + sourceAfter < timeRange.before + ? sourceAfter + : timeRange.after, + timelines: true, + }), + [timeRange, sourceAfter], + ); + + // the window the current source covers; the seek model, in-range + // checks, and stale-report guard use this, not the chunk timeRange + const sourceTimeRange = useMemo( + () => ({ after: recordingParams.after, before: recordingParams.before }), + [recordingParams], + ); + const onPlayerLoaded = useCallback(() => { sourceLoadedRef.current = true; governorRef.current?.sourceLoadEnded(); @@ -189,9 +283,9 @@ export default function DynamicVideoPlayer({ return; } - // an anchor outside this chunk is stale (e.g. a natural clip + // an anchor outside this source window is stale (e.g. a natural clip // advance); the playlist already starts where playback should - if (anchor < timeRange.after || anchor > timeRange.before) { + if (anchor < sourceTimeRange.after || anchor > sourceTimeRange.before) { return; } @@ -199,11 +293,7 @@ export default function DynamicVideoPlayer({ // start it: a mid-drag chunk prefetch can audibly blip before // onPlaying pauses it. The release seek starts playback controller.seekToTimestamp(anchor, !isScrubbing); - }, [controller, timeRange, isScrubbing]); - - // used to re-anchor the source when an auto quality switch rebuilds - // the playlist mid-playback - const lastPlayedTimestampRef = useRef(undefined); + }, [controller, sourceTimeRange, isScrubbing]); // the range the controller's playback model was last built for; while // a chunk change awaits its coverage, the outgoing source reports @@ -223,10 +313,10 @@ export default function DynamicVideoPlayer({ return; } - // drop reports until the controller's model matches this chunk + // drop reports until the controller's model matches this source if ( - modelTimeRangeRef.current?.after !== timeRange.after || - modelTimeRangeRef.current?.before !== timeRange.before + modelTimeRangeRef.current?.after !== sourceTimeRange.after || + modelTimeRangeRef.current?.before !== sourceTimeRange.before ) { return; } @@ -249,7 +339,7 @@ export default function DynamicVideoPlayer({ isBuffering, isLoading, isScrubbing, - timeRange, + sourceTimeRange, ], ); @@ -310,14 +400,6 @@ export default function DynamicVideoPlayer({ // state of playback player - const recordingParams = useMemo( - () => ({ - before: timeRange.before, - after: timeRange.after, - timelines: true, - }), - [timeRange], - ); const { data: coverage } = useSWR( [`${camera}/recordings/coverage`, recordingParams], { revalidateOnFocus: false }, @@ -628,8 +710,8 @@ export default function DynamicVideoPlayer({ const anchorTimestamp = qualityChanged && lastPlayed !== undefined && - lastPlayed >= timeRange.after && - lastPlayed <= timeRange.before + lastPlayed >= recordingParams.after && + lastPlayed <= recordingParams.before ? lastPlayed : startTimestamp; sourceAnchorRef.current = anchorTimestamp; @@ -679,6 +761,10 @@ export default function DynamicVideoPlayer({ HTMLMediaElement.HAVE_CURRENT_DATA; if (!modelOnlyUpdate) { + // an overwritten pending timer would escape onPlaying's clearTimeout + if (loadingTimeoutRef.current) { + clearTimeout(loadingTimeoutRef.current); + } loadingTimeoutRef.current = setTimeout( () => (hasDecodedFrame ? setIsBuffering(true) : setIsLoading(true)), 1000, @@ -687,9 +773,9 @@ export default function DynamicVideoPlayer({ controller.newPlayback({ recordings: recordings ?? [], - timeRange, + timeRange: sourceTimeRange, }); - modelTimeRangeRef.current = timeRange; + modelTimeRangeRef.current = sourceTimeRange; // we only want this to change when controller or recordings update // eslint-disable-next-line react-hooks/exhaustive-deps @@ -733,7 +819,7 @@ export default function DynamicVideoPlayer({