Skip to content

Camera feeds for Android, Windows and iOS and OGV support for MovieWriter #98416

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

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

penninghlhd
Copy link
Contributor

@penninghlhd penninghlhd commented Oct 22, 2024

Camera feeds

  • I have added camera feed support for Android and Windows.
  • Also improved camera support for MacOS, which should work on iOS as well.
  • Instead of one camera feed per device, CameraServer now creates a feed per device/format/size. This way you can easily query CameraServer for the most appropriate feed based on its orientation, data format and size.
  • Added example project using shaders to render camera feed.

MovieWriter

  • Added support for OGV audio/video recording to MovieWriter.
  • Tested on MacBook M3 with good results (30 fps with 720p).

@penninghlhd penninghlhd requested review from a team as code owners October 22, 2024 12:48
@AThousandShips
Copy link
Member

AThousandShips commented Oct 22, 2024

Your branch contains unrelated commits from the 4.3 branch, please remove them

Also these changes should be separated into two parts, one for the camera feature and one for the movie writer part

You've also included unrelated files

@penninghlhd
Copy link
Contributor Author

I do not know how to remove commits from a branch or two split commits into two separate PRs. I understand that this is better for the review/merge process, but I do not really have much time to get into this.

I think the changes are useful, at least for the further development of the CameraServer/Feed functionality. The same applies to the OGV support for MovieWriter. So if anyone else could pick this up, it would be great.

@fire
Copy link
Member

fire commented Nov 7, 2024

To split commits into two separate PRs I would make two duplicate pull requests and then delete the parts that don't fit.

@keendra
Copy link

keendra commented Nov 16, 2024

To split commits into two separate PRs I would make two duplicate pull requests and then delete the parts that don't fit.

Has this been done, can we follow the progress somewhere? Need the camera support for Android :)

@penninghlhd
Copy link
Contributor Author

As I said before, I do not have the time to fix the conflicts myself at the moment. Also the conflicts arise largely from the recently added support for Linux which also implied some interface changes.

I propose someone in charge of arcitecture for the Camera feature should redefine the interface in order to resolve the conflicts properly.

In my opinion the main issues to be addressed should be:

  1. Do we define a feed for each combination of supported dimension and data format and let the user choose to most appropriate feed (as I implemented in this PR), or do we define a feed per camera and make the preferred dimension and data format configurable via CameraTexture?
  2. Do we use (predefined/custom) shaders to render the different image formats (YUV, N21, etc) by the GPU via CanvasTexture channels (as I implmented in this PR), or do we use CPU based conversion to RGB format supported by the default renderer.

@BastiaanOlij
Copy link
Contributor

Was only just been made aware of this PR, shame to see it lying dormant because at first glance this implements a lot of missing logic.

Agree with some of the remarks up above that this PR should be split into multiple PRs, camera and movie logic should be separated, and it would be good if the camera logic can be split into a PR for the core changes, and then PRs for each added platform.

Also lots of nitpicking issues, formatting, documentation changes seem incomplete, there are unnecesairy breaking changes in reordering constants (enums) which do look better code wise but will be a blocker for merging.

@keendra
Copy link

keendra commented Mar 15, 2025

Was only just been made aware of this PR, shame to see it lying dormant because at first glance this implements a lot of missing logic.

Agree with some of the remarks up above that this PR should be split into multiple PRs, camera and movie logic should be separated, and it would be good if the camera logic can be split into a PR for the core changes, and then PRs for each added platform.

Also lots of nitpicking issues, formatting, documentation changes seem incomplete, there are unnecesairy breaking changes in reordering constants (enums) which do look better code wise but will be a blocker for merging.

Exactly, it would be great to see this picked up, fixed and pushed over the finish line. I'm worried it's going to be left to die like the pr to add support for android camera in 2021.

@vonweller

This comment was marked as off-topic.

@RubenGarcia
Copy link

I would also like to see this move forward, e.g. this is probably also needed for Vuforia integration to get access to the camera feeds.

shiena pushed a commit to shiena/godot that referenced this pull request Apr 29, 2025
@RubenGarcia
Copy link

Branches
https://github.com/shiena/godot/tree/feature/support-android-camera
https://github.com/shiena/godot/tree/feature/support-windows-camera
seem to have the latest code.
Let´s wait for @shiena to create PRs for these.

@shiena
Copy link
Contributor

shiena commented Apr 30, 2025

@RubenGarcia
There's a pull request (#105476 ) out to support Windows cameras.
Following that, I'm currently working on adding support for Android cameras.

shiena pushed a commit to shiena/godot that referenced this pull request Apr 30, 2025
@RubenGarcia
Copy link

@shiena For completion, I also found
https://github.com/Lamelynx/GodotGetImagePlugin-Android
for android cameras, but it would be great to have a unified architecture, so I look forward to your PRs. I´m particularly interested in android and iOS support.

iOS support is added by a plugin
https://github.com/godot-sdk-integrations/godot-ios-plugins
but the maintainer is not available for the last 4 months, so work has moved to
https://github.com/zt-pawer/SwiftGodotIosPlugins
but camera support is only being considered as a possible future addition.

@shiena
Copy link
Contributor

shiena commented Apr 30, 2025

@RubenGarcia
This extension works perfectly, but I only got 1-2 fps on Android.
https://github.com/j20001970/godot-cameraserver-extension

@RubenGarcia
Copy link

@shiena, for reference, how many fps do you get in your android branch?

@akien-mga
Copy link
Member

akien-mga commented May 2, 2025

Hello! Thanks for contributing.

This PR seems to be interesting, but has multiple issues that would need to be resolved so it can be properly reviewed and possibly merged.

  • It seems to be based off on old version of the 4.3 branch (see the 4.3.1.rc1 commit). This should be redone on top of the upstream master branch.
  • This PR does too much at once. Adding support for camera feeds for Android, Windows and iOS is orthogonal to adding support for the OGV format in MovieWriter. Those should be split two separate PRs (at least one for camera feeds and one for OGV).
  • There are various other form issues here like adding your own VS Code config to Git, which shouldn't be done (and the .gitignore you modified was meant specifically to prevent doing that). I won't review in-depth yet to point out such issues because the first 2 points need to be addressed first.

Edit: I see some of this was already pointed out earlier, and @penninghlhd mentioned they wouldn't have time to work on it further.

So someone should volunteer and pick up that work in new PRs. I'll mark this one as draft and salvageable.

@akien-mga akien-mga marked this pull request as draft May 2, 2025 09:45
shiena pushed a commit to shiena/godot that referenced this pull request May 3, 2025
shiena pushed a commit to shiena/godot that referenced this pull request May 5, 2025
shiena pushed a commit to shiena/godot that referenced this pull request May 8, 2025
shiena pushed a commit to shiena/godot that referenced this pull request May 9, 2025
shiena pushed a commit to shiena/godot that referenced this pull request May 12, 2025
shiena pushed a commit to shiena/godot that referenced this pull request May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants