-
-
Notifications
You must be signed in to change notification settings - Fork 22.5k
Add Ogg Theora support to MovieWriter #106700
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
base: master
Are you sure you want to change the base?
Conversation
586bf64
to
ab4e1ff
Compare
// Video stream keyframe frequency (one every N frames). | ||
ogg_uint32_t keyframe_frequency = 64; |
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.
This is hardcoded to 64
currently. Should we use a different value (to improve the quality/size ratio) or add a project setting to configure it?
cc @DeeJayLSP
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 think that's a good default, although a lower value might improve the quality in some cases, and the granularity on seeks. It could be a good candidate for a setting.
// Sets the encoder speed level. Higher speed levels favor quicker encoding over better quality per bit. Depending on the encoding | ||
// mode, and the internal algorithms used, quality may actually improve with higher speeds, but in this case bitrate will also | ||
// likely increase. The maximum value, and the meaning of each value, are implementation-specific and may change depending on the | ||
// current encoding mode. | ||
int speed = 4; |
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.
Same as above. This could perhaps be exposed as a project setting too (I haven't benchmarked each level yet).
ab4e1ff
to
bc9c696
Compare
I see I got pinged a lot on this, it's better to cc @berarma , who was responsible for #101958. I don't know anything about Theora other than how to use the encoder effectively. Also I don't see how having support for recording in such a legacy format is beneficial. If there is something worth to be spending time on, it would be enabling MovieWriter to pipe to FFmpeg. It would close a lot of these proposals, directly or indirectly. Just my two cents. |
A reminder that Windows 64bits has assembler optimizations disabled due to issues with register allocation, so it might run slower there.
The reason for these errors is that there's no target bitrate set (it's zero). They are only valid when there's a target bitrate. I think these parameters can be ignored, the code removed, and the problem would be solved. The keyframe errors have another cause. Please, see my code reviews, that might fix it. I don't have experimented much with encoding parameters apart from the GOP and quality settings. I would leave GOP and video/audio quality as settings, and maybe |
I've done a couple of tests with Higher values remove features. I would leave the default library setting removing the code that changes it. Unless someone gets different results, in which case I would add a setting for it. |
Movie Maker mode can now record files in `.ogv` format, which can be directly viewed in Godot's VideoStreamPlayer node along with most video players. This is a lossy format with inter-frame compression, unlike AVI + MJPEG which only performs intra-frame compression. Co-authored-by: Leo de Penning <leo.depenning@illuminoo.com>
bc9c696
to
0647374
Compare
0647374
to
a9a2854
Compare
I've tried to change |
I've done some tests using I haven't found any case where speed=1 was faster than speed=4 so maybe I did something wrong. WING IT! (cartoon with fast action and plane changes)
Cheetahs on the edge (Slow motion video)
Spring (Somewhere in between the other two)
Execution log for WING IT!
Execution log for Cheetahs on the edge
Execution log for Spring
|
Movie Maker mode can now record files in
.ogv
format, which can be directly viewed in Godot's VideoStreamPlayer node along with most video players. This is a lossy format with inter-frame compression, unlike AVI + MJPEG which only performs intra-frame compression.Thanks to @penninghlhd for the initial implementation from #98416 🙂
I've copied the code over, rebased it on top of
master
and tweaked the code style to match the code style guidelines. I've left some review comments accordingly where there are still open questions.Theora information from ffprobe with the default settings:
Testing project: https://github.com/Calinou/godot-movie-maker-demo
Regarding encoding performance, I've only tested it on a debug build so far, but OGV encoding is significantly slower than AVI + MJPEG with the default quality:
AVI
OGV
This means that for quick previewing and further video editing, AVI + MJPEG is still preferred.
TODO
[ffmpeg/demuxer] ogg: Broken file, keyframe not correctly marked.
.builtin_libtheora=no builtin_libvorbis=no builtin_libogg=no
(undefined references).modules/theora
module it depends on, like Move MovieWriterMJPEG class tojpg
module it depends on #106013.