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

ffmpeg: Clamp resolutions in filter expression #417

Merged
merged 1 commit into from
Aug 19, 2024
Merged

Conversation

j0sh
Copy link
Collaborator

@j0sh j0sh commented Aug 14, 2024

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 <= 0 but I am not sure why we would not also error out on , say, 1x99999

@j0sh j0sh requested review from thomshutt and emranemran August 14, 2024 07:37
@thomshutt
Copy link
Contributor

thomshutt commented Aug 14, 2024

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?

I much prefer the failure mode of clamping, given how deep in the stack this is

@j0sh j0sh force-pushed the ja/clamp-in-filter branch from 250f00f to 721d867 Compare August 14, 2024 21:25
@j0sh
Copy link
Collaborator Author

j0sh commented Aug 14, 2024

I much prefer the failure mode of clamping, given how deep in the stack this is

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

@j0sh
Copy link
Collaborator Author

j0sh commented Aug 19, 2024

Incorporated into #418

@j0sh j0sh closed this Aug 19, 2024
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.
@j0sh
Copy link
Collaborator Author

j0sh commented Aug 19, 2024

This somehow did not get pulled into #418 during the rebase 🤦

@j0sh j0sh reopened this Aug 19, 2024
@j0sh j0sh force-pushed the ja/clamp-in-filter branch from 721d867 to 99aa3b4 Compare August 19, 2024 18:03
@j0sh j0sh merged commit f873529 into master Aug 19, 2024
1 of 2 checks passed
@j0sh j0sh deleted the ja/clamp-in-filter branch August 19, 2024 18:04
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