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

Video: Limit FFmpeg Transcoding Resolution #3466

Closed
Lukasdotcom opened this issue Jun 12, 2023 · 8 comments
Closed

Video: Limit FFmpeg Transcoding Resolution #3466

Lukasdotcom opened this issue Jun 12, 2023 · 8 comments
Assignees
Labels
idea Feedback wanted / feature request released Available in the stable release video Video Formats, Transcoding, FFmpeg, Streaming & Co

Comments

@Lukasdotcom
Copy link
Contributor

Lukasdotcom commented Jun 12, 2023

Describe what problem this solves and why this would be valuable to many users

A lot of the time I am recording video in 4k, but on all of the clients I never actually need to watch the video at 4k and the sidecar file is then unnecessarily big and uses an unnecessary amount of bandwidth when played.

Describe the solution you'd like

For this purpose the environment variable "PHOTOPRISM_FFMPEG_SIZE" should be added to set the maximum resolution FFmpeg converts the video to, for example:

  • PHOTOPRISM_FFMPEG_SIZE=720
  • PHOTOPRISM_FFMPEG_SIZE=3840
@Lukasdotcom Lukasdotcom added the idea Feedback wanted / feature request label Jun 12, 2023
@lastzero lastzero added the video Video Formats, Transcoding, FFmpeg, Streaming & Co label Jun 14, 2023
@lastzero lastzero changed the title FFMPEG Maximum Resolution Video: Limit FFmpeg Transcoding Resolution Jun 14, 2023
@lastzero lastzero added the please-test Ready for acceptance test label Jun 29, 2023
lastzero added a commit that referenced this issue Jul 15, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero
Copy link
Member

@Lukasdotcom @theresagresch I wonder if it makes sense to only allow standard video resolutions to be specified? So HD, Full HD, Ultra HD, 2K and 4K. For example, few users will know that 4K actually means 3840 × 2160, and NOT a width of 4000 or 4096 pixels, as one might assume.

@lastzero lastzero added the waiting Impediment / blocked / waiting label Jul 15, 2023
@Lukasdotcom
Copy link
Contributor Author

We could either add cases to the original environmental variable to detect sizes like HD, HD+, UHD, 2K, and 4K and that they will cause certain predetermined resolutions to be used. Or you can only put in the more human readable names in that environmental variable and we create a new one that works like the original one called FFmpeg Resolution Advanced.

@Lukasdotcom
Copy link
Contributor Author

Ill make a new pull request for either option depending on what you think is the better option.

@Lukasdotcom
Copy link
Contributor Author

I just made the pull request based on the option where the FFMPEG_RESOLUTION variable can either use a standard one or an advanced user can specify a resolution down to the pixel #3549.

@lastzero
Copy link
Member

@Lukasdotcom Thanks a lot! I appreciate your PR but was prepared to refactor the config option myself today :)

lastzero added a commit that referenced this issue Jul 18, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero
Copy link
Member

@Lukasdotcom After checking all possible options, I decided to (re)use the default sizes defined in the thumbs package and adjust the config value to the next appropriate size. Supported sizes can be displayed by running the photoprism show video-sizes command in a terminal.

I also renamed PHOTOPRISM_FFMPEG_RESOLUTION to PHOTOPRISM_FFMPEG_SIZE so that the name of the option matches the image quality options e.g. PHOTOPRISM_THUMB_SIZE and PHOTOPRISM_JPEG_SIZE.

It would be great if you could quickly review my changes and let us know if this works for you so we can release it this week. Thank you!

@lastzero
Copy link
Member

The overview of config options has been updated in our docs: https://docs.photoprism.app/getting-started/config-options/#file-converters

lastzero added a commit that referenced this issue Jul 18, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
lastzero added a commit that referenced this issue Jul 18, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
@Lukasdotcom
Copy link
Contributor Author

Looks good to me. You might also want to mention in the docs that the resolution is only changed if the bitrate limit is exceeded.

@lastzero lastzero added tested Changes have been tested successfully and removed please-test Ready for acceptance test waiting Impediment / blocked / waiting labels Jul 18, 2023
lastzero added a commit that referenced this issue Jul 18, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
lastzero added a commit that referenced this issue Jul 18, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero lastzero added released Available in the stable release and removed tested Changes have been tested successfully labels Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea Feedback wanted / feature request released Available in the stable release video Video Formats, Transcoding, FFmpeg, Streaming & Co
Projects
Status: Release 🌈
Development

No branches or pull requests

2 participants