Add backend + link with CV model#5
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive web-based dashboard for real-time missing person detection, featuring a FastAPI backend and a Leaflet-powered frontend. The core detection logic has been refactored into a reusable pipeline class, and new services for database management and embedding training have been added. Feedback focuses on restoring CLI functionality in the standalone script, implementing atomic file swaps for the embeddings database, and fixing a potential division-by-zero error in the streaming service. Recommendations were also made to decouple recording frame rates from UI streaming rates and to adopt modern FastAPI lifespan management.
| video_source = 0 | ||
|
|
||
| main( | ||
| video_path=video_source, | ||
| output_path=args.output, | ||
| threshold=args.threshold, | ||
| frame_skip=args.skip, | ||
| no_display=args.no_display | ||
| output_path=None, | ||
| threshold=None, | ||
| frame_skip=None, | ||
| no_display=False, | ||
| ) |
There was a problem hiding this comment.
The removal of the argparse logic and hardcoding video_source = 0 is a significant regression. This script can no longer be used to process video files or specify different camera IDs via the command line, which breaks the documented usage and reduces the utility of the standalone script.
| db_dir=str(self.settings.missing_persons_db_dir), | ||
| output_path=str(versioned_output), | ||
| ) | ||
| shutil.copy2(versioned_output, root_output) |
There was a problem hiding this comment.
Using shutil.copy2 to overwrite the active database file is not atomic. If the process is interrupted or if the detector service attempts to read the file during the copy, it could result in a corrupted file or an EOFError during unpickling. It is recommended to copy to a temporary file first and then use os.replace() (or Path.replace()) for an atomic swap.
| else: | ||
| self._send_detection_if_changed(self._empty_detection()) | ||
|
|
||
| if now - last_stream_sent >= max(0.01, 1.0 / self.settings.stream_fps): |
There was a problem hiding this comment.
This line will raise a ZeroDivisionError if stream_fps is configured as 0. It is safer to validate this setting or add a check before division.
| if now - last_stream_sent >= max(0.01, 1.0 / self.settings.stream_fps): | |
| if self.settings.stream_fps > 0 and now - last_stream_sent >= max(0.01, 1.0 / self.settings.stream_fps): |
| timestamp = datetime.now().strftime("%Y%m%d_%H%M%S") | ||
| path = self.settings.recording_dir / f"recording_{timestamp}.mp4" | ||
| fourcc = cv2.VideoWriter_fourcc(*"mp4v") | ||
| return cv2.VideoWriter(str(path), fourcc, max(1.0, self.settings.stream_fps), (width, height)) |
There was a problem hiding this comment.
The recording FPS is hardcoded to stream_fps, which is the UI streaming rate. If the actual processing rate (inference speed) is different from this value, the saved video will play back at an incorrect speed (too fast or too slow). It is recommended to use the actual source FPS or the measured processing FPS for the VideoWriter.
| @app.on_event("startup") | ||
| async def on_startup() -> None: | ||
| manager.attach_loop(asyncio.get_running_loop()) | ||
| runtime.ensure_started() | ||
|
|
||
|
|
||
| @app.on_event("shutdown") | ||
| async def on_shutdown() -> None: | ||
| runtime.shutdown() |
There was a problem hiding this comment.
The @app.on_event decorators for startup and shutdown are deprecated in FastAPI. It is recommended to use the lifespan context manager instead for managing the application lifecycle.
2 participants
No description provided.