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

test_PR_529 fails in debian sid - possibly ffmpeg 5.0 #1798

Open
emollier opened this issue Jun 28, 2022 · 15 comments
Open

test_PR_529 fails in debian sid - possibly ffmpeg 5.0 #1798

emollier opened this issue Jun 28, 2022 · 15 comments
Labels
bug Issues that report (apparent) bugs. environment Using moviepy with, or issues possibly stemming from specific OS/platforms/environments. lib-FFmpeg Issues pertaining to dependency FFmpeg. tests Related to individual tests in the test suite or running the test suite.

Comments

@emollier
Copy link

Greetings,

For information, there is an open Debian Bug#1004642 in which it has been noticed that MoviePy was failing its unit test with ffmpeg 5.0 (or some other change that might have appeared at the same time, but that sounds unlikely).

Expected Behavior

All unit tests should pass

Actual Behavior

The test_PR_529 fails with error:

|     def test_PR_529():
|         with VideoFileClip(SOURCE_PATH+"media/fire2.mp4") as video_clip:
| >           assert video_clip.rotation == 180
| E           assert 0 == 180
| E            +  where 0 = <moviepy.video.io.VideoFileClip.VideoFileClip object at 0x7ff9fd890880>.rotation

Steps to Reproduce the Problem

Run the test suite using:

python3.10 -m pytest tests

Specifications

  • Python Version: 3.10.5
  • Moviepy Version: 1.0.3
  • FFmpeg Version: 5.0.1
  • Platform Name: Debian
  • Platform Version: Sid

Thank you for your work on MoviePy!

Have a nice day, :)
Étienne.

@emollier emollier added the bug Issues that report (apparent) bugs. label Jun 28, 2022
@emollier
Copy link
Author

urgh, the debian build log is somewhat old, I should have done a test build myself earlier, I'm seeing a lot more unit test failures (including the test_PR_529) at the moment:

=========================== short test summary info ============================
FAILED tests/test_ImageSequenceClip.py::test_1 - TypeError: must be real numb...
FAILED tests/test_PR.py::test_PR_458 - TypeError: must be real number, not No...
FAILED tests/test_PR.py::test_PR_528 - TypeError: must be real number, not No...
FAILED tests/test_PR.py::test_PR_529 - assert 0 == 180
FAILED tests/test_VideoClip.py::test_write_image_sequence - TypeError: unsupp...
FAILED tests/test_VideoClip.py::test_subfx - TypeError: must be real number, ...
FAILED tests/test_VideoClip.py::test_oncolor - TypeError: must be real number...
FAILED tests/test_VideoClip.py::test_setaudio - TypeError: must be real numbe...
FAILED tests/test_VideoClip.py::test_setaudio_with_audiofile - TypeError: mus...
FAILED tests/test_VideoClip.py::test_setopacity - TypeError: must be real num...
FAILED tests/test_VideoClip.py::test_toimageclip - TypeError: must be real nu...
FAILED tests/test_VideoFileClip.py::test_setup - TypeError: must be real numb...
FAILED tests/test_Videos.py::test_afterimage - TypeError: must be real number...
FAILED tests/test_compositing.py::test_clips_array_duration - TypeError: must...
FAILED tests/test_fx.py::test_blackwhite - TypeError: must be real number, no...
FAILED tests/test_fx.py::test_colorx - TypeError: must be real number, not No...
FAILED tests/test_fx.py::test_crop - TypeError: must be real number, not None...
FAILED tests/test_fx.py::test_fadein - TypeError: must be real number, not No...
FAILED tests/test_fx.py::test_fadeout - TypeError: must be real number, not N...
FAILED tests/test_fx.py::test_invert_colors - TypeError: must be real number,...
FAILED tests/test_fx.py::test_lum_contrast - TypeError: must be real number, ...
FAILED tests/test_fx.py::test_make_loopable - TypeError: must be real number,...
FAILED tests/test_fx.py::test_margin - TypeError: must be real number, not No...
FAILED tests/test_fx.py::test_mirror_x - TypeError: must be real number, not ...
FAILED tests/test_fx.py::test_mirror_y - TypeError: must be real number, not ...
FAILED tests/test_fx.py::test_resize - TypeError: must be real number, not No...
FAILED tests/test_fx.py::test_rotate - TypeError: must be real number, not No...
FAILED tests/test_fx.py::test_speedx - TypeError: must be real number, not No...
FAILED tests/test_fx.py::test_time_mirror - TypeError: must be real number, n...
FAILED tests/test_fx.py::test_time_symmetrize - TypeError: must be real numbe...
FAILED tests/test_issues.py::test_issue_334 - TypeError: must be real number,...
FAILED tests/test_issues.py::test_issue_470 - Failed: DID NOT RAISE <class 'O...
FAILED tests/test_issues.py::test_issue_547 - assert 9 == 6
FAILED tests/test_misc.py::test_subtitles - TypeError: must be real number, n...
FAILED tests/test_resourcerelease.py::test_release_of_file_via_close - TypeEr...
FAILED tests/test_resourcereleasedemo.py::test_failure_to_release_file - Type...
FAILED tests/test_videotools.py::test_credits - TypeError: must be real numbe...
======= 37 failed, 72 passed, 1 skipped, 79 warnings in 93.58s (0:01:33) =======

I'll probably want to double check there's not too many things wrong on my end...

@keikoro keikoro added tests Related to individual tests in the test suite or running the test suite. lib-FFmpeg Issues pertaining to dependency FFmpeg. environment Using moviepy with, or issues possibly stemming from specific OS/platforms/environments. labels Jun 30, 2022
@tillea
Copy link

tillea commented Sep 30, 2022

Any news for this issue? Debian will freeze in a couple of months and several packages depend from moviepy and are in danger not to be included into the next release.

@keikoro
Copy link
Collaborator

keikoro commented Oct 6, 2022

@tburrows13 @mondeja Tagging you into this because this is about creating a new release.

@tillea
Copy link

tillea commented Dec 1, 2022

Hi again, ist would be really cool if you could do a release quickly. Otherwise we will loose moviepy and its dependencies in Debian.

@keikoro
Copy link
Collaborator

keikoro commented Dec 1, 2022

Hi @tillea, thanks for the reminder, is there a timeline for this?

I've primarily managed our issues for the last several years in an attempt to prevent complete chaos in this repo but I've not had time to actively participate in development, so I really shouldn't be putting releases together.

@Zulko could you please check in here? Also pinging @mondeja and @tburrows13 again as they were the last devs most heavily involved with development.

@keikoro
Copy link
Collaborator

keikoro commented Dec 1, 2022

@tillea Am I understanding correctly that the issue didn't originate with moviepy but is the result of moviepy being used with a (newer) version of ffmpeg?

I feel like the fact that all sorts of smaller bug fixes got continuously merged into the repo's master branch in anticipation of a new release is not helping our situation.

If development had been kept separate, we could just go in and fix this one issue and release a new (minor) version, but I don't know how stable our current master really is. (I feel like it's something like halfway to MoviePy 2.0 + random fixes atop of that but could be wrong about that.)

@tillea
Copy link

tillea commented Dec 1, 2022 via email

@tillea
Copy link

tillea commented Dec 1, 2022 via email

@keikoro
Copy link
Collaborator

keikoro commented Dec 1, 2022

Ok, several things I'm unclear about:

  • Am I assuming correctly that Debian requires libraries to be compatible with the default versions of other libraries it comes with? Hence the requirement for ffmpeg 5? What about Python? Is there an online reference for the exact version numbers that need to be considered? Is it the ones provided by @emollier?
  • What we're talking about here is not actually moviepy (not) getting included in Debian bookworm, but other libraries which depend on moviepy, yes? Which ones are those?
  • By default moviepy 1.0.3 installs ffmpeg via imageio, which appears to result in ffmpeg-linux64-v4.2.2 being downloaded and used. I'm aware that it makes more sense to try to (re)use system(-provided) libraries, but I don't quite understand why insisting on changing the default would make moviepy ineligible as dependency for other libraries.

Thanks.

@keikoro
Copy link
Collaborator

keikoro commented Dec 1, 2022

It's possible the linked PR fixes the problem with this particular test in MoviePy 1.0.3, but I'll have to manually build ffmpeg 5 to verify...

On the current development version of MoviePy, however, several other tests fail (on the default install, i.e. with ffmpeg 4.2.2), so those would also need fixing for Debian not to reject moviepy-dependent libs.

@tillea
Copy link

tillea commented Dec 2, 2022 via email

@mondeja
Copy link
Collaborator

mondeja commented Dec 2, 2022

Please, don't ping me again, thanks.

@keikoro
Copy link
Collaborator

keikoro commented Dec 2, 2022

@mondeja Ok, then perhaps you should remove yourself from the maintainers list? Or clarify what you understand your role to be? It's confusing for everyone when you're listed as a maintainer and still partially contribute but don't want people to ping you. Thx.

@keikoro
Copy link
Collaborator

keikoro commented Dec 2, 2022

@tillea Hi Andreas, thanks for all the info.

The reason why I asked if the inclusion in Debian was about libraries which have moviepy as a dependency is that I couldn't find moviepy in the Debian versions I'm using (and if it wasn't there so far, it's not getting lost now but just not getting included), though it's possible that I was looking in the wrong place.

What I've found is this: https://tracker.debian.org/pkg/moviepy which seems to be about what we're talking about here. I don't know my way around the Debian tracker, but I'm including it for reference.

@emollier Sorry btw. for hijacking your issue – it obviously is kind of related but wasn't created with the intention of talking about the upcoming Debian release, I guess. Happy to move the discussion elsewhere.

@emollier
Copy link
Author

emollier commented Dec 2, 2022

Hi, no worries, Andreas and myself are both working on Debian packaging. The discussion might have drifted while remaining somewhat relevant; I'm loosely following.

What the tracker is showing in section "versions" is that moviepy is only available in Debian unstable. It has been introduced at the beginning of the year, so after the current stable release, but it is not in testing for the upcoming release of Debian 12 so far. The reason why this came up is because another project, psychopy, begun to depend on moviepy at some point. This is one of the missing pieces to get psychopy fit for the upcoming release (but not the only one).

In hope this clarifies the situation,
Étienne.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues that report (apparent) bugs. environment Using moviepy with, or issues possibly stemming from specific OS/platforms/environments. lib-FFmpeg Issues pertaining to dependency FFmpeg. tests Related to individual tests in the test suite or running the test suite.
Projects
None yet
Development

No branches or pull requests

4 participants