godot
godot copied to clipboard
godotengine
Updates VideoDecoder plugin API to GDExt.
Adds VideoStreamExtension and relevant resource loaders to handle external GDNative plugins.
There are known bugs in godot-cpp w.r.t. reference counting in Ref<T> (https://github.com/godotengine/godot-cpp/issues/652) However, from godot's side this should work just fine once that is fixed.
TODO: Integrate Theora to use VideoDecoderServer.
Bugsquad edit: This closes https://github.com/godotengine/godot-proposals/issues/3286.
I think it's missing:
GDREGISTER_ABSTRACT_CLASS(VideoStreamPlayback);
somewhere
We are not using the Extension suffix much except for some specific cases, I suggest checking how this is done for AudioStream, including how the class is registered as VIRTUAL. Other than that I think its fine.
Bypasses Ref<T> issue (https://github.com/godotengine/godot-cpp/issues/652) by using Object* when the ownership is completely transferred from the Extension to Godot.
Plugins make the engine crash on exit due to invalid access to _instance_bindings->free_callback for any singletons used (OS and ResourceLoader). Existing issue (https://github.com/godotengine/godot/issues/62152)
If you've got a moment, can you write a guide how to test, and how to develop for this? Let me know if it's too much work.
If you've got a moment, can you write a guide how to test, and how to develop for this? Let me know if it's too much work.
This commit registers all the virtual functions to their closest supported behaviors in GDExtension.
For the WebM plugin, clone the following: https://github.com/kidrigger/godot-video-reference/tree/webm-integ-serverless
SCons should work with similar options as Godot (target=editor p=x11 ... ) Use target as editor.
Use option test=yes to generate the Dynamic library for the test project (a godot project in the repo itself) Run godot and load the editor. You should have a scene with a VideoPlayer with a pre-loaded BigBuckBunny Webm video. If it plays with sound - it works.
You should be able to copy and paste the plugin directory to any project without issues.
Custom plugins for other codecs can register their own classes and decoders in the same way as the reference.
It's specific to webm, but someone added seeking. https://github.com/V-Sekai/godot/commit/d6930fa85286a7990676b7374a8d0416cebbf3eb
Fire is asking if you can update this to fix the conflicting files. It would be great to get this into 4.0
My understanding is there is agreement that it needs to get in place in time for 4.0. Due to the holidays and time constraints at the GDExtension meetings, it didn't get discussed until recently.
If you don't have time to update this, then I can try my hand.
Fire is asking if you can update this to fix the conflicting files. It would be great to get this into 4.0
My understanding is there is agreement that it needs to get in place in time for 4.0. Due to the holidays and time constraints at the GDExtension meetings, it didn't get discussed until recently.
If you don't have time to update this, then I can try my hand.
Updated. Hope it can be merged soon.
Edit: I see the errors. Will take a look ASAP
Hi, I appreciate that you went ahead and updated the PR... I have no idea what happened but it looks like git tried to include a merge into your rebase.
I went ahead and redid the rebase myself since I'm familiar with git and didn't want you to waste time trying to figure out how to undo that merge. It's all okay.
A quick question: is it okay if I move the video_stream.h/cpp back into resources? I see the analogy with audio, but a lot of times, the Godot approach to organization is pragmatic and it seems a bit overkill to have a folder with three files in it. If we get more video-related code, it could warrant its own directory, but right now I'd prefer to keep things simpler.
Hi, I appreciate that you went ahead and updated the PR... I have no idea what happened but it looks like git tried to include a merge into your rebase.
I went ahead and redid the rebase myself since I'm familiar with git and didn't want you to waste time trying to figure out how to undo that merge. It's all okay. Thank you!
A quick question: is it okay if I move the video_stream.h/cpp back into resources? I see the analogy with audio, but a lot of times, the Godot approach to organization is pragmatic and it seems a bit overkill to have a folder with three files in it. If we get more video-related code, it could warrant its own directory, but right now I'd prefer to keep things simpler.
Sounds good. I hope this gets merged soon!
Hey, so I noticed a bunch of new file apis being added, which aren't used by theora. For example, VideoStreamPlaybackTheora uses set_file while VideoStreamPlayback uses open_file.
I also noticed VideoStreamPlayback.open_file isn't exposed but is called automatically. additionally new file_seek, file_len, file_pos functions were added which don't seem to be called. Furthermore, they seem to just be wrapping FileAccess. I would like to remove these, and instead call _set_file on the gdext instead of _file_opened... and then if the extension need to open a file, it should it do it by itself.
Hey, so I noticed a bunch of new file apis being added, which aren't used by theora. For example, VideoStreamPlaybackTheora uses set_file while VideoStreamPlayback uses open_file.
I also noticed VideoStreamPlayback.open_file isn't exposed but is called automatically. additionally new file_seek, file_len, file_pos functions were added which don't seem to be called. Furthermore, they seem to just be wrapping FileAccess. I would like to remove these, and instead call
_set_fileon the gdext instead of_file_opened... and then if the extension need to open a file, it should it do it by itself.
Last I remember the whole FileAccess wrapper existed because FileAccess wasn't accessible from GDExtension.
Please make sure FileAccess is exposed to extension before modifying that.
Yes, FileAccess was fully exposed to GDExtension sometime around beta3. So that is no longer a concern.
In that case, I'll remove the file_ functions from this PR. Everything else is looking good. Next step is to test this with your webm implementation and make sure it all works end-to-end.
I won't be able to add the time features in the plugin until after the 8th.
On Mon, Jan 30, 2023, 23:40 lyuma @.***> wrote:
Yes, FileAccess was fully exposed to GDExtension sometime around beta3. So that is no longer a concern.
In that case, I'll remove the file_ functions from this PR. Everything else is looking good. Next step is to test this with your webm implementation and make sure it all works end-to-end.
— Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/pull/62737#issuecomment-1409471902, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEDSN5UN4O6F4LYLE5ICWQDWVA7O3ANCNFSM52WL4X7A . You are receiving this because you were mentioned.Message ID: @.***>
set_loop and get_loop was removed. @lyuma Is this intended?
I have an implementation of this in webm and I'm sure it can be done in ffmpeg
tl;dr how do you set loop in the new system?
I removed them because they were not being called by Godot, except in one place which hardcoded set_loop(false).
if we add a way to perform set_loop(true) in Godot, we can add the functionality back in. but I wasn't sure the intended usage, so I couldn't properly document it.
will there be a godot4 demo using this API?
https://github.com/kidrigger/godot-video-reference is pretty much that demo :slightly_smiling_face:
