diff --git a/frigate/record/maintainer.py b/frigate/record/maintainer.py index 7f9dbc19da..9ce504e34f 100644 --- a/frigate/record/maintainer.py +++ b/frigate/record/maintainer.py @@ -388,6 +388,22 @@ class RecordingMaintainer(threading.Thread): RecordingsDataTypeEnum.valid.value, ) + # assume that empty means the relevant recording info has not been received yet + camera_info = self.object_recordings_info[camera] + most_recently_processed_frame_time = ( + camera_info[-1][0] if len(camera_info) > 0 else 0 + ) + + # ensure delayed segment info does not lead to lost segments, every + # retention decision below depends on complete stats for the segment + if ( + datetime.datetime.fromtimestamp( + most_recently_processed_frame_time + ).astimezone(datetime.UTC) + < end_time + ): + return None + record_config = self.config.cameras[camera].record segment_stats: SegmentInfo | None = None highest = None @@ -401,37 +417,22 @@ class RecordingMaintainer(threading.Thread): # we should first just check if this segment matches that # and avoid any DB calls if highest is not None: - # assume that empty means the relevant recording info has not been received yet - camera_info = self.object_recordings_info[camera] - most_recently_processed_frame_time = ( - camera_info[-1][0] if len(camera_info) > 0 else 0 + record_mode = ( + RetainModeEnum.all if highest == "continuous" else RetainModeEnum.motion ) + segment_stats = self.segment_stats(camera, start_time, end_time) - # ensure delayed segment info does not lead to lost segments - if ( - datetime.datetime.fromtimestamp( - most_recently_processed_frame_time - ).astimezone(datetime.UTC) - >= end_time - ): - record_mode = ( - RetainModeEnum.all - if highest == "continuous" - else RetainModeEnum.motion + # Here we only check if we should move the segment based on non-object recording retention + # we will always want to check for overlapping review items below before dropping the segment + if not segment_stats.should_discard_segment(record_mode): + return await self.move_segment( + camera, + start_time, + end_time, + duration, + cache_path, + segment_stats, ) - segment_stats = self.segment_stats(camera, start_time, end_time) - - # Here we only check if we should move the segment based on non-object recording retention - # we will always want to check for overlapping review items below before dropping the segment - if not segment_stats.should_discard_segment(record_mode): - return await self.move_segment( - camera, - start_time, - end_time, - duration, - cache_path, - segment_stats, - ) # we fell through the continuous / motion check, so we need to check the review items # if the cached segment overlaps with the review items: @@ -487,10 +488,6 @@ class RecordingMaintainer(threading.Thread): # continuous/motion retention (either disabled or segment_stats said # discard), so waiting longer just fills the cache. else: - camera_info = self.object_recordings_info[camera] - most_recently_processed_frame_time = ( - camera_info[-1][0] if len(camera_info) > 0 else 0 - ) retain_cutoff = datetime.datetime.fromtimestamp( most_recently_processed_frame_time - record_config.event_pre_capture ).astimezone(datetime.UTC) diff --git a/frigate/test/test_maintainer.py b/frigate/test/test_maintainer.py index 715cd5a1a1..5d7067ba7e 100644 --- a/frigate/test/test_maintainer.py +++ b/frigate/test/test_maintainer.py @@ -1,7 +1,7 @@ import datetime import sys import unittest -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, patch # Mock complex imports before importing maintainer, saving originals so we can # restore them after import and avoid polluting sys.modules for other tests. @@ -16,7 +16,7 @@ for name in _MOCKED_MODULES: sys.modules[name] = MagicMock() # Now import the class under test -from frigate.config import FrigateConfig # noqa: E402 +from frigate.config import FrigateConfig, RetainModeEnum # noqa: E402 from frigate.record.maintainer import RecordingMaintainer # noqa: E402 # Restore original modules (or remove mock if there was no original) @@ -115,6 +115,70 @@ class TestMaintainer(unittest.IsolatedAsyncioTestCase): self.assertIsNone(result) maintainer.drop_segment.assert_called_once_with(cache_path) + async def test_defers_review_overlap_segment_until_metadata_catches_up(self): + # Regression: a segment overlapping an active_objects review must not + # be dropped while detection metadata lags behind the segment end, + # the missing frames may hold the active objects (or continuous + # retention would keep it anyway). + config = MagicMock(spec=FrigateConfig) + + camera_config = MagicMock() + camera_config.record.enabled = True + camera_config.record.continuous.days = 7 + camera_config.record.motion.days = 0 + camera_config.record.alerts.retain.mode = RetainModeEnum.active_objects + camera_config.record.get_review_pre_capture.return_value = 5 + camera_config.record.get_review_post_capture.return_value = 5 + config.cameras = {"test_cam": camera_config} + + stop_event = MagicMock() + maintainer = RecordingMaintainer(config, stop_event) + + now = datetime.datetime.now(datetime.UTC) + start_time = now - datetime.timedelta(seconds=20) + end_time = now - datetime.timedelta(seconds=10) + cache_path = "/tmp/cache/test_cam@20260417150000+0000.mp4" + + maintainer.end_time_cache = {cache_path: (end_time, 10.0)} + # Metadata has only reached partway into the segment. + maintainer.object_recordings_info["test_cam"] = [ + (end_time.timestamp() - 8, [], [], []) + ] + maintainer.audio_recordings_info["test_cam"] = [] + + maintainer.drop_segment = MagicMock() + maintainer.move_segment = AsyncMock(return_value=None) + maintainer.recordings_publisher = MagicMock() + + review = MagicMock() + review.severity = "alert" + review.start_time = start_time.timestamp() - 30 + review.end_time = None + + result = await maintainer.validate_and_move_segment( + "test_cam", + reviews=[review], + recording={"start_time": start_time, "cache_path": cache_path}, + ) + + self.assertIsNone(result) + maintainer.drop_segment.assert_not_called() + maintainer.move_segment.assert_not_awaited() + + # Once metadata passes the segment end, continuous retention keeps it. + maintainer.object_recordings_info["test_cam"].append( + (now.timestamp(), [], [], []) + ) + + await maintainer.validate_and_move_segment( + "test_cam", + reviews=[review], + recording={"start_time": start_time, "cache_path": cache_path}, + ) + + maintainer.drop_segment.assert_not_called() + maintainer.move_segment.assert_awaited_once() + async def test_expire_stale_recordings_info_drops_only_absent_cameras(self): config = MagicMock(spec=FrigateConfig) config.cameras = {}