manim
manim copied to clipboard
ManimCommunity
Let `SceneFileWriter` access `ffmpeg` via `av` instead of via external process
Potentially, this is a pretty substantial improvement to the overall usability of Manim:
-
avprovides wheels for a lot of architectures that come with a statically linked version offfmpeg(-> users would no longer need to install it on their own) - As a result, we could also throw all of the extra steps in our CI pipelines that setup ffmpeg away
- Plus documentation build times should increase too as less setup is needed; but not only that, ...
- Piping to an external process is potentially slower than inner-process communication; in a non-exact measurement on my local device I found
OpeningManiminexample_scenes/basic.pyto require only around 7.7sec CPU time on the currentmainbranch vs. 5.7sec CPU time on this branch. (Unsure whether this is significant, we'll see in the documentation build times.)
Depending on how experiments go, there is a lot more to do here:
- [x] Refactor partial movie file creation
- [x] Figure out how video filters work exactly (we currently need
-vf vflipfor OpenGL rendering ... or we solve that differently) - [ ] Test creation of movies with other codecs
- [x] Refactor combination of partial movie files into final output file
- [ ] Make sure users can also interact with the av settings somehow
- [ ] Simplify installation documentation (simply
pip install manim, plus optional LaTeX installation ... 👀)
I'd gladly join forces on that, but could we share a branch where we can both edit on?
I've sent an invitation to the repo this branch lies on, behackl/manim -- as soon as you accept, you should have write permissions to the branch too. I've created a forum post in our Discord and pinged you there, just to have a place to chat about this too. 👍
@jeertmans locally you are probably on an older version of cairo, which we disabled running graphical tests for (check the output, there are ~200 skipped tests).
The tests currently seem to fail because of a mismatch in the number of generated frames in the graphical tests. Curious!
Ah yes... indeed. But https://github.com/ManimCommunity/manim/blob/main/.github/scripts/ci_build_cairo.py install Cairo 1.18, which is not even available from apt (Ubuntu 22.04). The latest version available is 1.16.
Ah yes... indeed. But https://github.com/ManimCommunity/manim/blob/main/.github/scripts/ci_build_cairo.py install Cairo 1.18, which is not even available from
apt(Ubuntu 22.04). The latest version available is 1.16.
I have raised an issue about that (#3521), as I think this may be good to document this.
The tests currently seem to fail because of a mismatch in the number of generated frames in the graphical tests. Curious!
Yeah I don't know why it occurs... I have updated the reference frames (not sure if this is the right solution) in https://github.com/ManimCommunity/manim/pull/3501/commits/904cfb46ae8db7dfac01036671991cf81a7823de.
I tested the new manim in a large project I did, and encountered some rendering issue, so will reach back to you when I (hopefully) fixex them!
Rendering for non-standard output formats needs some work; I believe that the -c copy flag currently is just getting ignored. The .mov generated when rendering with -t appears to be broken.
@behackl maybe that might be good to save video files in various format with the release version of Manim, and compare the output video files with this new version (this is assuming that everything was done correctly in prior versions of Manim)
@behackl maybe that might be good to save video files in various format with the release version of Manim, and compare the output video files with this new version (this is assuming that everything was done correctly in prior versions of Manim)
Indeed, I am not even sure in how far correctness of generated files is tested currently. I am thinking of adding some video tests (using the pyav-equivalent of ffprobe) to check that things like the generated format + codec etc. is actually correct. I'm on this! :+1:
@behackl maybe that might be good to save video files in various format with the release version of Manim, and compare the output video files with this new version (this is assuming that everything was done correctly in prior versions of Manim)
Indeed, I am not even sure in how far correctness of generated files is tested currently. I am thinking of adding some video tests (using the
pyav-equivalent offfprobe) to check that things like the generated format + codec etc. is actually correct. I'm on this! 👍
Nice, perfect! Tell me if I can help (I don't have much time, but I'd love to have the release :p)
I might have some time in the next few days to finish this, finally. To do so, I propose the following:
- The task that remains here is the creation of an (ideally parameterized) test to cover all possible options affecting the renderer (mainly with respect to the different codecs and transparency). There are already a bunch of individual tests, but I am not sure that all possible options are covered. I'll look into this.
- The tests appear to be failing due to a control data mismatch. Needs to be investigated.
After that, we should merge this: generally speaking, this PR is in a pretty good shape already with the more common rendering options working as intended.
Allowing users to pass custom options to av would be a nice-to-have / new feature anyways -- and a more radical installation instruction rewrite should also rather happen on a separate PR.
Hello @behackl, this would be great if we can finalize this PR in the next days/weeks.
I would like to emphasize a few points that are, in my opinion, important to consider for this PR:
- Fix #3648: Manim somehow produces empty videos, which are silently skipped* when concatenating because FFMPEG's CLI does a lot of cleaning, that PyAV does not do automatically. This is an error that one user of Manim Slides discovered https://github.com/jeertmans/manim-slides/issues/390 (because Manim Slides already uses PyAV) and the easiest work-around I found was to probe each file and remove files without any video stream;
- Implement #3347 by directly writing audio to partial movies files. I know I am biased because this is a feature request of mine, but IMO that would really benefit Manim Slides, and also potentially Manim users that want to manipulate partial movie files :-)
I initially planned to create and use a separate PyAV library, to reduce to code-complexity (see https://github.com/jeertmans/avfilters), but tackling all possible video formats (especially multiple video streams, audio streams, and subtitles) can be very challenging, and is out of the scope of Manim or Manim Slides. Still, I think you may find interesting code, especially for testing if two media are the same, or else (see https://github.com/jeertmans/avfilters/blob/main/tests/utils.py).
Nevertheless, I think this is important to note that testing for equality (or close) between media is very hard, as many encodings will somehow be lossy.
*: Actually, using ffmpeg to concat four 1 second files + one empty file (see https://github.com/jeertmans/avfilters/blob/48d6f01bbb755660e5b5493d6b49a5629f228ae0/tests/test_concatenate.py#L67-L84) results in a 3.9 seconds files (the empty file seems to the
Let me know what you think :-)
Hey @jeertmans!
-
Regarding #3648: we've found that these partial movie files with run_time 0 are also causing issues with the current ffmpeg CLI-based implementation. From my point of view, these files are pathological; there should not be any animations being rendered with a
run_timeof 0 seconds. There has been some effort (#3491) to disallow creating these files, and I am open to introducing more restrictions if necessary (I'll check whether the problem reported in manim-slides persists with #3491). -
As for #3347, it looks to me as if the main problem with an implementation of this functionality -- the requirement of a mechanism that cuts audio of tracks playing across multiple partial movie files and redistributes the pieces correctly -- is unrelated to this PR. In the spirit of getting the av migration done, I'd rather declare this as out of scope for this PR. I'm not against the feature itself though, I can see how it is useful to have the audio in the partial movie files.
Thanks for the pointer to jeertmans/avfilters! You are, of course, right -- video tests are inherently hard. I'm already more or less satisfied with proper metadata tests (which you provided an initial rewritten version via av of on this branch here already :-)). Moreover, we have our frame comparison tests still in place anyways.
Hey @jeertmans!
Thanks for you comprehensive reply, let me address the two points separately.
- Regarding FFMPEG concat issue - Manim produces empty media and silently ignores warning #3648: we've found that these partial movie files with run_time 0 are also causing issues with the current ffmpeg CLI-based implementation. From my point of view, these files are pathological; there should not be any animations being rendered with a
run_timeof 0 seconds. There has been some effort (Fix animations with zero runtime length to give a useful error instead of a broken pipe #3491) to disallow creating these files, and I am open to introducing more restrictions if necessary (I'll check whether the problem reported in manim-slides persists with Fix animations with zero runtime length to give a useful error instead of a broken pipe #3491).
I wasn’t aware of #3491 and this is, to me, a valid solution to #3648. The idea of just that we need to report that error to the user in a meaningful way, and not a libav error :) Which should now be the case here.
- As for Add audio in partial_movie_files, not at the end #3347, it looks to me as if the main problem with an implementation of this functionality -- the requirement of a mechanism that cuts audio of tracks playing across multiple partial movie files and redistributes the pieces correctly -- is unrelated to this PR. In the spirit of getting the av migration done, I'd rather declare this as out of scope for this PR. I'm not against the feature itself though, I can see how it is useful to have the audio in the partial movie files.
Sure, adding audio to each segment might not be an easy feature, and probably needs to rewrite the actual thing. If you think this is not part of this PR’s, which I sort of agree, then I don’t mind creating another PR for that feature, once this one is closed (as the audio feature will also use av). Nonetheless, I just wanted to point that so we can keep in mind that you might add an audio stream in the future, and so hopefully we do not write code that is against that :)
- there is a nice parametrized test for video metadata given different formats and cli-options now. not all formats (e.g., gif) are tested yet.
- the ubuntu runners apparently segfault 😢
- transparent vp9-encoded files are currently broken as the new test reveals.
regardless, progress is being made here!
Good news: the pipeline looks already much better.
Still to test:
-
--format=gif - Scenes with included audio
Bad news: something weird is going on with transparency in webm-files. It appears that this branch produces partial video files that are actually transparent, but this transparency is lost as soon as the files are recombined to the final output.
Indeed, even though we are using the yuva420p pixel format to save the partial movie files, opening such a file with av and inspecting the pixel format of the video stream returns
>>> container = av.open("/home/behackl/code/manim/example_scenes/media/videos/bla/720p30/partial_movie_files/StarScene/4117703622_1377611505_3088195676.webm")
>>> container.streams[0].pix_fmt
'yuv420p'
I am not quite sure how to salvage this yet.
With the change in 4c458366, manim correctly produces transparent vp9-encoded (webm) files, verified by inspection -- which, however, I can't test because I can't seem to find a way to extract the first frame with pyav in a way that exposes the alpha channel.
(And conveniently, calling VideoFrame.to_ndarray(format="rgba") on a frame extracted from the video causes a segmentation fault...)
With the change in 4c45836, manim correctly produces transparent vp9-encoded (webm) files, verified by inspection -- which, however, I can't test because I can't seem to find a way to extract the first frame with pyav in a way that exposes the alpha channel.
(And conveniently, calling
VideoFrame.to_ndarray(format="rgba")on a frame extracted from the video causes a segmentation fault...)
Great work, thanks!! Here is what I found about transparency with PyAV/ffmpeg:
- https://github.com/PyAV-Org/PyAV/issues/500#issuecomment-513621698
- and the first comment in https://www.reddit.com/r/ffmpeg/s/fgdOzMiuVZ
- https://github.com/PyAV-Org/PyAV/issues/1269#issuecomment-1918635446
Maybe this helps?
It did indeed help a little, thanks! But there is still a bit of mystery left. If I extract the first frame using
from av.codec.context import CodecContext
context = CodecContext.create("libvpx-vp9", "r")
packet = next(container.demux(video=0))
first_frame = context.decode(packet)[0].to_ndarray()
then I actually get the correct RGBA-values for the pixel in the upper left corner -- however, the red pixel in the bottom right corner now yields (255, 252, 0, 0) (in ARGB) instead of (255, 255, 0, 0); the red-value is slightly wrong now. Maybe this is some sort of color space issue. I'll play around with this some more.
It did indeed help a little, thanks! But there is still a bit of mystery left. If I extract the first frame using
from av.codec.context import CodecContext context = CodecContext.create("libvpx-vp9", "r") packet = next(container.demux(video=0)) first_frame = context.decode(packet)[0].to_ndarray()then I actually get the correct RGBA-values for the pixel in the upper left corner -- however, the red pixel in the bottom right corner now yields (255, 252, 0, 0) (in ARGB) instead of (255, 255, 0, 0); the red-value is slightly wrong now. Maybe this is some sort of color space issue. I'll play around with this some more.
Nice! Yes I also observed that it is hard to get the same pixel values, probably due to color space or maybe lossy encodings. This is why is use assert_all_close :)
- gifs are now covered by the parametrized test as well,
- for mp4 + mov audio works (just tested locally, not yet covered by the test)
- TODO: fix rendering the audio track for
--format=webm
Just a reminder to update with main and change the CI for MacOS-13 to MacOS-latest
I did change the pipeline to macos-latest again, but it seems that there are now problems with pango. We might want to delay having a working macos-latest pipeline until after this PR has been merged and deal with this in a follow-up PR.
- gifs are now covered by the parametrized test as well,
- for mp4 + mov audio works (just tested locally, not yet covered by the test)
- TODO: fix rendering the audio track for
--format=webm
@behackl Can I help you with one or more?
@jeertmans thanks, I have some time today to work on this today, I'm somewhat optimistic that this can be finished. :-)
Current state of things: In 5fe5bb5 I've produced some code that works in principle:
- We no longer attempt to add an audio stream to gifs,
- Audio for webm files works, but the current implementation (where an
pydub.AudioSegmentis exported to anopusfile) unfortunately requires ffmpeg. I'll investigate whether we can just write the audio files to.wavfiles and reencode them internally using pyav again.
I'd then like to modify the codec test to check for the presence of a non-empty audio track, but that should be comparatively easy as soon as the pipeline runs. Will keep sending updates here.
All of our file formats work as intended locally; I've split the codec test, there now is a separate one for gifs. For webm, mov, mp4 the parametrized test explicitly checks whether there is a non-silent audio track.
I think this is in a sufficiently final state to be reviewed / merged. I'd like to completely revamp our installation instructions in a follow-up. We should
- Significantly simplify the instructions for different operating systems,
- Investigate the required steps for installing in case there is no binary wheel for
pyavfor some particular OS/Python version combination, and document them.
Hey, overall amazing job here on this PR! I believe this will make Manim much easier to use for the average user :) Seeing as the tests passed (including the new ones), I tried to "break" the rendering system and came up with a case where it fails. Hopefully :crossed_fingers: it shouldn't be too hard to fix :) Other than that, LGTM! Thanks for your work on this PR :)
Small Wait time
def construct(self):
m = ManimBanner()
self.play(m.expand())
self.wait(1e-9) # this fails
Hey, overall amazing job here on this PR! I believe this will make Manim much easier to use for the average user :) Seeing as the tests passed (including the new ones), I tried to "break" the rendering system and came up with a case where it fails. Hopefully 🤞 it shouldn't be too hard to fix :) Other than that, LGTM! Thanks for your work on this PR :)
Small Wait time
def construct(self): m = ManimBanner() self.play(m.expand()) self.wait(1e-9) # this fails
Hello, does it fail to render (I.e., an error is raised) or just fails to add the wait time?
It full on fails. Good find!
Stack trace
╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ /Users/behackl/code/manim/manim/cli/render/commands.py:120 in render │
│ │
│ 117 │ │ │ try: │
│ 118 │ │ │ │ with tempconfig({}): │
│ 119 │ │ │ │ │ scene = SceneClass() │
│ ❱ 120 │ │ │ │ │ scene.render() │
│ 121 │ │ │ except Exception: │
│ 122 │ │ │ │ error_console.print_exception() │
│ 123 │ │ │ │ sys.exit(1) │
│ │
│ /Users/behackl/code/manim/manim/scene/scene.py:239 in render │
│ │
│ 236 │ │ │ return True │
│ 237 │ │ self.tear_down() │
│ 238 │ │ # We have to reset these settings in case of multiple renders. │
│ ❱ 239 │ │ self.renderer.scene_finished(self) │
│ 240 │ │ │
│ 241 │ │ # Show info only if animations are rendered or to get image │
│ 242 │ │ if ( │
│ │
│ /Users/behackl/code/manim/manim/renderer/cairo_renderer.py:268 in scene_finished │
│ │
│ 265 │ def scene_finished(self, scene): │
│ 266 │ │ # If no animations in scene, render an image instead │
│ 267 │ │ if self.num_plays: │
│ ❱ 268 │ │ │ self.file_writer.finish() │
│ 269 │ │ elif config.write_to_movie: │
│ 270 │ │ │ config.save_last_frame = True │
│ 271 │ │ │ config.write_to_movie = False │
│ │
│ /Users/behackl/code/manim/manim/scene/scene_file_writer.py:452 in finish │
│ │
│ 449 │ │ if write_to_movie(): │
│ 450 │ │ │ if hasattr(self, "writing_process"): │
│ 451 │ │ │ │ self.writing_process.terminate() │
│ ❱ 452 │ │ │ self.combine_to_movie() │
│ 453 │ │ │ if config.save_sections: │
│ 454 │ │ │ │ self.combine_to_section_videos() │
│ 455 │ │ │ if config["flush_cache"]: │
│ │
│ /Users/behackl/code/manim/manim/scene/scene_file_writer.py:671 in combine_to_movie │
│ │
│ 668 │ │ │ return │
│ 669 │ │ │
│ 670 │ │ logger.info("Combining to Movie file.") │
│ ❱ 671 │ │ self.combine_files( │
│ 672 │ │ │ partial_movie_files, │
│ 673 │ │ │ movie_file_path, │
│ 674 │ │ │ is_gif_format(), │
│ │
│ /Users/behackl/code/manim/manim/scene/scene_file_writer.py:636 in combine_files │
│ │
│ 633 │ │ │ │ output_container.mux(packet) │
│ 634 │ │ │
│ 635 │ │ else: │
│ ❱ 636 │ │ │ for packet in partial_movies_input.demux(partial_movies_stream): │
│ 637 │ │ │ │ # We need to skip the "flushing" packets that `demux` generates. │
│ 638 │ │ │ │ if packet.dts is None: │
│ 639 │ │ │ │ │ continue │
│ │
│ in demux:166 │
│ │
│ in av.container.core.Container.err_check:286 │
│ │
│ in av.error.err_check:328 │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
FileNotFoundError: [Errno 2] No such file or directory:
'/Users/behackl/code/manim/example_scenes/media/videos/bug/720p30/partial_movie_files/BugScene/partial_movie_file_list.t
xt'; last error log: [concat] Impossible to open
'file:/Users/behackl/code/manim/example_scenes/media/videos/bug/720p30/partial_movie_files/BugScene/543634251_3305253770
_1995303018.mp4'