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

exporter: add a frame cache #19314

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fries1234
Copy link

This commit adds a frame cache which limits the amount of frames that can be rendered into memory before writing to disk and getting more frames. This can save memory on big 4k frames, especially if you have alot of them as it can fill up 32 gigs of ram pretty fast.

I also split the options structs into its own file.

@fries1234 fries1234 force-pushed the export-frames branch 2 times, most recently from dec8057 to c4d38b0 Compare January 26, 2025 02:41
@evilpie evilpie added the waiting-on-review Waiting on review from a Ruffle team member label Jan 26, 2025
@fries1234 fries1234 force-pushed the export-frames branch 2 times, most recently from a1d4e4b to 42466cb Compare January 27, 2025 23:36
@danielhjacobs danielhjacobs added T-perf Type: Performance Improvements A-desktop Area: Desktop Application labels Jan 30, 2025
This commit adds a frame cache which limits the amount of frames that
can be rendered into memory before writing to disk and getting more
frames. This can save memory on big 4k frames, especially if you have
alot of them as it can fill up 32 gigs of ram pretty fast.
Copy link
Collaborator

@adrian17 adrian17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, sorry for looking at this with such a delay.

I'm trying to understand exactly what this is doing, and - cache is a bit of a bad name, right? It's more of batching control?

To be sure, is there any advantage of storing these together at all, and not saving and throwing away each frame as soon as it's captured? It's possible we shouldn't have been storing frames in the Vec in the first place.

And assuming there is, I think the new logic is kinda confusing. If you call take_screenshot several times, won't it skip skipframes frames each time?

EDIT: Talking on discord, looks like it really might make sense to just stop storing multiple frames entirely and just do something like this instead (pseudocode):

for _ in 0..opt.skipframes { run_frame(); }
for i in 0..opt.frames {
    run_frame();
    let frame = take_screenshot();
    write(frame, i, opt.frames); // does the `opt.frames == 1` check inside, or maybe we can just always emit frame number in filename
}

@fries1234
Copy link
Author

fries1234 commented Feb 7, 2025

I'm trying to understand exactly what this is doing, and - cache is a bit of a bad name, right? It's more of batching control?

Yeah, I'd say it is doing batching stuff. I coded this because I was trying to export 4k frames from a SWF file and I ran out of memory because 4k frames are big and thousands of them is not kind to 32 gigs of ram.

To be sure, is there any advantage of storing these together at all, and not saving and throwing away each frame as soon as it's captured? It's possible we shouldn't have been storing frames in the Vec in the first place.

I don't know if there's any advantage to doing this. I would say just writing frames to the disk after every screenshot would be fine. Maybe there might be a reason to do a batching thing if I/O is a concern?

And assuming there is, I think the new logic is kinda confusing. If you call take_screenshot several times, won't it skip skipframes frames each time?

I think the capture_swf function does compensate for that when calling the take_screenshot function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-desktop Area: Desktop Application T-perf Type: Performance Improvements waiting-on-review Waiting on review from a Ruffle team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants