-
Notifications
You must be signed in to change notification settings - Fork 27
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
Feat: Add Loop property #33
base: master
Are you sure you want to change the base?
Conversation
I only made it a property (with get/setter) for consistency with other functions like |
Hi, I am currently will look into this as soon as I am free |
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.
This is a REQUIRED change. Not because of the reason though. This is required because the original documented way to loop causes a massive memory leak. Just replaying in that way creates a new instance in memory and does not release the old instance from memory and Python's GC does not catch and drop it. After 3-4 mins a 3 second video fills up 2GB+ of RAM. This change here fixes the memory leak because it uses the same instance. This isn't the perfect implementation, but may be the best possible. Implementing in this way causes the video to be unable to be changed unless you destroy the widget entirely and have it reestablished which is inconvenient. This is because it will continue loop forever, not looking for changes without ever breaking (Line 194+195). This does NOT stop the flickering and jittering. I don't know if there is a better solution to implement this given how tkinter is constructed. If you wrap the load and play lines in a function and thread that using threading that DOES get rid of the flickering, however it does not get rid of the stuttering every ~1sec.
Ok, I am looking into this. @Brandon4466 can you help me understand which part is not being Garbage collected? I will try to implement this ASAP |
@Brandon4466 were you suggesting changes to this PR? Because on github it shows that you are suggesting changes but I don't see anything new that you changed. |
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.
Just two very minor suggestions, to have more consistency in the code base
Makes it easier to toggle looping, and prevents flickering that would be caused by calling `TkinterVideo.play()` after the "<<Ended>>" event.
Makes it easier to toggle looping, and prevents flickering that would be caused by calling
TkinterVideo.play()
after the "<>" event.Fixes #32