Add support for encoding partial movie files in parallel - #4899
Conversation
… files concurrently with rendering Add a max_inflight_encoders config option (default 1) bounding how many partial movie files may be encoding while the scene continues rendering. The default keeps the existing serial behavior: each animation's file is fully encoded and closed before the next animation starts. Values > 1 overlap encoding with rendering; 4 is a good value on typical hardware and cuts wall-clock time on an **encode-heavy** 30-animation benchmark from 9.3 s to 4.1 s (-56 %). Output is byte-identical in both modes for mp4, mov, transparent mov, gif, and PNG; webm bytes differ run-to-run either way because the Matroska muxer generates random TrackUIDs. Each partial movie file is now encoded by a self-contained _PartialMovieEncodeJob owning its container, stream, bounded frame queue, and worker thread. - Closing a stream seals the job and defers its join - Jobs are joined oldest-first once the cap is reached, when a cache lookup or a new stream targets the same path, and all are drained at scene end and on the interactive rerun path (which previously replaced the file writer while jobs could still be writing). - Worker failures are captured first-exception-wins across the encode, flush, and close stages and re-raised at join - A failed job deletes its truncated partial file so a later run cannot cache-hit it. - The former SceneFileWriter internals (listen_and_write, encode_and_write_frame, queue, writer_thread, video_container, video_stream) are removed as part of this restructuring (as equivalents now live in the _PartialMovieEncodeJob class). Added tests covering failure propagation and its precedence, removal of failed partial files, the encoder cap and FIFO join order, same-path guards, cache behavior across renders, success logging, and encoder-thread cleanup.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Note, the python 3.13 builds are failing precisely because of the error identified in this other PR I submitted a couple of days ago): #4896 The basics of the issue are:
I validated this by running a python 3.13, with the commit prior to this PR, I ran the two runs for the test, and the second render, instead of cache hitting all 32 plays, it re-renders 15 of them, which triggers the assert in this new test. The diff is attached. The particular scene I ran for both runs is this Would you prefer I merge that PR into this one? or should this one wait on the other PR to complete? (The .txt file wouldn't upload, so instead the following image is an extract from the diff file [taken from Notepad++]) |
|
bench is rerunning on this, and I want to check feasibility of adding a command line flag for the new config option |
|
Sounds like a good idea! I'd go for |
Expose the max_inflight_encoders config option (the cap on how many partial movie files may be encoded concurrently while the scene continues rendering) as a render CLI flag, so it can be set per-run without a config file. The flag validates with IntRange(min=1) and defaults to None so config-file values still apply when it is omitted; tests cover flag-beats-config precedence, config preservation when absent, and rejection of non-positive values.
|
@behackl bench ran, and cli flag added. The updated full video bench results are summarised below:
After updating for all the latest performance related PRs it now means that running this parallel cached is faster than the default serialised uncached result, and the updated performance improvement when parallelising the encoding is between 11.5% & 14%. The behaviour of the feature is:
|
behackl
changed the title
|
I’ve pushed a few follow-up improvements from review. Serial encoding now preserves the previous unbounded frame queue when max_inflight_encoders is 1, keeping the default rendering flow as close as possible to existing behaviour. Parallel encoding uses a configurable bounded queue through the new I also strengthened failure handling: if an early job join fails, all remaining in-flight jobs are drained while preserving the original exception, and a failure to delete an incomplete partial file no longer masks the encoder error. Finally, the integration test now actually runs with parallel encoding enabled at cap 3, uses a smaller scene, verifies exact cache reuse, and checks that no encoder threads survive the render. I'll read over everything once more tomorrow, but this is looking pretty good to me already; thanks for your efforts! |
|
Pushed the two fixes mentioned on discord, plus the tests as three separate commits: d8e8a38: fail-fast on encoder failure. write_frame now checks whether the current job's worker has captured an exception; if so it seals the job, detaches it, and joins it, so the failure surfaces at the first write after it was captured (usually the animation that caused it) instead of at a join site several animations later. join() already handles unlinking the truncated partial and re-raising. 06cdb67: teardown for renders that abort mid-play. New SceneFileWriter.abort_encode_jobs(), called from Scene.render() when any exception (including KeyboardInterrupt) escapes, and from the rerun handler. Previously a mid-play exception (e.g. from a user updater) left the current encode job unsealed, and its user process worker stayed blocked on queue.get(), then the interpreter then hangs at exit. That hang is the same shape on main with the serial encode thread, so this fixes a pre-existing issue too. The aborted job's partial file is deleted unconditionally: it's structurally valid but truncated, so it would cache-hit on a later run. The rerun path still re-raises encoder failures (a rerun shouldn't continue past corrupt output); the exception path logs them so the primary exception stays visible. I reproduced the hang before the fix, a scene whose updater raises during the second play prints its traceback and then hangs until killed. After the fix it exits nonzero promptly. That's now a subprocess test with a timeout (test_mid_play_exception_does_not_hang_process). One subtlety baked into the test scene: the updater only raises when dt > 0, because compile_animation_data pre-runs updaters with dt=0 before the partial movie stream opens. aa73ed3: the tests mentioned above: cap-1 vs cap-4 byte-identity on the same scene, an end-to-end render at cap 4 (restoring end to end coverage of the raised cap now that the cache test runs at cap 3), plus the smaller gaps (freeze-frame num_frames repetition, is_already_cached return values, guard paths). I also re-ran my 32-partial bench scene before/after: partial-file manifests byte-identical for every format except webm (which differs run-to-run on my box even at identical code). |
behackl
left a comment
behackl
left a comment
There was a problem hiding this comment.
I've reviewed your changes, everything looks fine to me now. Good job, thank you very much!
In order to have the rendering deep dive guide reflect the most recent changes I've pushed one more commit to update it accordingly.
Happy to get this merged once the pipeline passes! 🚀
behackl
added
highlight
3 participants
Overview: What does this pull request change
Adds a new ability to parallelise the encoding after the render pass to speed up the encoding output of the scene, with a new max_inflight_encoders config option (defaulting to 1) bounding how many partial movie files may be encoding while the scene continues rendering.
The default value of 1 keeps the existing serial behaviour: each animation's file is fully encoded and closed before the next animation starts. Values greater than 1 overlap encoding with rendering. In testing for the performance testing a value of 4 was used.
Output is byte-identical in both modes for mp4, mov, transparent mov, gif, and PNG. Output for webm output has bytes differing run-to-run either way because the Matroska muxer generates random TrackUIDs.
Each partial movie file is now encoded by a self-contained _PartialMovieEncodeJob owning its container, stream, bounded frame queue, and worker thread.
Added tests covering failure propagation and its precedence, removal of failed partial files, the encoder cap and FIFO join order, same-path guards, cache behavior across renders, success logging, and encoder-thread cleanup.
Added the new config flag to the list of all config options in docs/source/guides/configuration.rst (but noted that some other new flags [format, media_embed, save_sections, seed, disable_caching_warning, and zero_pad] are missing from this manual list)
Motivation and Explanation: Why and how do your changes improve the library?
In the library as it stands, the default serial approach where rendering and encoding are performed serially, leaves performance on the table for those who have the hardware to enable parallelism.
During testing the biggest speed improvement is in encoder heavy scenes, where caching is not used. Using an encode-heavy 30-animation benchmark the wall-clock time was reduced from 9.3 s to 4.1 s (-56 %).
In a real world render heavy video generation, consisting of 8 scenes, some with many mobjects, from an as yet unreleased personal project, rendered serially, the wall clock improvement was more modest (~8% - 11% reduction depending on whether or not cache was utilised).
The full video render performance results are:
--disable_caching--disable_caching- cached: ~41.5 mins
- uncached: ~ 24.5 mins
- ~3.5 min saved per full-video pass when caching was on
- ~2.5 mins saved when caching was off
For the encoder heavy performance benchtest this class was used:
Links to added or changed documentation pages
Expected:
Gen Index
Configuration
ManimConfig
SceneFileWriter
manim._config.utils source
manim.scene.scene source
manim.scene.scene_file_writer source
Unexpected (likely due to commits to main after 0.20.1):
Camera
Images
Further Information and Comments
Reviewer Checklist