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

Fix resize func for https://github.com/Zulko/moviepy/issues/2049 #2093

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

Conversation

Dotrar
Copy link

@Dotrar Dotrar commented Jan 12, 2024

Fixes #2049

  • I have provided code that clearly demonstrates the bug and that only works correctly when applying this fix
  • I have added suitable tests demonstrating a fixed bug or new/changed feature to the test suite in tests/

Sorry - I couldn't get precommit to work without the first commit in my PR, feel free to drop that if you choose to merge.

From issue:

I added a little change to fix but it raises a few questions on how should we cater to this case.

Should we:

  1. leave as is - because if you want a specific resolution, you should specify it completely.
  2. use the target dimension as specified, calculate the other. and/or
  3. use proper rounding functions? perhaps use ceil or at the very least round even?

One could argue that if a result comes to " 12.2 pixels" (or in the example case 223.9999) then it should be ceil'd so that we have a pixel to cover that extra little bit that should be there.

But on the whole, I would expect that if i said "I want target height to be 100" it should come out as 100.

Dre Westcook added 2 commits January 13, 2024 07:09
When only one dimension was specified in the target resolution it was
used to find the scale factor, then used to scale the video with a
possible rounding issue, causing a mismatch between what was given and
what was received.

This change will only calculate the unspecified dimension and use the
given value as is.
@Dotrar Dotrar marked this pull request as ready for review January 12, 2024 20:44
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.

Resize gives wrong shaped output
1 participant