Improve recording retention logic (#20506)

* Change default event retention

* Update docs

* Handle both record and event record

* Catch edge case

* Undo motion change and improve motion behavior

* fix typo

* Remove record retention banner

* Remove unused

* Fix tests

docs/docs/configuration/record.md

@@ -25,16 +25,16 @@ record:
   alerts:
     retain:
       days: 30
-      mode: motion
+      mode: all
   detections:
     retain:
       days: 30
-      mode: motion
+      mode: all
 ```
 
 ### Reduced storage: Only saving video when motion is detected
 
-In order to reduce storage requirements, you can adjust your config to only retain video where motion was detected.
+In order to reduce storage requirements, you can adjust your config to only retain video where motion / activity was detected.
 
 ```yaml
 record:
@@ -53,7 +53,7 @@ record:
 
 ### Minimum: Alerts only
 
-If you only want to retain video that occurs during a tracked object, this config will discard video unless an alert is ongoing.
+If you only want to retain video that occurs during activity caused by tracked object(s), this config will discard video unless an alert is ongoing.
 
 ```yaml
 record:

docs/docs/configuration/reference.md

@@ -537,7 +537,7 @@ record:
     # Optional: Retention settings for recordings of alerts
     retain:
       # Required: Retention days (default: shown below)
-      days: 14
+      days: 10
       # Optional: Mode for retention. (default: shown below)
       #   all - save all recording segments for alerts regardless of activity
       #   motion - save all recordings segments for alerts with any detected motion
@@ -557,7 +557,7 @@ record:
     # Optional: Retention settings for recordings of detections
     retain:
       # Required: Retention days (default: shown below)
-      days: 14
+      days: 10
       # Optional: Mode for retention. (default: shown below)
       #   all - save all recording segments for detections regardless of activity
       #   motion - save all recordings segments for detections with any detected motion

frigate/record/cleanup.py

@@ -180,7 +180,11 @@ class RecordingCleanup(threading.Thread):
             # Delete recordings outside of the retention window or based on the retention mode
             if (
                 not keep
-                or (mode == RetainModeEnum.motion and recording.motion == 0)
+                or (
+                    mode == RetainModeEnum.motion
+                    and recording.motion == 0
+                    and recording.objects == 0
+                )
                 or (mode == RetainModeEnum.active_objects and recording.objects == 0)
             ):
                 Path(recording.path).unlink(missing_ok=True)

frigate/record/maintainer.py

@@ -57,14 +57,25 @@ class SegmentInfo:
         self.average_dBFS = average_dBFS
 
     def should_discard_segment(self, retain_mode: RetainModeEnum) -> bool:
-        return (
-            retain_mode == RetainModeEnum.motion
-            and self.motion_count == 0
-            and self.average_dBFS == 0
-        ) or (
-            retain_mode == RetainModeEnum.active_objects
-            and self.active_object_count == 0
-        )
+        keep = False
+
+        # all mode should never discard
+        if retain_mode == RetainModeEnum.all:
+            keep = True
+
+        # motion mode should keep if motion or audio is detected
+        if (
+            not keep
+            and retain_mode == RetainModeEnum.motion
+            and (self.motion_count > 0 or self.average_dBFS > 0)
+        ):
+            keep = True
+
+        # active objects mode should keep if any active objects are detected
+        if not keep and self.active_object_count > 0:
+            keep = True
+
+        return not keep
 
 
 class RecordingMaintainer(threading.Thread):
@@ -348,63 +359,10 @@ class RecordingMaintainer(threading.Thread):
         elif record_config.motion.days > 0:
             highest = "motion"
 
-        # continuous / motion recording is not enabled
-        if highest is None:
-            # if the cached segment overlaps with the review items:
-            overlaps = False
-            for review in reviews:
-                severity = SeverityEnum[review.severity]
-
-                # if the review item starts in the future, stop checking review items
-                # and remove this segment
-                if (
-                    review.start_time - record_config.get_review_pre_capture(severity)
-                ) > end_time.timestamp():
-                    overlaps = False
-                    break
-
-                # if the review item is in progress or ends after the recording starts, keep it
-                # and stop looking at review items
-                if (
-                    review.end_time is None
-                    or (
-                        review.end_time
-                        + record_config.get_review_post_capture(severity)
-                    )
-                    >= start_time.timestamp()
-                ):
-                    overlaps = True
-                    break
-
-            if overlaps:
-                record_mode = (
-                    record_config.alerts.retain.mode
-                    if review.severity == "alert"
-                    else record_config.detections.retain.mode
-                )
-                # move from cache to recordings immediately
-                return await self.move_segment(
-                    camera,
-                    start_time,
-                    end_time,
-                    duration,
-                    cache_path,
-                    record_mode,
-                )
-            # if it doesn't overlap with an review item, go ahead and drop the segment
-            # if it ends more than the configured pre_capture for the camera
-            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.timezone.utc)
-                if end_time < retain_cutoff:
-                    self.drop_segment(cache_path)
-        # continuous / motion is enabled
-        else:
+        # if we have continuous or motion recording enabled
+        # 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 = (
@@ -427,6 +385,59 @@ class RecordingMaintainer(threading.Thread):
                     camera, start_time, end_time, duration, cache_path, record_mode
                 )
 
+        # we fell through the continuous / motion check, so we need to check the review items
+        # if the cached segment overlaps with the review items:
+        overlaps = False
+        for review in reviews:
+            severity = SeverityEnum[review.severity]
+
+            # if the review item starts in the future, stop checking review items
+            # and remove this segment
+            if (
+                review.start_time - record_config.get_review_pre_capture(severity)
+            ) > end_time.timestamp():
+                overlaps = False
+                break
+
+            # if the review item is in progress or ends after the recording starts, keep it
+            # and stop looking at review items
+            if (
+                review.end_time is None
+                or (review.end_time + record_config.get_review_post_capture(severity))
+                >= start_time.timestamp()
+            ):
+                overlaps = True
+                break
+
+        if overlaps:
+            record_mode = (
+                record_config.alerts.retain.mode
+                if review.severity == "alert"
+                else record_config.detections.retain.mode
+            )
+            # move from cache to recordings immediately
+            return await self.move_segment(
+                camera,
+                start_time,
+                end_time,
+                duration,
+                cache_path,
+                record_mode,
+            )
+        # if it doesn't overlap with an review item, go ahead and drop the segment
+        # if it ends more than the configured pre_capture for the camera
+        # BUT only if continuous/motion is NOT enabled (otherwise wait for processing)
+        elif highest is None:
+            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.timezone.utc)
+            if end_time < retain_cutoff:
+                self.drop_segment(cache_path)
+
     def segment_stats(
         self, camera: str, start_time: datetime.datetime, end_time: datetime.datetime
     ) -> SegmentInfo:

frigate/test/test_record_retention.py

@@ -12,11 +12,11 @@ class TestRecordRetention(unittest.TestCase):
         assert not segment_info.should_discard_segment(RetainModeEnum.motion)
         assert segment_info.should_discard_segment(RetainModeEnum.active_objects)
 
-    def test_object_should_keep_object_not_motion(self):
+    def test_object_should_keep_object_when_motion(self):
         segment_info = SegmentInfo(
             motion_count=0, active_object_count=1, region_count=0, average_dBFS=0
         )
-        assert segment_info.should_discard_segment(RetainModeEnum.motion)
+        assert not segment_info.should_discard_segment(RetainModeEnum.motion)
         assert not segment_info.should_discard_segment(RetainModeEnum.active_objects)
 
     def test_all_should_keep_all(self):

web/public/locales/en/views/live.json

@@ -165,8 +165,7 @@
       "all": "All",
       "motion": "Motion",
       "active_objects": "Active Objects"
-    },
-    "notAllTips": "Your {{source}} recording retention configuration is set to <code>mode: {{effectiveRetainMode}}</code>, so this on-demand recording will only keep segments with {{effectiveRetainModeName}}."
+    }
   },
   "editLayout": {
     "label": "Edit Layout",

web/src/views/live/LiveCameraView.tsx

@@ -108,7 +108,7 @@ import axios from "axios";
 import { toast } from "sonner";
 import { Toaster } from "@/components/ui/sonner";
 import { useIsAdmin } from "@/hooks/use-is-admin";
-import { Trans, useTranslation } from "react-i18next";
+import { useTranslation } from "react-i18next";
 import { useDocDomain } from "@/hooks/use-doc-domain";
 import PtzControlPanel from "@/components/overlay/PtzControlPanel";
 import ObjectSettingsView from "../settings/ObjectSettingsView";
@@ -712,42 +712,6 @@ export default function LiveCameraView({
   );
 }
 
-function OnDemandRetentionMessage({ camera }: { camera: CameraConfig }) {
-  const { t } = useTranslation(["views/live", "views/events"]);
-  const rankMap = { all: 0, motion: 1, active_objects: 2 };
-  const getValidMode = (retain?: { mode?: string }): keyof typeof rankMap => {
-    const mode = retain?.mode;
-    return mode && mode in rankMap ? (mode as keyof typeof rankMap) : "all";
-  };
-
-  const recordRetainMode = getValidMode(camera.record.retain);
-  const alertsRetainMode = getValidMode(camera.review.alerts.retain);
-
-  const effectiveRetainMode =
-    rankMap[alertsRetainMode] < rankMap[recordRetainMode]
-      ? recordRetainMode
-      : alertsRetainMode;
-
-  const source = effectiveRetainMode === recordRetainMode ? "camera" : "alerts";
-
-  return effectiveRetainMode !== "all" ? (
-    <div>
-      <Trans
-        ns="views/live"
-        values={{
-          source,
-          effectiveRetainMode,
-          effectiveRetainModeName: t(
-            "effectiveRetainMode.modes." + effectiveRetainMode,
-          ),
-        }}
-      >
-        effectiveRetainMode.notAllTips
-      </Trans>
-    </div>
-  ) : null;
-}
-
 type FrigateCameraFeaturesProps = {
   camera: CameraConfig;
   recordingEnabled: boolean;
@@ -841,11 +805,10 @@ function FrigateCameraFeatures({
         const toastId = toast.success(
           <div className="flex flex-col space-y-3">
             <div className="font-semibold">{t("manualRecording.started")}</div>
-            {!camera.record.enabled || camera.record.alerts.retain.days == 0 ? (
-              <div>{t("manualRecording.recordDisabledTips")}</div>
-            ) : (
-              <OnDemandRetentionMessage camera={camera} />
-            )}
+            {!camera.record.enabled ||
+              (camera.record.alerts.retain.days == 0 && (
+                <div>{t("manualRecording.recordDisabledTips")}</div>
+              ))}
           </div>,
           {
             position: "top-center",