-
Notifications
You must be signed in to change notification settings - Fork 32
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
Piping frames to ffmpeg #15
base: master
Are you sure you want to change the base?
Conversation
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 your efforts! Great work, we are getting there!
I have added some comments, where I did not understand or it needs a bit of work. Please clarify, and i will make sure to approve the pull asap.
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.
Great! It looks good now. I will do some check-ups on the device just so we didn't miss something before merging. Thanks a lot for your contribution so far! Any limitations that you have noticed?
It looks good to me now. There are some optimizations that can be done with MotionFormat, such as allowing users to set presets and tune to balance between video size and render speed. However, I will handle these changes in another pull request. |
(I think) I have improved some things:
I couldn't help but notice, that the piping process sometimes (almost randomly) takes significantly longer than storing each frame - the old process approach. Although piping should be the better way, I cannot see a major improvement...?! For web on the other hand this feature might be essential?! |
Yes, there was no major improvement because it's a trade-off between capturing and handling images in parallel, and then batch processing them at the final stage (old approach). On the other hand, the new approach involves piping images in a queue, converting them, and processing them simultaneously in ffmpeg. I believe that for short durations, there may not be a significant impact, but for recording over a long period, it can help save memory, storage, and time as well. Regarding the web, I have conducted some research but have not yet found any way to implement piping. |
Let's keep this in mind though! Although web such as using ffmpeg_wasm might not require piping, long-period rendering might require this. If we could separate normal capturing under let's say 5-10min and longer period capturing + recording, this reasonable and working solution may be merged. |
@tienthanh1993 Thinking about giving your piping contribution a new chance in life. Check out #6 |
@polarby I wanted to check if piping resolves some RAM issues I have on older devices such as the iPhone 8. I cannot get this PR to run, though. For some reason, the closing of the |
Summary
This pull request updates a broken link and rewrites some code to make it compatible with piping frames.
Changes
Testing
I have tested these changes locally and they appear to work as expected. Please let me know if you have any questions or concerns.
Thanks,
Thanh Pham
This is just a prototype, but I hope it helps you save some time!
Feel free to change!