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

[IMPROVEMENT] FPS increase #34

Merged
merged 1 commit into from
Aug 1, 2023
Merged

Conversation

havetc
Copy link
Contributor

@havetc havetc commented Jun 10, 2023

Two way were used to increase fps:
-use of av resize integrated with "to_image" method. It's made to resize video stream and is far more optimized than PIL for this usecase
-use of imagetk paste method. Display frame was creating a new PhotoImage each call, but a PhotoImage call is just a imagetk creation of the good dimension/type, then a paste of the content inside it. Now we create a new PhotoImage only if the dimension changes.
It also seems that less call to the Tk event loop are made when we paste new content instead of creating a new PhotoImage

Tested with and without my other pull request about framerate precision.
For a 4K60fps video, on a 1440p screen framerate went from 9fps to about 27fps, and when played on a reduced window it went from about 20 to the full 60fps (resize where too slow on 4k, no matter how small the window the full framerate was never attained)

Two way used to increase fps:
-use of av resize integrated with "to_image" method. It's
made to resize video stream and is far more optimized than
PIL for this usecase
-use of imagetk paste method. Display frame was creating
a new PhotoImage each call, but a PhotoImage call is just
a imagetk creation of the good dimension/type, then a
paste of the content inside it. Now we create a new PhotoImage
only if the dimension changes.
It also seems that less call to the Tk event loop are made
when we paste new content instead of creating a new PhotoImage
@havetc havetc mentioned this pull request Jun 10, 2023
@PaulleDemon
Copy link
Owner

Ok, this is a significant improvement. But for smaller files it becomes too fast, your #28 worked perfectly for smaller files. Can you check this?

@PaulleDemon
Copy link
Owner

here's a test file you can try with

test.mp4

@PaulleDemon
Copy link
Owner

If you get time can you also review and see if there are any improvements you can bring to pull request #33? I'll try to push it all at once so the next version will fix most of these issues.

@havetc
Copy link
Contributor Author

havetc commented Jul 31, 2023

Hello, thanks for the reply!
To be clear, this pull request is complimentary with #28 , it increases frame throughput in cases where it was limited by the system but doesn't check if the run speed is correct.
On the other hand #28 doesn't change the max frame througput, but ensure that the running speed of the video is correct.

Ok, this is a significant improvement. But for smaller files it becomes too fast, your #28 worked perfectly for smaller files. Can you check this?

I've tested, and without #28 I'v got about the same too fast running speed with and without #34 . I've got about 16,5s (checked manually with a chronometer so not very precise) for the 24s test.webm, so I don't think this pull request causes this problem.

@PaulleDemon
Copy link
Owner

Ok also can you look into pull #33, can you check if you can bring any improvements? I am not sure why the current implementation that I made is increasing memory usage very loop.

@PaulleDemon PaulleDemon merged commit 51966f3 into PaulleDemon:master Aug 1, 2023
@havetc
Copy link
Contributor Author

havetc commented Aug 1, 2023

Ok also can you look into pull #33, can you check if you can bring any improvements? I am not sure why the current implementation that I made is increasing memory usage very loop.

Not sure I can improve it, it looks ok as an implementation of a loop feature.
I think the problem with the leak in current implementation is the way that self._load_thread = None is done at the end of _load function from within said thread. As the thread join function is never called from the main thread, I guess that even if the load thread has terminated its ressource are not released. The fact that it is flagged as daemonic may also cause that, apparently daemonic threads are not meant to be released.

Maybe by setting the load_thread as non daemonic, and binding the event <> in the main thread to self._load_thread.join() and self._load_thread = None could avoid the leak, however it only works if no unexpected exception is raised in load_thread, which would bypass the <> event.

As #33 avoid stopping / restarting and seeks to the beginning for a loop, it avoids that leaks in case of a loop in addition to being more optimized by avoiding to reload the video.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants