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

Make combination of video + audio streams more robust; fix incompatibility with pyav >= 13.0.0 #3953

Closed
wants to merge 2 commits into from

Conversation

behackl
Copy link
Member

@behackl behackl commented Oct 13, 2024

This rewrites the remuxing and combination of the video + audio streams that have been produced throughout Scene.render, which is happening in the SceneFileWriter.combine_to_movie method. The rewritten version now works (at least locally) for pyav >= 12.0.0, while the previous version apparently only worked for pyav >= 12.0.0, pyav < 13.0.0.

As the RTD render images apparently predominantly had a more recent release of pyav, this is what broke the docbuilld for recent builds.

The rewritten version

  • differs from the old one primarily in the way how audio streams are added to the final output container. In cases where we target mp4 output, instantiating the stream directly from the stream given in the auxiliary audio file broke rendering, so now we instantiate it by passing codec name and sample rate explicitly.
  • additionally supports cases where audio files with multiple audio channels are created.

@behackl behackl added pr:bugfix Bug fix for use in PRs solving a specific issue:bug enhancement Additions and improvements in general labels Oct 13, 2024
@behackl behackl added this to the v0.19.0 milestone Oct 13, 2024
@behackl behackl marked this pull request as draft October 13, 2024 14:48
@behackl
Copy link
Member Author

behackl commented Oct 13, 2024

I've been debugging this for a bit longer now. While I did successfully fix the verison issue outside of the RTD build container, I can still reproduce it consistently inside of the container. I don't see any obvious differences between the setups any more; it would probably be a good idea to consult our colleagues from pyav and see whether they can tell us more about this.

I'll prepare a minimal example when I have some more time. For now, if anyone else wants to give this a shot; here is my container setup:

$ docker run --rm -it -v /home/behackl/code/manim:/home/docs/manim --user root readthedocs/build:ubuntu-22.04-2024.01.29 bash
# apt install libpango1.0-dev graphviz
# asdf plugin-add python
# su docs
$ bash
$ asdf install python 3.11.9
$ asdf global python 3.11.9
$ cd manim
$ pip install --upgrade --no-cache-dir pip setuptools sphinx
$ pip install --exists-action=w --no-cache-dir -r docs/rtd-requirements.txt
$ pip install --exists-action=w --no-cache-dir -r docs/requirements.txt
$ pip install --upgrade --upgrade-strategy only-if-needed --no-cache-dir .
$ cd .. && mkdir debug && cp manim/example_scenes/debug.py debug/debug.py
$ cp manim/docs/source/_static/click.wav debug/
$ cd debug
$ manim debug.py

where I have prepared the file debug.py in my example_scenes directory containing

import av

from manim import *

av.logging.set_level(av.logging.DEBUG)
config.verbosity = "DEBUG"

class SoundExample(Scene):
    def construct(self):
        dot = Dot().set_color(GREEN)
        self.add_sound("click.wav")
        self.add(dot)
        self.wait()
        self.add_sound("click.wav")
        dot.set_color(BLUE)
        self.wait()
        self.add_sound("click.wav")
        dot.set_color(RED)
        self.wait()

@behackl
Copy link
Member Author

behackl commented Oct 14, 2024

Superseded by #3956.

@behackl behackl closed this Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Additions and improvements in general pr:bugfix Bug fix for use in PRs solving a specific issue:bug
Projects
Status: Rejected
Development

Successfully merging this pull request may close these issues.

1 participant