fixes for tests - Githubissues

Zulko / moviepy

Video editing with Python

https://zulko.github.io/moviepy/

MIT License

12.07k stars 1.51k forks source link

fixes for tests #2195

Open antonpetrov145 opened 3 weeks ago

antonpetrov145 commented 3 weeks ago

set requred version of pillow to 9.5.0
test-ffmpeg-reader - change duration number to match video
ffmpeg-reader - add displaymatrix to be read as video rotation
config set binary to autodetect

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

keikoro commented 3 weeks ago

@antonpetrov145 Thanks for the contribution!

I'm not sure if removing ffmpeg-imageio for the sake of fixing the tests is the right solution, though. See this comment by Zulko in one of the big threads/discussions about the future of the project (the paragraph before the bullet point summary specifically, which is about dependency management).

keikoro commented 3 weeks ago

Idk, maybe worth throwing your suggested fix into the discussion?

antonpetrov145 commented 3 weeks ago

Thank you for your time to look at the changes I made.

The idea behind setting the ffmpeg binary to auto-detect by default and removing the test is because of an issue that was because of ffmpeg-imageio setting the default binary to exe even on Linux (my other PR) causing Path error on importing moviepy. #2189 #2190

Idk, maybe worth throwing your suggested fix into the discussion?

Yes sure, is it ok for me to write there?

keikoro commented 2 weeks ago

@antonpetrov145 I've now had (more of) a look at the code (and related issues), though haven't been able to test it – am I understanding correctly that changing FFMPEG_BINARY is not actually needed to fix the issue at hand? If so, I'd ask you leave off that particular change and only (force) push the relevant bits of code.

antonpetrov145 commented 2 weeks ago

@keikoro made the change you proposed Irrelevant: this is irrelevant actually it is - without setting the binary to auto-detect (in code or in ENV) tests are not passing, at least on my side. If you think I'm doing something wrong I'm happy to make the change you propose