cleanup and tweaks

This commit is contained in:
Josh Hawkins 2026-05-03 11:43:18 -05:00
parent a142b2dcef
commit 4cfad07378
7 changed files with 89 additions and 41 deletions

View File

@ -38,8 +38,8 @@ class DebugReplayStatusResponse(BaseModel):
Returns only session-presence fields. Startup progress and error Returns only session-presence fields. Startup progress and error
details flow through the job_state WebSocket topic via the details flow through the job_state WebSocket topic via the
``debug_replay`` job (see :mod:`frigate.jobs.debug_replay`); the debug_replay job (see frigate.jobs.debug_replay); the
Replay page subscribes there with ``useJobStatus("debug_replay")``. Replay page subscribes there with useJobStatus("debug_replay").
""" """
active: bool active: bool

View File

@ -1,8 +1,8 @@
"""Debug replay camera management for replaying recordings with detection overlays. """Debug replay camera management for replaying recordings with detection overlays.
The startup work (ffmpeg concat + camera config publish) lives in The startup work (ffmpeg concat + camera config publish) lives in
``frigate.jobs.debug_replay``. This module owns only session presence frigate.jobs.debug_replay. This module owns only session presence
(``active``), session metadata, and post-session cleanup. (active), session metadata, and post-session cleanup.
""" """
import logging import logging
@ -35,9 +35,9 @@ logger = logging.getLogger(__name__)
class DebugReplayManager: class DebugReplayManager:
"""Owns the lifecycle pointers for a single debug replay session. """Owns the lifecycle pointers for a single debug replay session.
A session exists from the moment ``mark_starting`` is called (synchronously, A session exists from the moment mark_starting is called (synchronously,
inside the API handler) until ``clear_session`` runs (on success cleanup, inside the API handler) until clear_session runs (on success cleanup,
failure, or stop). The ``active`` property is the source of truth that the failure, or stop). The active property is the source of truth that the
status bar consumes broader than the startup job, which only covers the status bar consumes broader than the startup job, which only covers the
preparing_clip / starting_camera window. preparing_clip / starting_camera window.
""" """
@ -52,7 +52,7 @@ class DebugReplayManager:
@property @property
def active(self) -> bool: def active(self) -> bool:
"""True from ``mark_starting`` until ``clear_session``.""" """True from mark_starting until clear_session."""
return self.replay_camera_name is not None return self.replay_camera_name is not None
def mark_starting( def mark_starting(
@ -64,7 +64,7 @@ class DebugReplayManager:
) -> None: ) -> None:
"""Synchronously claim the session before the job runner starts. """Synchronously claim the session before the job runner starts.
Called inside the API handler so the status bar sees ``active=True`` Called inside the API handler so the status bar sees active=True
immediately, before the worker thread does any ffmpeg work. immediately, before the worker thread does any ffmpeg work.
""" """
with self._lock: with self._lock:
@ -82,7 +82,7 @@ class DebugReplayManager:
def clear_session(self) -> None: def clear_session(self) -> None:
"""Reset session pointers without publishing camera removal. """Reset session pointers without publishing camera removal.
Used by the job runner on failure paths. ``stop()`` does the camera Used by the job runner on failure paths. stop() does the camera
teardown plus this clear in one step. teardown plus this clear in one step.
""" """
with self._lock: with self._lock:
@ -105,7 +105,7 @@ class DebugReplayManager:
) -> None: ) -> None:
"""Build the in-memory replay camera config and publish the add event. """Build the in-memory replay camera config and publish the add event.
Called by the job runner during the ``starting_camera`` phase. Called by the job runner during the starting_camera phase.
""" """
source_config = frigate_config.cameras[source_camera] source_config = frigate_config.cameras[source_camera]
camera_dict = self._build_camera_config_dict( camera_dict = self._build_camera_config_dict(
@ -121,7 +121,12 @@ class DebugReplayManager:
config_data["cameras"] = {} config_data["cameras"] = {}
config_data["cameras"][replay_name] = camera_dict config_data["cameras"][replay_name] = camera_dict
new_config = FrigateConfig.parse_object(config_data) try:
new_config = FrigateConfig.parse_object(config_data)
except Exception as e:
raise RuntimeError(
f"Failed to validate replay camera config: {e}"
) from e
frigate_config.cameras[replay_name] = new_config.cameras[replay_name] frigate_config.cameras[replay_name] = new_config.cameras[replay_name]
config_publisher.publish_update( config_publisher.publish_update(

View File

@ -1,10 +1,10 @@
"""Debug replay startup job: ffmpeg concat + camera config publish. """Debug replay startup job: ffmpeg concat + camera config publish.
The runner orchestrates the async portion of starting a debug replay The runner orchestrates the async portion of starting a debug replay
session. The :class:`DebugReplayManager` (in :mod:`frigate.debug_replay`) session. The DebugReplayManager (in frigate.debug_replay) owns session
owns session presence so the status bar can keep reading a single presence so the status bar can keep reading a single `active` flag from
``active`` flag from ``/debug_replay/status`` for the entire session /debug_replay/status for the entire session window which is broader
window which is broader than this job's lifetime. than this job's lifetime.
""" """
import logging import logging
@ -69,8 +69,8 @@ class DebugReplayJob(Job):
def to_dict(self) -> dict[str, Any]: def to_dict(self) -> dict[str, Any]:
"""Whitelisted payload for the job_state WS topic. """Whitelisted payload for the job_state WS topic.
Replay-specific fields land in ``results`` so the frontend's Replay-specific fields land in results so the frontend's
generic ``Job<TResults>`` type can be parameterised cleanly. generic Job<TResults> type can be parameterised cleanly.
""" """
return { return {
"id": self.id, "id": self.id,
@ -114,8 +114,8 @@ def query_recordings(source_camera: str, start_ts: float, end_ts: float):
class DebugReplayJobRunner(threading.Thread): class DebugReplayJobRunner(threading.Thread):
"""Worker thread that drives the startup job to completion. """Worker thread that drives the startup job to completion.
Owns the live ffmpeg ``Popen`` reference for cancellation. Cancellation Owns the live ffmpeg Popen reference for cancellation. Cancellation
is two-step (``threading.Event`` + ``proc.terminate()``) so the runner is two-step (threading.Event + proc.terminate()) so the runner
both knows it should stop and is unblocked from its blocking subprocess both knows it should stop and is unblocked from its blocking subprocess
wait. wait.
""" """
@ -290,10 +290,9 @@ class DebugReplayJobRunner(threading.Thread):
self.job.status = JobStatusTypesEnum.cancelled self.job.status = JobStatusTypesEnum.cancelled
self.job.end_time = time.time() self.job.end_time = time.time()
self._broadcast(force=True) self._broadcast(force=True)
# Manager session pointers are cleared by stop() on the API side # The caller of cancel_debug_replay_job (DebugReplayManager.stop) owns
# (it already holds the cleanup contract). On any other cancellation # session cleanup — db rows, filesystem artifacts, clear_session. We
# path, also clear so /start can run again. # only clean up the partial concat output we created.
self.replay_manager.clear_session()
_remove_silent(clip_path) _remove_silent(clip_path)
@ -316,8 +315,8 @@ def start_debug_replay_job(
) -> str: ) -> str:
"""Validate, create job, start runner. Returns the job id. """Validate, create job, start runner. Returns the job id.
Raises ``ValueError`` for bad params (camera missing, time range Raises ValueError for bad params (camera missing, time range
invalid, no recordings) and ``RuntimeError`` if a session is already invalid, no recordings) and RuntimeError if a session is already
active. active.
""" """
if job_is_running(JOB_TYPE) or replay_manager.active: if job_is_running(JOB_TYPE) or replay_manager.active:

View File

@ -204,6 +204,43 @@ class TestDebugReplayManagerPublishCamera(unittest.TestCase):
topic_arg = publisher.publish_update.call_args.args[0] topic_arg = publisher.publish_update.call_args.args[0]
self.assertEqual(topic_arg.update_type, CameraConfigUpdateEnum.add) self.assertEqual(topic_arg.update_type, CameraConfigUpdateEnum.add)
def test_publish_camera_wraps_parse_failure_in_runtime_error(self) -> None:
from frigate.debug_replay import DebugReplayManager
manager = DebugReplayManager()
frigate_config = MagicMock()
frigate_config.cameras = {"front": MagicMock()}
publisher = MagicMock()
with (
patch.object(
manager,
"_build_camera_config_dict",
return_value={"enabled": True},
),
patch("frigate.debug_replay.find_config_file", return_value="/cfg.yml"),
patch("frigate.debug_replay.YAML") as yaml_cls,
patch(
"frigate.debug_replay.FrigateConfig.parse_object",
side_effect=ValueError("zone foo has invalid coordinates"),
),
patch("builtins.open", unittest.mock.mock_open(read_data="cameras:\n")),
):
yaml_cls.return_value.load.return_value = {"cameras": {}}
with self.assertRaises(RuntimeError) as ctx:
manager.publish_camera(
source_camera="front",
replay_name="_replay_front",
clip_path="/tmp/clip.mp4",
frigate_config=frigate_config,
config_publisher=publisher,
)
self.assertIn("replay camera config", str(ctx.exception))
self.assertIn("invalid coordinates", str(ctx.exception))
publisher.publish_update.assert_not_called()
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()

View File

@ -449,6 +449,11 @@ class TestRunnerCancellation(unittest.TestCase):
job = get_current_job("debug_replay") job = get_current_job("debug_replay")
self.assertEqual(job.status, JobStatusTypesEnum.cancelled) self.assertEqual(job.status, JobStatusTypesEnum.cancelled)
# Runner must not clear the manager session on cancellation —
# that belongs to the caller of cancel_debug_replay_job (stop()).
# If the runner cleared it, stop() would log "no active session"
# and skip its cleanup_db / cleanup_files calls.
self.assertTrue(self.manager.active)
if __name__ == "__main__": if __name__ == "__main__":

View File

@ -27,24 +27,24 @@
} }
}, },
"page": { "page": {
"noSession": "No Active Replay Session", "noSession": "No Active Debug Replay Session",
"noSessionDesc": "Start a debug replay from the History view by clicking the Debug Replay button in the toolbar.", "noSessionDesc": "Start a Debug Replay from History view by clicking the Actions button in the toolbar and choosing Debug Replay.",
"goToRecordings": "Go to History", "goToRecordings": "Go to History",
"preparingClip": "Preparing replay clip…", "preparingClip": "Preparing clip…",
"preparingClipDesc": "Stitching together recordings for the selected time range. This can take a minute for longer ranges.", "preparingClipDesc": "Frigate is stitching together recordings for the selected time range. This can take a minute for longer ranges.",
"startingCamera": "Starting replay camera…", "startingCamera": "Starting Debug Replay…",
"startError": { "startError": {
"title": "Failed to start debug replay", "title": "Failed to start Debug Replay",
"back": "Back to History" "back": "Back to History"
}, },
"sourceCamera": "Source Camera", "sourceCamera": "Source Camera",
"replayCamera": "Replay Camera", "replayCamera": "Replay Camera",
"initializingReplay": "Initializing replay...", "initializingReplay": "Initializing Debug Replay...",
"stoppingReplay": "Stopping replay...", "stoppingReplay": "Stopping Debug Replay...",
"stopReplay": "Stop Replay", "stopReplay": "Stop Replay",
"confirmStop": { "confirmStop": {
"title": "Stop Debug Replay?", "title": "Stop Debug Replay?",
"description": "This will stop the replay session and clean up all temporary data. Are you sure?", "description": "This will stop the session and clean up all temporary data. Are you sure?",
"confirm": "Stop Replay", "confirm": "Stop Replay",
"cancel": "Cancel" "cancel": "Cancel"
}, },
@ -55,6 +55,6 @@
"activeTracking": "Active tracking", "activeTracking": "Active tracking",
"noActiveTracking": "No active tracking", "noActiveTracking": "No active tracking",
"configuration": "Configuration", "configuration": "Configuration",
"configurationDesc": "Fine tune motion detection and object tracking settings for the debug replay camera. No changes are saved to your Frigate configuration file." "configurationDesc": "Fine tune motion detection and object tracking settings for the Debug Replay camera. No changes are saved to your Frigate configuration file."
} }
} }

View File

@ -177,7 +177,6 @@ export default function Replay() {
position: "top-center", position: "top-center",
}); });
refreshStatus(); refreshStatus();
navigate("/review");
}) })
.catch((error) => { .catch((error) => {
const errorMessage = const errorMessage =
@ -191,7 +190,7 @@ export default function Replay() {
.finally(() => { .finally(() => {
setIsStopping(false); setIsStopping(false);
}); });
}, [navigate, refreshStatus, t]); }, [refreshStatus, t]);
// Camera activity for the replay camera // Camera activity for the replay camera
const { data: config } = useSWR<FrigateConfig>("config", { const { data: config } = useSWR<FrigateConfig>("config", {
@ -278,8 +277,11 @@ export default function Replay() {
); );
} }
// No active session. // No active session. Also covers the brief window between the runner
if (!status?.active) { // pushing job.status = "cancelled" via WS and the next SWR refresh
// flipping status.active to false — without this, render falls through
// to the full replay UI and you see a flash of it before stop completes.
if (!status?.active || replayJob?.status === "cancelled") {
return ( return (
<div className="flex size-full flex-col items-center justify-center gap-4 p-8"> <div className="flex size-full flex-col items-center justify-center gap-4 p-8">
<MdReplay className="size-12" /> <MdReplay className="size-12" />
@ -320,7 +322,7 @@ export default function Replay() {
</div> </div>
</div> </div>
) : ( ) : (
<ActivityIndicator className="size-3.5" /> <ActivityIndicator className="size-8" />
)} )}
<Heading as="h3" className="text-center"> <Heading as="h3" className="text-center">
{phaseTitle} {phaseTitle}