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

Rework usage of FFMPEG_VideoReader's pos and lastread; FFMPEG_AudioReader.close_proc() -> close() #1220

Merged

Conversation

tburrows13
Copy link
Collaborator

Closes #1078, closes #1191, closes #648.

In general, this PR simplifies the handling of reader.pos by updating it only when reader.proc is created or accessed. This keeps the 2 up-to-date with each other.

I also fixed #1078 by ensuring that when the clip is being closed only to be reopened straight away, lastread is not deleted. This can happen legitimately (when we need to get a frame before the previous one or when we skip far ahead), but it can also happen if the reader has been previously closed and is then called upon. I still don't know why this is allowed, so perhaps it should be removed in the future?

I renamed FFMPEG_AudioReader.close_proc() to .close() to stay consistent with FFMPEG_VideoReader. In the future I might look at combining these 2 classes, probably with each of them inheriting a base FFMPEG_Reader class. This will avoid duplicating the (quite complex!) code and make it easier to analyse and fix.

Finally, I removed the logging.captureWarnings(True) (#1191) so that the tests would work. I will probably revisit how MoviePy handles warnings/logging in the future.

  • 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/
  • I have properly documented new or changed features in the documentation or in the docstrings
  • I have properly explained unusual or unexpected code in the comments around it
  • I have formatted my code using black -t py36

@tburrows13 tburrows13 added enhancement Positive change that does not change the API, i.e. improved performance, using less memory etc. bug-fix For PRs and issues solving bugs. labels Jun 6, 2020
@tburrows13 tburrows13 changed the title Rework of FFMPEG_VideoReader's pos and lastread; FFMPEG_AudioReader.close_proc() -> close() Rework usage of FFMPEG_VideoReader's pos and lastread; FFMPEG_AudioReader.close_proc() -> close() Jun 6, 2020
@coveralls
Copy link

coveralls commented Jun 6, 2020

Coverage Status

Coverage increased (+0.1%) to 67.558% when pulling 617ee91 on tburrows13:fix-OSError-failed-to-read-first-frame into 2390aec on Zulko:master.

@thenewguy
Copy link

Is there anything I could do to help move this one along? A release with this fix would be appreciated

@tburrows13
Copy link
Collaborator Author

I like to leave largish PRs open for a while in case anyone wants to review it or object to it, but I'm happy to merge this one today. Do you need it included in a pip-installable release (v2.0.0.dev2)?

@thenewguy
Copy link

@tburrows13 if that is convenient for you it would be awesome

@tburrows13
Copy link
Collaborator Author

Sure. My only holdup is that I merged #1063 but I've since decided that I wasn't so sure about that change. I need to do some more research about ffmpeg's durations and decide whether it is ok as-is, whether it should be reverted or whether #1222 is a suitable fix. Shouldn't be more than a couple of days.

@thenewguy
Copy link

thenewguy commented Jun 15, 2020

@tburrows13 sounds good. I can force use of master from github in the meantime. thanks!

@thenewguy
Copy link

@tburrows13 forgot to mention this fixed all of my expected failures around this issue! awesome work

@fancydancing
Copy link

Hi guys! Haven't you released this fix yet? No option to install it via pip?..

@tburrows13
Copy link
Collaborator Author

Hi @fancydancing, stay tuned, I'm planning a prerelease pip release this week that will include this :)

@fancydancing
Copy link

Hi @fancydancing, stay tuned, I'm planning a prerelease pip release this week that will include this :)

Thank you! Can't wait :) we're using your cool package in production and it started causing errors when making thumbnails for videos (which is by the way strange, nothing changed there...). So this prerelease would help us a lot ) Thank you for your work!

@tburrows13
Copy link
Collaborator Author

This PR has now been included in v2.0.0.dev2. Sorry for the delay and let us know how it goes :)

@tburrows13 tburrows13 deleted the fix-OSError-failed-to-read-first-frame branch October 5, 2020 00:27
@fancydancing
Copy link

Hi Tom! Upgraded to 2.0.0.dev2. Seems that it solves my problem!) But I have troubles installing it with Pipfile. It's not on PyPi, right?

@tburrows13
Copy link
Collaborator Author

tburrows13 commented Oct 6, 2020

It is on PyPI as a prerelease (https://pypi.org/project/moviepy/#history). With pip you can install it by using the --pre flag. I don't know about pipfile.

@fancydancing
Copy link

Thanks) Yes, pip install went well, will keep googling about pipfile then. Anyway, this issue is gone for me and it is great!

@josebenitezg
Copy link

josebenitezg commented Apr 7, 2021

Still facing the same error even if I make the upgrade.
When I trying to concatenate videos I am suceed with a small list of videos but when is bigger I face this error:

/home/ubuntu/anaconda3/envs/tensorflow_p37/lib/python3.7/site-packages/moviepy/video/io/ffmpeg_reader.py:161: UserWarning: In file video.mkv, 921600 bytes wanted but 0 bytes read,at frame index 0 (out of a total 24 frames), at time 0.00/1.00 sec. Using the last valid frame instead. UserWarning, Traceback (most recent call last): File "get_s3.py", line 113, in <module> clips.append(VideoFileClip(filename)) File "<decorator-gen-88>", line 2, in __init__ File "/home/ubuntu/anaconda3/envs/tensorflow_p37/lib/python3.7/site-packages/moviepy/decorators.py", line 89, in wrapper return f(*new_a, **new_kw) File "/home/ubuntu/anaconda3/envs/tensorflow_p37/lib/python3.7/site-packages/moviepy/video/io/VideoFileClip.py", line 109, in __init__ fps_source=fps_source, File "/home/ubuntu/anaconda3/envs/tensorflow_p37/lib/python3.7/site-packages/moviepy/video/io/ffmpeg_reader.py", line 73, in __init__ self.initialize() File "/home/ubuntu/anaconda3/envs/tensorflow_p37/lib/python3.7/site-packages/moviepy/video/io/ffmpeg_reader.py", line 131, in initialize self.lastread = self.read_frame() File "/home/ubuntu/anaconda3/envs/tensorflow_p37/lib/python3.7/site-packages/moviepy/video/io/ffmpeg_reader.py", line 166, in read_frame "MoviePy error: failed to read the first frame of " OSError: MoviePy error: failed to read the first frame of video file video.mkv. That might mean that the file is corrupted. That may also mean that you are using a deprecated version of FFMPEG. On Ubuntu/Debian for instance the version in the repos is deprecated. Please update to a recent version from the website.

`Python 3.7.10 | packaged by conda-forge | (default, Feb 19 2021, 16:07:37)
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.

import moviepy
print(moviepy.version)
2.0.0.dev2`

@tburrows13
Copy link
Collaborator Author

@josebenitezg would you mind making a new issue with that information please? Are you able to provide the videofile that is causing the crash?

PS Use 3 backticks at the start and end of your code to format it properly :)

@energy888666
Copy link

我升级到最新版本

@josebenitezg would you mind making a new issue with that information please? Are you able to provide the videofile that is causing the crash?

PS Use 3 backticks at the start and end of your code to format it properly :)

我升级到最新版本 pip install moviepy==2.0.0.dev2 还是无法解决
[这是我的视频链接:https://pan.baidu.com/s/1MGIv86_ogIr2kZ-XYdJVkA
提取码:8v3z

@ShunLu91
Copy link

我升级到最新版本

@josebenitezg would you mind making a new issue with that information please? Are you able to provide the videofile that is causing the crash?
PS Use 3 backticks at the start and end of your code to format it properly :)

我升级到最新版本 pip install moviepy==2.0.0.dev2 还是无法解决 [这是我的视频链接:https://pan.baidu.com/s/1MGIv86_ogIr2kZ-XYdJVkA 提取码:8v3z

可以试试把num_workers改为1,报错频率会降低;报错后,从断点重启即可

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix For PRs and issues solving bugs. enhancement Positive change that does not change the API, i.e. improved performance, using less memory etc.
Projects
None yet
7 participants