-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Update preview.py, fix issue #575 #1537
Conversation
Fixed issue Zulko#575. Added two pg.quit statements at lines 146 and 162. For line 146, this statement adds quit functionality for " event.type == pg.KEYDOWN and event.key == pg.K_ESCAPE". This allows the window to close whenever the event triggers. For line 162, once the for loop ends (the video ends), the window closes instead of crashing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for opening this pull request @Victrollia
I'm really worried about if this could have side effects, for example: have you tried this change concatenating multiples previews in the same Python session? Something like:
from moviepy.editor import VideoFileClip
filename = r"media\chaplin.mp4"
clip1 = VideoFileClip(filename)
clip2 = VideoFileClip(filename)
clip1.preview()
clip2.preview()
Also, I've tried to reproduce the problem and I couldn't in the next environment:
- Windows: 10 version 1909
- Moviepy: master branch
- pygame: 2.0.1
Could you specify what are the versions of Windows, moviepy and pygame for which this problem happens? This only happens when running the preview in a Jupyter Notebook, or also appears standalone?
Tested on Jupyter Notebook, on Windows 10 10.0.19041 Build 19041. Tested with two previews like in the sample code you provided, once one closes, the next clip preview opens. Didn't add a checkmark on tests because it was mentioned prior that it is tricky to test pygame. Only two lines were added to close the pygame to minimize the chances of any side effects. The first one closes the window if it was interrupted midway while the clip was playing, but then I noticed that it would still crash if the preview ended, that's why another line of code was added that closes the window once it ended the preview. Contributing to this project has been an assignment for a capstone class for CS degree, and therefore my team (3 other people) had the same issue with crashing previews on their Jupyter Notebooks. The Moviepy master branch was cloned recently so it's the latest version. Tested on Python 3.8 & 3.9. Using the latest version of pygame (2.0.1). Let me know if you have any other questions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the detailed response @Victrollia 🙏
I finally have been able to reproduce it. Seems that your fix has no side effects in any platform and solves the problem, it's perfect 👍
Thanks again for the fix! If you or someone from your team finds another problem, do not hesitate to open a pull request 💯 |
Fixed issue #575. Added two pg.quit statements at lines 146 and 162. For line 146, this statement adds quit functionality for " event.type == pg.KEYDOWN and event.key == pg.K_ESCAPE". This allows the window to close whenever the event triggers. For line 162, once the for loop ends (the video ends), the window closes instead of crashing.
tests/