Refactor event cleanup to consider review severity (#15415)

* Keep track of objects max review severity

* Refactor cleanup to split snapshots and clips

* Cleanup events based on review severity

* Cleanup review imports

* Don't catch detections

frigate/api/defs/query/review_query_parameters.py

@@ -3,7 +3,7 @@
 from pydantic import BaseModel
 from pydantic.json_schema import SkipJsonSchema
 
-from frigate.review.maintainer import SeverityEnum
+from frigate.review.types import SeverityEnum
 
 
 class ReviewQueryParams(BaseModel):

frigate/api/defs/response/review_response.py

@@ -3,7 +3,7 @@
 
 from pydantic import BaseModel, Json
 
-from frigate.review.maintainer import SeverityEnum
+from frigate.review.types import SeverityEnum
 
 
 class ReviewSegmentResponse(BaseModel):

frigate/api/review.py

@@ -26,7 +26,7 @@
 )
 from frigate.api.defs.tags import Tags
 from frigate.models import Recordings, ReviewSegment
-from frigate.review.maintainer import SeverityEnum
+from frigate.review.types import SeverityEnum
 from frigate.util.builtin import get_tz_modifiers
 
 logger = logging.getLogger(__name__)

frigate/events/cleanup.py

@@ -4,7 +4,6 @@
 import logging
 import os
 import threading
-from enum import Enum
 from multiprocessing.synchronize import Event as MpEvent
 from pathlib import Path
 
@@ -16,11 +15,6 @@
 logger = logging.getLogger(__name__)
 
 
-class EventCleanupType(str, Enum):
-    clips = "clips"
-    snapshots = "snapshots"
-
-
 CHUNK_SIZE = 50
 
 
@@ -67,30 +61,19 @@ def get_camera_labels(self, camera: str) -> list[Event]:
 
         return self.camera_labels[camera]["labels"]
 
-    def expire(self, media_type: EventCleanupType) -> list[str]:
+    def expire_snapshots(self) -> list[str]:
         ## Expire events from unlisted cameras based on the global config
-        if media_type == EventCleanupType.clips:
-            expire_days = max(
-                self.config.record.alerts.retain.days,
-                self.config.record.detections.retain.days,
-            )
-            file_extension = None  # mp4 clips are no longer stored in /clips
-            update_params = {"has_clip": False}
-        else:
-            retain_config = self.config.snapshots.retain
-            file_extension = "jpg"
-            update_params = {"has_snapshot": False}
+        retain_config = self.config.snapshots.retain
+        file_extension = "jpg"
+        update_params = {"has_snapshot": False}
 
         distinct_labels = self.get_removed_camera_labels()
 
         ## Expire events from cameras no longer in the config
         # loop over object types in db
         for event in distinct_labels:
             # get expiration time for this label
-            if media_type == EventCleanupType.snapshots:
-                expire_days = retain_config.objects.get(
-                    event.label, retain_config.default
-                )
+            expire_days = retain_config.objects.get(event.label, retain_config.default)
 
             expire_after = (
                 datetime.datetime.now() - datetime.timedelta(days=expire_days)
@@ -162,24 +145,17 @@ def expire(self, media_type: EventCleanupType) -> list[str]:
 
         ## Expire events from cameras based on the camera config
         for name, camera in self.config.cameras.items():
-            if media_type == EventCleanupType.clips:
-                expire_days = max(
-                    camera.record.alerts.retain.days,
-                    camera.record.detections.retain.days,
-                )
-            else:
-                retain_config = camera.snapshots.retain
+            retain_config = camera.snapshots.retain
 
             # get distinct objects in database for this camera
             distinct_labels = self.get_camera_labels(name)
 
             # loop over object types in db
             for event in distinct_labels:
                 # get expiration time for this label
-                if media_type == EventCleanupType.snapshots:
-                    expire_days = retain_config.objects.get(
-                        event.label, retain_config.default
-                    )
+                expire_days = retain_config.objects.get(
+                    event.label, retain_config.default
+                )
 
                 expire_after = (
                     datetime.datetime.now() - datetime.timedelta(days=expire_days)
@@ -206,19 +182,143 @@ def expire(self, media_type: EventCleanupType) -> list[str]:
                 for event in expired_events:
                     events_to_update.append(event.id)
 
-                    if media_type == EventCleanupType.snapshots:
-                        try:
-                            media_name = f"{event.camera}-{event.id}"
-                            media_path = Path(
-                                f"{os.path.join(CLIPS_DIR, media_name)}.{file_extension}"
-                            )
-                            media_path.unlink(missing_ok=True)
-                            media_path = Path(
-                                f"{os.path.join(CLIPS_DIR, media_name)}-clean.png"
-                            )
-                            media_path.unlink(missing_ok=True)
-                        except OSError as e:
-                            logger.warning(f"Unable to delete event images: {e}")
+                    try:
+                        media_name = f"{event.camera}-{event.id}"
+                        media_path = Path(
+                            f"{os.path.join(CLIPS_DIR, media_name)}.{file_extension}"
+                        )
+                        media_path.unlink(missing_ok=True)
+                        media_path = Path(
+                            f"{os.path.join(CLIPS_DIR, media_name)}-clean.png"
+                        )
+                        media_path.unlink(missing_ok=True)
+                    except OSError as e:
+                        logger.warning(f"Unable to delete event images: {e}")
+
+        # update the clips attribute for the db entry
+        for i in range(0, len(events_to_update), CHUNK_SIZE):
+            batch = events_to_update[i : i + CHUNK_SIZE]
+            logger.debug(f"Updating {update_params} for {len(batch)} events")
+            Event.update(update_params).where(Event.id << batch).execute()
+
+        return events_to_update
+
+    def expire_clips(self) -> list[str]:
+        ## Expire events from unlisted cameras based on the global config
+        expire_days = max(
+            self.config.record.alerts.retain.days,
+            self.config.record.detections.retain.days,
+        )
+        file_extension = None  # mp4 clips are no longer stored in /clips
+        update_params = {"has_clip": False}
+
+        # get expiration time for this label
+
+        expire_after = (
+            datetime.datetime.now() - datetime.timedelta(days=expire_days)
+        ).timestamp()
+        # grab all events after specific time
+        expired_events: list[Event] = (
+            Event.select(
+                Event.id,
+                Event.camera,
+            )
+            .where(
+                Event.camera.not_in(self.camera_keys),
+                Event.start_time < expire_after,
+                Event.retain_indefinitely == False,
+            )
+            .namedtuples()
+            .iterator()
+        )
+        logger.debug(f"{len(list(expired_events))} events can be expired")
+        # delete the media from disk
+        for expired in expired_events:
+            media_name = f"{expired.camera}-{expired.id}"
+            media_path = Path(f"{os.path.join(CLIPS_DIR, media_name)}.{file_extension}")
+
+            try:
+                media_path.unlink(missing_ok=True)
+                if file_extension == "jpg":
+                    media_path = Path(
+                        f"{os.path.join(CLIPS_DIR, media_name)}-clean.png"
+                    )
+                    media_path.unlink(missing_ok=True)
+            except OSError as e:
+                logger.warning(f"Unable to delete event images: {e}")
+
+        # update the clips attribute for the db entry
+        query = Event.select(Event.id).where(
+            Event.camera.not_in(self.camera_keys),
+            Event.start_time < expire_after,
+            Event.retain_indefinitely == False,
+        )
+
+        events_to_update = []
+
+        for batch in query.iterator():
+            events_to_update.extend([event.id for event in batch])
+            if len(events_to_update) >= CHUNK_SIZE:
+                logger.debug(
+                    f"Updating {update_params} for {len(events_to_update)} events"
+                )
+                Event.update(update_params).where(
+                    Event.id << events_to_update
+                ).execute()
+                events_to_update = []
+
+        # Update any remaining events
+        if events_to_update:
+            logger.debug(
+                f"Updating clips/snapshots attribute for {len(events_to_update)} events"
+            )
+            Event.update(update_params).where(Event.id << events_to_update).execute()
+
+        events_to_update = []
+        now = datetime.datetime.now()
+
+        ## Expire events from cameras based on the camera config
+        for name, camera in self.config.cameras.items():
+            expire_days = max(
+                camera.record.alerts.retain.days,
+                camera.record.detections.retain.days,
+            )
+            alert_expire_date = (
+                now - datetime.timedelta(days=camera.record.alerts.retain.days)
+            ).timestamp()
+            detection_expire_date = (
+                now - datetime.timedelta(days=camera.record.detections.retain.days)
+            ).timestamp()
+            # grab all events after specific time
+            expired_events = (
+                Event.select(
+                    Event.id,
+                    Event.camera,
+                )
+                .where(
+                    Event.camera == name,
+                    Event.retain_indefinitely == False,
+                    (
+                        (
+                            (Event.data["max_severity"] != "detection")
+                            | (Event.data["max_severity"].is_null())
+                        )
+                        & (Event.end_time < alert_expire_date)
+                    )
+                    | (
+                        (Event.data["max_severity"] == "detection")
+                        & (Event.end_time < detection_expire_date)
+                    ),
+                )
+                .namedtuples()
+                .iterator()
+            )
+
+            # delete the grabbed clips from disk
+            # only snapshots are stored in /clips
+            # so no need to delete mp4 files
+            for event in expired_events:
+                events_to_update.append(event.id)
 
         # update the clips attribute for the db entry
         for i in range(0, len(events_to_update), CHUNK_SIZE):
@@ -230,8 +330,9 @@ def expire(self, media_type: EventCleanupType) -> list[str]:
 
     def run(self) -> None:
         # only expire events every 5 minutes
-        while not self.stop_event.wait(300):
-            events_with_expired_clips = self.expire(EventCleanupType.clips)
+        while not self.stop_event.wait(1):
+            events_with_expired_clips = self.expire_clips()
+            return
 
             # delete timeline entries for events that have expired recordings
             # delete up to 100,000 at a time
@@ -242,7 +343,7 @@ def run(self) -> None:
                     Timeline.source_id << deleted_events_list[i : i + max_deletes]
                 ).execute()
 
-            self.expire(EventCleanupType.snapshots)
+            self.expire_snapshots()
 
             # drop events from db where has_clip and has_snapshot are false
             events = (

frigate/events/maintainer.py

@@ -210,6 +210,7 @@ def handle_object_detection(
                     "top_score": event_data["top_score"],
                     "attributes": attributes,
                     "type": "object",
+                    "max_severity": event_data.get("max_severity"),
                 },
             }
 

frigate/object_processing.py

@@ -702,30 +702,7 @@ def should_retain_recording(self, camera: str, obj: TrackedObject):
             return False
 
         # If the object is not considered an alert or detection
-        review_config = self.config.cameras[camera].review
-        if not (
-            (
-                obj.obj_data["label"] in review_config.alerts.labels
-                and (
-                    not review_config.alerts.required_zones
-                    or set(obj.entered_zones) & set(review_config.alerts.required_zones)
-                )
-            )
-            or (
-                (
-                    not review_config.detections.labels
-                    or obj.obj_data["label"] in review_config.detections.labels
-                )
-                and (
-                    not review_config.detections.required_zones
-                    or set(obj.entered_zones)
-                    & set(review_config.detections.required_zones)
-                )
-            )
-        ):
-            logger.debug(
-                f"Not creating clip for {obj.obj_data['id']} because it did not qualify as an alert or detection"
-            )
+        if obj.max_severity is None:
             return False
 
         return True

frigate/review/maintainer.py

@@ -7,7 +7,6 @@
 import string
 import sys
 import threading
-from enum import Enum
 from multiprocessing.synchronize import Event as MpEvent
 from pathlib import Path
 from typing import Optional
@@ -27,6 +26,7 @@
 from frigate.events.external import ManualEventState
 from frigate.models import ReviewSegment
 from frigate.object_processing import TrackedObject
+from frigate.review.types import SeverityEnum
 from frigate.util.image import SharedMemoryFrameManager, calculate_16_9_crop
 
 logger = logging.getLogger(__name__)
@@ -39,11 +39,6 @@
 THRESHOLD_DETECTION_ACTIVITY = 30
 
 
-class SeverityEnum(str, Enum):
-    alert = "alert"
-    detection = "detection"
-
-
 class PendingReviewSegment:
     def __init__(
         self,

frigate/review/types.py

@@ -0,0 +1,6 @@
+from enum import Enum
+
+
+class SeverityEnum(str, Enum):
+    alert = "alert"
+    detection = "detection"

frigate/test/http_api/base_http_test.py

@@ -10,7 +10,7 @@
 from frigate.api.fastapi_app import create_fastapi_app
 from frigate.config import FrigateConfig
 from frigate.models import Event, Recordings, ReviewSegment
-from frigate.review.maintainer import SeverityEnum
+from frigate.review.types import SeverityEnum
 from frigate.test.const import TEST_DB, TEST_DB_CLEANUPS
 
 

frigate/test/http_api/test_http_review.py

@@ -3,7 +3,7 @@
 from fastapi.testclient import TestClient
 
 from frigate.models import Event, Recordings, ReviewSegment
-from frigate.review.maintainer import SeverityEnum
+from frigate.review.types import SeverityEnum
 from frigate.test.http_api.base_http_test import BaseTestHttp