Skip to content

Media handling improvements + fixed logging #363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

t3hk0d3
Copy link

@t3hk0d3 t3hk0d3 commented May 24, 2025

Improved video_analyzer media handling

  • Added URL support to for video_path - including media-source:// and relative /api/frigate/... paths.

  • Parallel video processing to avoid huge delays when processing multiple files.

  • Replaced os.system() with asyncio.create_subprocess_shell() and added extensive debug logging around FFmpeg

    • Ideally it should be using asyncio.create_subprocess_exec to avoid security issues, but it comes with a problem of locating ffmpeg binary, because it needs a full path to executable.
    • Changes are already a bit too much for a single PR, so i've decided to leave it for a next time.
  • Fixed video_path escaping using shlex.quote - f"'{video_path}'" could be broken too easily (Doorbell's video.mp4)

  • Refactoring file fetching (_fetch method) to write files using streaming.

Debug logging fixes

  • Fixed request/response dumps (including API keys) written to logs with INFO level.

@t3hk0d3 t3hk0d3 changed the title Misc fixes Media handling improvements + fixed logging May 24, 2025
t3hk0d3 added 2 commits May 24, 2025 22:40
- Enhanced `_fetch` method to support saving response directly to a file.
- Updated `add_video` method to streamline video processing and handle media source IDs.
- Improved error handling and logging throughout video and image processing methods.
- Added parallel processing for handling multiple videos.
@t3hk0d3
Copy link
Author

t3hk0d3 commented May 28, 2025

@valentinfrlch can i get a PR review pls?

@valentinfrlch
Copy link
Owner

Very busy this and next week, but I'll review it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants