Skip to content
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

FEAT: Support for ffmpeg presets #3840

Merged
merged 15 commits into from
Nov 29, 2022

Conversation

NickM-27
Copy link
Sponsor Collaborator

@NickM-27 NickM-27 commented Sep 14, 2022

Implementation of #3429

I decided to go with a preset for each ffmpeg arg separately as having a separate preset field presented a few problems.

  1. There would either be a low number of presets and customization (picking and choosing what to support) would be low, or there would be an incredible amount of variations. Per camera there would need to be an option for specific types of cameras * rtsp / other protocols * tcp / udp * default / with audio
  2. The override behavior would also be difficult to handle, if a user sets preset but then also sets other fields as well.

To-Do:

  • Convert the preset args from a string to a list to avoid converting them
  • Potentially add more test cases
  • Likely need to add presets for the other cuvid decoders
  • Refine docs especially around cuvid

@NickM-27 NickM-27 linked an issue Sep 14, 2022 that may be closed by this pull request
@NickM-27 NickM-27 force-pushed the ffmpeg-presets branch 2 times, most recently from d8cd331 to 8cba07b Compare September 15, 2022 15:52
@NickM-27 NickM-27 linked an issue Sep 23, 2022 that may be closed by this pull request
@NickM-27 NickM-27 changed the base branch from release-0.11.0 to dev September 25, 2022 02:39
@netlify
Copy link

netlify bot commented Nov 2, 2022

Deploy Preview for frigate-docs ready!

Name Link
🔨 Latest commit a71ef93
🔍 Latest deploy log https://app.netlify.com/sites/frigate-docs/deploys/638566c30d14ed0009768f0f
😎 Deploy Preview https://deploy-preview-3840--frigate-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@NickM-27 NickM-27 marked this pull request as ready for review November 6, 2022 03:25
@NickM-27 NickM-27 force-pushed the ffmpeg-presets branch 3 times, most recently from 39e03c8 to 87516a3 Compare November 19, 2022 13:48
@felipecrs
Copy link
Contributor

Wow. This really looks good. I think you forgot to update this page maybe: https://docs.frigate.video/configuration/index

Copy link
Owner

@blakeblackshear blakeblackshear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nitpick. Otherwise looks great.

frigate/ffmpeg_presets.py Outdated Show resolved Hide resolved
@NickM-27
Copy link
Sponsor Collaborator Author

Wow. This really looks good. I think you forgot to update this page maybe: https://docs.frigate.video/configuration/index

Thank you! I want to leave that page the same, it isn't meant to be copied and it is a good reference for what the default args actually are.

@blakeblackshear blakeblackshear merged commit 87144cd into blakeblackshear:dev Nov 29, 2022
@roger-
Copy link

roger- commented Nov 29, 2022

This looks useful. Will it be possible to define presets in the yaml as well?

@felipecrs
Copy link
Contributor

felipecrs commented Nov 29, 2022

This looks useful. Will it be possible to define presets in the yaml as well?

That's a great idea! I'd suggest you open a separate issue/feature request for it.

@NickM-27
Copy link
Sponsor Collaborator Author

This looks useful. Will it be possible to define presets in the yaml as well?

Not in this iteration, maybe in the future.

@felipecrs
Copy link
Contributor

I just realized that FFmpeg have built-in support for presets, through files: https://ffmpeg.org/ffmpeg.html#Preset-files

Perhaps we should have taken that route instead. =/

@felipecrs felipecrs mentioned this pull request Dec 4, 2022
@NickM-27
Copy link
Sponsor Collaborator Author

NickM-27 commented Dec 4, 2022

I just realized that FFmpeg have built-in support for presets, through files: https://ffmpeg.org/ffmpeg.html#Preset-files

Perhaps we should have taken that route instead. =/

It would be difficult to do the custom fps args for mjpeg and other cases like that

@felipecrs
Copy link
Contributor

That's true. :)

@NickM-27 NickM-27 deleted the ffmpeg-presets branch December 9, 2022 14:43
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.

Add ffmpeg.presets
4 participants