ffmpeg: Clamp resolutions in filter expression#417
Merged
Conversation
Contributor
I much prefer the failure mode of clamping, given how deep in the stack this is |
thomshutt
reviewed
Aug 14, 2024
thomshutt
approved these changes
Aug 14, 2024
j0sh
force-pushed
the
ja/clamp-in-filter
branch
from
250f00f to
721d867
Compare
Collaborator
Author
I mis-read this comment and added a check for 0x0 but then reverted it 👍 Will probably merge this PR once the rest of the green screen / rotation changes are ready to go, in case this PR needs any more tweaks |
Collaborator
Author
|
Incorporated into #418 |
This allows the transcoded resolution to be re-clamped correctly if the input resolution changes mid-segment. As a result, we no longer need to do this clamping in golang. Additionally, make the behavior between GPU and CPU more consistent by applying nvidia codec limits and clamping CPU transcodes.
Collaborator
Author
|
This somehow did not get pulled into #418 during the rebase 🤦 |
j0sh
force-pushed
the
ja/clamp-in-filter
branch
from
721d867 to
99aa3b4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This allows the transcoded resolution to be re-clamped correctly if the input resolution changes mid-segment.
As a result, we no longer need to do this clamping in golang.
Additionally, make the behavior between GPU and CPU more consistent by applying nvidia codec limits and clamping CPU transcodes.
Note that we effectively do not use the smaller side of the requested resolution. Eg, the "720" would be ignored in "1280x270"
Open question - should we error out if the requested VideoProfile resolution is under/over the codec limits, rather than implicitly clamping the resolution within the limits? Previously we were erroring out if the W/H was
<= 0but I am not sure why we would not also error out on , say, 1x99999