Parse rotation from displaymatrix

[LoipesMas]

Newer ffmpeg versions no longer provide rotate field in Metadata section. So I changed the parsing to get this value from Side data's displaymatrix field. But that angle value is in opposite direction, however it's in-line with ffmpegs rotation. I.e. if we rotate video using ffmpeg with rotate="90", old value would show 270, while new value shows 90. So it's potentially a breaking change (but IMO for the better). Also, I think this new value is in range -180:180, instead of 0:360, not sure if that could cause other problems.
Older ffmpeg versions (4.4) do provide displaymatrix field as well, so this code works for both older and newer versions.

I renamed the test file and changed the test value, instead of changing the file contents, to avoid increasing repo size.

[coveralls]

Coverage increased (+1.0%) to 84.717% when pulling 4c5f4e9 on TraseeHQ:fix_rotation into 858bb81 on Zulko:master.

[B3QL]

@mondeja Can you take a look?
BTW the CI is failing on linux (3.6) due to Ubuntu 22.04 rollout on GitHub Actions.

Related issues:
actions/setup-python#543
actions/runner-images#6399

[keikoro]

@B3QL Thanks for the info on the build failure.

@LoipesMas Which ffmpeg versions are we talking about here? You're only referencing 4.4 as "older" here, so it's unclear if this is applicable to other 4.x versions as well.

I'm asking because ffmpeg 5 seems to be the cause of what's discussed in #1798. The test that fails in the other issue is for the last PyPI release, i.e. MoviePy 1.0.3. Am I assuming correctly you are using the latest development version of MoviePy? Thanks.

[LoipesMas]

@keikoro I tested this now with 4.2.2 (provided by imageio_ffmpeg) and 4.0.3 (just to be sure), and displaymatrix field is there as well, with the same values. And tests in CI do pass, which I assume are running 4.2.2.

Yes, I'm using latest development version of MoviePy, i.e. master branch. And all tests seem to pass with ffmpeg 5.1.2 (env IMAGEIO_FFMPEG_EXE=/path/to/ffmpeg5.1.2 python -m pytest).
I don't know how the release state looks like (when was the last release, what changed, etc), but perhaps this commit could be cherry picked and applied on top of latest release to hotfix this Debian issue?

Cheers.

[mondeja]

Please, don't mention me again. I've said yet that I haven't publication rights, so I'm only reviewing those changes that could compromise my personal usage of moviepy, I haven't motivation to maintain this project.

That said, if we're going to accept support for FFMpeg v5 the CI should be updated to test against the both versions, right? Seems that the only one with this concerns are me but the library has 2.5 million downloads every month, that doesn't have sense.

[B3QL]

Please, don't mention me again. I've said yet that I haven't publication rights, so I'm only reviewing those changes that could compromise my personal usage of moviepy, I haven't motivation to maintain this project.

OK, sorry for bothering you, I just assumed that you partially maintain the project based on the last few PRs that you accepted and were merged :)

[keikoro]

@B3QL No worries, I didn't know either and I'm another maintainer for this repo, so...

@LoipesMas Thanks for the update!

The last release in this repo was v2.0.0.dev2 from October 2020. All PRs which were merged since got merged right into master. I'm assuming all releases so far were also created directly from master – instead of keeping changes for both upcoming releases and various "random" fixes contained to, say, one or moredev branches.

I'm saying I'm assuming that's what happened because I wasn't involved in the process so far. I also didn't contribute to development toward MoviePy 2.0. @tburrows13 was the main person behind that, #1089 is the ticket that contains the most info on that.

What I do know with certainty is that I can't create new releases on PyPI, where the last release is 1.0.3 (!) and I believe that's the relevant thing here for Debian.

Regarding GitHub Actions, I guess FFMPEG 5 should get added to the CI tests as Álvaro pointed out earlier, and Python 3.10 probably as well if we want to test against what the new Debian release will come with.

[OsaAjani]

Thank you for your contributions and for reporting issues in this repository. With the release of v2, which introduces significant changes to the codebase and API, we’ve reviewed the backlog of open PRs and issues. Due to the length of the backlog and the likelihood that many of these are either fixed or no longer applicable, we’ve made the decision to close all previous PRs and issues.

If you believe that any of these are still relevant to the current version or if you'd like to reopen a related discussion, please feel free to create a new issue or pull request, referencing the old one.

Thank you for your understanding and continued support!