-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Make livestreaming a thing. #778
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
Changes from 119 commits
7725ec2
a5523e2
ab07add
e275d11
b4f1fce
8341da8
1ed3f68
fd14d66
e8ba6f3
4a4be30
49e8759
22bab46
2254be0
5f77979
0d08bc7
3c1fd9b
8d5507a
75eeb79
a6d1fa3
44ce2d1
eb7a137
4deae04
995db49
7490898
6f4ce3b
3a67a2f
4c04a9f
7b22388
4f821bd
3437a19
9a381f6
42533e9
7c9cff1
5292be3
33bc6f3
2094a13
2df207e
7f27a50
173fc05
24e6c67
502646e
866933b
e52b4b9
38edbc8
2cb6459
a501084
0342be0
9671ad1
78fc7fb
7e21430
a7384da
6c9a140
9dc7e16
74cded9
fc37ec2
6f6ea55
be04f30
cd612ff
f311539
ea5e2c7
034a4d1
553e309
4267ae8
3903bca
0e9ef90
424a671
71dfb58
7f78dd8
5a93fb7
9677699
b68b93a
289d11e
962a0f7
945621b
350da82
8a90a9f
c8821a6
c18216e
f22322f
d0ead32
eec5f4b
d05600e
2c60617
a48ca1e
066f195
1c08a67
ed2d86f
9c23094
5b6285e
ce8cbfd
6135afc
6c6a875
212e894
4e33544
a789aff
e16c556
d76772e
aaca74a
040281f
c2782db
1b4e80b
000539c
08dc895
0a8b49a
9584944
973ae77
b011fc4
9f352c8
97b41c8
2312d9d
1baeea9
b353209
07bd4c8
85a0c8b
a5e0dc7
434155d
ce6c76a
f6f5553
eb7cf71
eb8430f
4810b56
638a03b
d600098
48d2009
82a7960
6b61eae
ad36871
7daf498
449954b
e347ed9
9ef7af2
069b771
3e7d9d8
d7313db
e796f60
6e86313
9b4bd67
cda56b8
d514586
ffa1cd1
3bd9d1c
44ea5cf
7719958
f0e6df6
c6e35a7
eb76955
515f836
641d68e
67f6f14
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| ) | ||
| from manim.utils.file_ops import open_file as open_media_file | ||
| from manim._config.main_utils import parse_args | ||
| from manim.stream_starter import livestream | ||
|
|
||
| try: | ||
| from manim.grpc.impl import frame_server_impl | ||
|
|
@@ -76,6 +77,11 @@ def main(): | |
|
|
||
| else: | ||
| config.digest_args(args) | ||
|
|
||
| if args.livestream: | ||
| livestream(use_ipython=args.use_ipython) | ||
| return | ||
|
|
||
|
Comment on lines
+48
to
+52
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would need to move to cli/render/commands.py -- just keep the upstream changes for this file entirely |
||
| input_file = config.get_dir("input_file") | ||
| if config["use_js_renderer"]: | ||
| try: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -187,6 +187,8 @@ def _parse_args_no_subcmd(args: list) -> argparse.Namespace: | |
|
|
||
| parser.add_argument( | ||
| "file", | ||
| nargs="?", | ||
| default="", | ||
NeoPlato marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| help="Path to file holding the python code for the scene", | ||
| ) | ||
| parser.add_argument( | ||
|
|
@@ -433,6 +435,20 @@ def _parse_args_no_subcmd(args: list) -> argparse.Namespace: | |
| "section of the config file to define the output folder structure", | ||
| ) | ||
|
|
||
| # Overrides default false streaming so streaming can happen | ||
| parser.add_argument( | ||
| "--livestream", | ||
| action="store_true", | ||
| help="Run in streaming mode", | ||
| ) | ||
|
|
||
| # Optional use of IPython for streaming | ||
| parser.add_argument( | ||
| "--use-ipython", | ||
| action="store_true", | ||
| help="Use IPython for streaming", | ||
| ) | ||
|
|
||
|
Comment on lines
+454
to
+467
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is entire file doesn't even exist anymore. these options would need to be moved to one of the four files I mentioned for categories under the render command. Argparse has |
||
| # Specify the verbosity | ||
| parser.add_argument( | ||
| "-v", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -280,6 +280,7 @@ class MyScene(Scene): | |
| "save_pngs", | ||
| "scene_names", | ||
| "show_in_file_browser", | ||
| "streaming_dir", | ||
| "sound", | ||
| "tex_dir", | ||
| "tex_template_file", | ||
|
|
@@ -539,6 +540,7 @@ def digest_parser(self, parser: configparser.ConfigParser) -> "ManimConfig": | |
| "movie_file_extension", | ||
| "background_color", | ||
| "js_renderer_path", | ||
| "streaming_dir", | ||
| ]: | ||
| setattr(self, key, parser["CLI"].get(key, fallback="", raw=True)) | ||
|
|
||
|
|
@@ -560,6 +562,23 @@ def digest_parser(self, parser: configparser.ConfigParser) -> "ManimConfig": | |
| if val: | ||
| setattr(self, "ffmpeg_loglevel", val) | ||
|
|
||
| streaming_config = { | ||
| opt: parser["streaming"].get(opt, fallback="", raw=True) | ||
| for opt in [ | ||
| "streaming_client", | ||
| "streaming_protocol", | ||
| "streaming_ip", | ||
| "streaming_port", | ||
| ] | ||
| } | ||
| url = parser["streaming"].get("streaming_url", fallback="", raw=True) | ||
| sdp_name = parser["streaming"].get("sdp_name", fallback="", raw=True) | ||
| streaming_config["streaming_url"] = url.format(**streaming_config) | ||
| streaming_config["sdp_path"] = os.path.join( | ||
| self.get_dir("streaming_dir"), sdp_name.format(**streaming_config) | ||
| ) | ||
|
Comment on lines
+586
to
+600
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fine, but the file still needs to resolve conflicts -- keep both changes |
||
| self.streaming_config = streaming_config | ||
|
|
||
| return self | ||
|
|
||
| def digest_args(self, args: argparse.Namespace) -> "ManimConfig": | ||
|
|
@@ -679,7 +698,7 @@ def digest_args(self, args: argparse.Namespace) -> "ManimConfig": | |
| "partial_movie_dir", | ||
| ]: | ||
| self[opt] = self._parser["custom_folders"].get(opt, raw=True) | ||
| # --media_dir overrides the deaful.cfg file | ||
| # --media_dir overrides the default.cfg file | ||
| if hasattr(args, "media_dir") and args.media_dir: | ||
| self.media_dir = args.media_dir | ||
|
|
||
|
|
@@ -1161,6 +1180,7 @@ def get_dir(self, key: str, **kwargs: str) -> Path: | |
| "input_file", | ||
| "output_file", | ||
| "partial_movie_dir", | ||
| "streaming_dir", | ||
| ] | ||
| if key not in dirs: | ||
| raise KeyError( | ||
|
|
@@ -1236,6 +1256,12 @@ def _set_dir(self, key: str, val: typing.Union[str, Path]): | |
| doc="Directory to place partial movie files (no flag). See :meth:`ManimConfig.get_dir`.", | ||
| ) | ||
|
|
||
| streaming_dir = property( | ||
| lambda self: self._d["streaming_dir"], | ||
| lambda self, val: self._set_dir("streaming_dir", val), | ||
| doc="Directory to have streamed files. See :meth:`ManimConfig.get_dir`.", | ||
| ) | ||
behackl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| custom_folders = property( | ||
| lambda self: self._d["custom_folders"], | ||
| lambda self, val: self._set_boolean("custom_folders", val), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| from .cairo_renderer import CairoRenderer | ||
| from .. import config | ||
| from ..scene.stream_file_writer import StreamFileWriter | ||
|
|
||
|
|
||
| class StreamCairoRenderer(CairoRenderer): | ||
| def init_scene(self, scene): | ||
| """For compatibility with the __init__ from scene that's not being | ||
| directly overridden | ||
| """ | ||
| self.file_writer = StreamFileWriter(self) |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,103 @@ | ||||||||||||||
| from abc import ABCMeta | ||||||||||||||
|
|
||||||||||||||
| import os | ||||||||||||||
| import subprocess | ||||||||||||||
|
|
||||||||||||||
| from .. import config, logger | ||||||||||||||
| from ..constants import FFMPEG_BIN | ||||||||||||||
| from .scene import Scene | ||||||||||||||
| from .scene_file_writer import SceneFileWriter | ||||||||||||||
| from ..utils.file_ops import guarantee_existence | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| class StreamFileWriter(SceneFileWriter): | ||||||||||||||
naveen521kk marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
| def __init__(self, renderer): | ||||||||||||||
| super().__init__(renderer, "") | ||||||||||||||
| vars(self).update(config.streaming_config) | ||||||||||||||
| path = os.path.join(config.get_dir("streaming_dir"), "clips") | ||||||||||||||
| self.FOLDER_PATH = os.path.relpath(guarantee_existence(path)) | ||||||||||||||
| # To prevent extensive overwriting | ||||||||||||||
| self.partial_movie_directory = self.FOLDER_PATH | ||||||||||||||
|
|
||||||||||||||
| def init_output_directories(self, scene_name): | ||||||||||||||
| """The original :class:`SceneFileWriter` uses this method while initializing. | ||||||||||||||
| The method is overwritten carefully so as to correspond arguments in the | ||||||||||||||
| original and still avoid the folder creation effects of this method. | ||||||||||||||
| """ | ||||||||||||||
|
||||||||||||||
| """The original :class:`SceneFileWriter` uses this method while initializing. | |
| The method is overwritten carefully so as to correspond arguments in the | |
| original and still avoid the folder creation effects of this method. | |
| """ | |
| """Overridden to avoid creation of unnecessary output directories.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should probably say more on this but sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What file path is this for? Trying to access it yields
>>> manim.renderer.file_writer.file_path
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/home/behackl/code/manim/manim/scene/stream_file_writer.py", line 31, in file_path
return self.partial_movie_files[-1]
IndexError: list index out of rangeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Either please remove this method if it is not used, or add a one-line docstring if it is used.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self for this one too
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring of the inherited method says
Internally used by Manim to stop streaming to
FFMPEG gracefully.
I don't understand how this fits together, please clarify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what you are saying here. You override the method to reduce overriding code?
| """Also to reduce overriding code.""" | |
| """Not required for live streaming: overridden.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, needs more explanation, this does
naveen521kk marked this conversation as resolved.
Show resolved
Hide resolved
naveen521kk marked this conversation as resolved.
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The amount of code duplication between here and SceneFileWriter.open_movie_pipe is unfortunate, and could be reduced substantially by refactoring a little. I won't block the PR because of it, however.
Please add a one-line docstring summary, suggestion:
| def open_movie_pipe(self): | |
| def open_movie_pipe(self): | |
| """Used internally by Manim to initialise | |
| FFMPEG and begin writing to FFMPEG's input | |
| buffer. | |
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, that bit me a little that that code looks eerily similar to the one from the inherited class. That one I noticed as I was coding.
How would I refactor the command from the base class? That would be interesting to know
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More or less the same comment as above. Also, I am a bit confused: shouldn't the partial movie files also exist for livestreaming? If so, why is overriding this necessary at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damn I see the flaw! Good sopt
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,111 @@ | ||||||||||||||||||||||||||||||
| from .. import config | ||||||||||||||||||||||||||||||
| from ..mobject.frame import FullScreenRectangle as Frame | ||||||||||||||||||||||||||||||
| from ..renderer.stream_renderer import StreamCairoRenderer | ||||||||||||||||||||||||||||||
| from ..utils.simple_functions import get_parameters | ||||||||||||||||||||||||||||||
| from .scene import Scene | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| class Stream: | ||||||||||||||||||||||||||||||
| """This class is really intended for inheritance of the style: | ||||||||||||||||||||||||||||||
| >>> class Streamer(Stream, Scene): # doctest: +SKIP | ||||||||||||||||||||||||||||||
| ... pass | ||||||||||||||||||||||||||||||
| ... | ||||||||||||||||||||||||||||||
| >>> | ||||||||||||||||||||||||||||||
NeoPlato marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||
| This order is paramount. This :class:`Stream` class does the switch to | ||||||||||||||||||||||||||||||
| the specialized renderer, which uses the :class:`StreamFileWriter` to | ||||||||||||||||||||||||||||||
| handle specialized streaming services. That explains the calls to super, | ||||||||||||||||||||||||||||||
| which dig through the MRO of a class instead of using just a single | ||||||||||||||||||||||||||||||
| implementation contained in Scene. Okay, the references in super probably | ||||||||||||||||||||||||||||||
| point to things in the :class:`Scene` class only, but it's already happened so... | ||||||||||||||||||||||||||||||
| PS: You can probably already tell using this class on its own will bring you | ||||||||||||||||||||||||||||||
| errors. | ||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||
NeoPlato marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| def __init__(self, **kwargs): | ||||||||||||||||||||||||||||||
| camera_class = self.mint_camera_class() | ||||||||||||||||||||||||||||||
| renderer = StreamCairoRenderer(camera_class=camera_class) | ||||||||||||||||||||||||||||||
| super().__init__(renderer=renderer, **kwargs) | ||||||||||||||||||||||||||||||
| # To identify the frame in a black background | ||||||||||||||||||||||||||||||
| self.add(Frame()) | ||||||||||||||||||||||||||||||
| self.setup() | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| @classmethod | ||||||||||||||||||||||||||||||
| def mint_camera_class(cls): | ||||||||||||||||||||||||||||||
| """Only __init__ arguments have the camera class now. In order for the | ||||||||||||||||||||||||||||||
| specialized renderer to be used, the scene's camera class must be found. | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
| """Only __init__ arguments have the camera class now. In order for the | |
| specialized renderer to be used, the scene's camera class must be found. | |
| """Only ``__init__`` arguments have the camera class now. In order for the | |
| specialized renderer to be used, the scene's camera class must be found. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self, update this
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Returns: | |
| A camera class from the scene's inheritance hierachy | |
| Raises: | |
| AttributeError: If this lookup fails | |
| Returns | |
| ------- | |
| :class:`~.Camera` | |
| A camera class from the scene's inheritance hierachy. | |
| Raises | |
| ------ | |
| AttributeError | |
| When the lookup fails. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this section exists after the click merge