Repository navigation
perf: encode ProRes MOV exports across all cores (4x faster) - #2528
Conversation
Config::count leaves thread_type at 0, so prores_ks never threaded and MOV export ran on one core. Use frame threading, and convert each frame into a pooled buffer the encoder no longer references, since frame threads hold the sent frame until a worker encodes it. 1080p30: 13.2 -> 54.3 fps. 4K30: 3.2 -> 11.8 fps. Encoded packets are byte-identical to the single-threaded encoder.
|
@Aqua-123 is attempting to deploy a commit to the Cap Software Inc Team on Vercel. A member of the Team first needs to authorize it. |
| let mut mov = MOVFile::init(path.clone(), |output| { | ||
| ProResEncoder::builder(info).build(output) | ||
| }) |
There was a problem hiding this comment.
If the host reports only one CPU, this test uses one encoder thread and never exercises the delayed frame reuse that this PR fixes. It can pass even if unsafe reuse returns. Let the test request at least two encoder threads so it checks that failure on every host.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/enc-ffmpeg/src/mux/mov.rs
Line: 145-147
Comment:
**Test can miss threaded reuse**
If the host reports only one CPU, this test uses one encoder thread and never exercises the delayed frame reuse that this PR fixes. It can pass even if unsafe reuse returns. Let the test request at least two encoder threads so it checks that failure on every host.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Valid in theory, but every CI runner and supported Mac has ≥2 cores, and on those the test fails 3/3 against the unsafe reuse. Forcing a thread count would mean adding an encoder option only for the test, so leaving it as is.
There was a problem hiding this comment.
That’s fair. Given the supported environments and CI runners all have at least two cores, and the regression reproduces reliably there, the test exercises the threaded reuse path without adding a test-only encoder option. I’ll withdraw this finding.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
| if let Ok(keep) = std::env::var("MOV_BENCH_KEEP") { | ||
| fs::copy(&path, keep).ok(); | ||
| } | ||
| fs::remove_file(&path).ok(); |
There was a problem hiding this comment.
MOV_BENCH_KEEP silently ignores a failed copy, then deletes the exported file. A missing destination directory or unwritable path leaves no retained output and no error, which blocks the requested hash comparison. Report the copy failure instead of continuing as if the file was kept.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/export/examples/mov-export-benchmark.rs
Line: 106-109
Comment:
**Failed copies lose output**
`MOV_BENCH_KEEP` silently ignores a failed copy, then deletes the exported file. A missing destination directory or unwritable path leaves no retained output and no error, which blocks the requested hash comparison. Report the copy failure instead of continuing as if the file was kept.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| #[arg(long, default_value = "5")] | ||
| runs: usize, |
There was a problem hiding this comment.
--runs 0 is accepted, but leaves fps_samples empty and panics when printing the median. Reject zero runs during argument parsing, before generating the test video.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/export/examples/mov-export-benchmark.rs
Line: 22-23
Comment:
**Zero runs crashes**
`--runs 0` is accepted, but leaves `fps_samples` empty and panics when printing the median. Reject zero runs during argument parsing, before generating the test video.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.|
Thanks for the PR. Greptile currently has this at 4/5, and we only merge PRs once Greptile scores the current head 5/5. Please address the remaining Greptile feedback, then comment |
|
@greptileai Addressed the review on 975ac5b: |
|
@richiemcilroy 5/5 on greptile now |
|
Measured this on an M4 Max (16 cores) with real Studio recordings, main vs this branch,
Every variant decodes to the same frames as main (framemd5 identical, 1080p and 4K). The 1080p export goes 102.6 s to 12.8 s. Past 8 threads the export no longer gets faster, but each extra thread holds about 130 MB at 4K, so I pushed e9bfb4d capping frame threads at 8. That keeps the full speedup and takes 1 GB off the 4K peak. |
|
@greptileai review |
1 similar comment
|
@greptileai review |
… GB Macs do not swap on 4K exports
|
Pushed a follow-up: the frame thread count now also respects RAM (threads may use at most 1/16 of physical memory, at about 16 bytes per output pixel each). An 8 GB Mac exporting 4K gets 4 threads instead of 8; 16 GB and up is unchanged at 8. Unknown memory falls back to 1 thread (main's behaviour). Tests cover the hostile sizes too (0x0, 8K on 1 GB, u32::MAX). @greptileai review |
richiemcilroy
left a comment
There was a problem hiding this comment.
Verified: frame-identical output vs main, CI and engine/real-capture suites green, Greptile 5/5. Thanks!
Summary
MOV (ProRes 4444) export from the GPUI editor ran on a single CPU core. This PR makes it use all cores. On an Apple M5, 1080p30 export is 4.1× faster and 4K30 export is 3.7× faster. The encoded bitstream is unchanged.
testsrc2, M5, release)Root cause
ProResEncoderBuildercalledset_threading(Config::count(n)). In ffmpeg-next,Config::countkeepskind: Type::None, sothread_typeis written as0. Forprores_ks, libavcodec treatsthread_type == 0as "no threading" and resetsthread_countto 1. A profile of the old build showed one thread spending about 95% of its samples in libavcodec and swscale while the other cores were idle.The other
Config::countcall sites (libx264, libx265, AAC, Opus) are not affected. libx264/libx265 manage their own threads fromthread_count(FF_CODEC_CAP_AUTO_THREADS), and the audio encoders don't thread.Change
prores.rsturns on frame threading forprores_ks. Slice threading was measured too. It gave no speedup.converted_frame_poolframe, so the next swscale call would overwrite a frame that was still being encoded. The pool now reuses a converted frame only whenav_frame_is_writablesays the encoder has released it, and allocates a new one otherwise. Without this check the output was corrupted. That was caught by comparing packet hashes and is covered by the new test.Output is unchanged
ffmpeg -i out.mov -map 0:v -c copy -f framemd5 -) of a 90-frame 1080p export match exactly before and after.mux::mov::tests::every_queued_frame_is_encoded_with_its_own_pixelsencodes 24 differently shaded frames throughMOVFileand checks that each one decodes to its own shade. It fails 3/3 when the pool reuses frames without the writability check, and passes with this change.Trade-off: peak memory
Frame threading keeps roughly
threads + 2frames in flight, so peak RSS rises with core count and resolution. On the 10-core M5, 1080p went from 247 MB to 704 MB and 4K from 747 MB to 2.37 GB. A 4K thread sweep showed fps scaling all the way to 10 threads at about 180 MB per thread, so lowering the thread count trades speed for memory roughly linearly. The PR keepsavailable_parallelism(), which is what the code already asked for. Happy to add a cap if you'd prefer a lower memory ceiling.Benchmark
crates/export/examples/mov-export-benchmark.rsbuilds a synthetic studio project and times fullMovExportSettings::exportruns.MOV_BENCH_KEEP=<path>keeps the last output for hash comparison.Checks
cargo test --release -p cap-enc-ffmpeg -p cap-export: 152 and 40 tests pass.cargo clippy --release -p cap-enc-ffmpeg -p cap-export --all-targets --no-deps -- -D warnings: clean.cargo fmt --all -- --check: clean.🤖 Generated with Claude Code


Confidence Score: 5/5
The PR appears safe to merge, with no new blocking issues found.Summary
The PR enables parallel ProRes encoding while protecting frames that workers still use. The follow-up limits threads by physical RAM and output size.
Reviews (5) · Last reviewed commit: "perf(enc-ffmpeg): size ProRes frame thre..." · Reviewed by Greptile