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

Camera: Skip non platform-specifc CameraFeed types in Linux/macOS driver #100540

Merged

Conversation

j20001970
Copy link
Contributor

Closes #100522

Platform implementations of CameraServer was expecting added camera feeds always being corresponding CameraFeed subclass, this breaks the possibility of implementing custom camera feeds with either GDScript or GDExtension.

This PR add type check, ignoring feeds that are not added by the server itself.

@j20001970 j20001970 requested a review from a team as a code owner December 18, 2024 02:50
@akien-mga
Copy link
Member

akien-mga commented Dec 18, 2024

The changes seem fine, but what's the use case for having feeds registered in CameraServer that will only be skipped?

In other words, should these be silent skips like this, or ERR_FAIL_NULL checks?

@j20001970
Copy link
Contributor Author

Considering a custom feed written in GDScript, it has to be registered in CameraServer, so that it can be referenced with feed id by its consumer (i.e. CameraTexture). When the server iterating over registered feeds, downcasting to platform-specific subclass without check leads to null reference if such custom feed is registered.

As for whether skipped feeds should be silent or error, since the intention of this PR is to enable custom feed in GDScript/GDExtension, I think they should stay silent.

@akien-mga akien-mga changed the title Check CameraFeed type Camera: Skip non platform-specifc CameraFeed types in Linux/macOS driver Dec 18, 2024
@akien-mga akien-mga merged commit 48167ff into godotengine:master Dec 18, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@j20001970 j20001970 deleted the cameraserver-check-feed-type branch December 19, 2024 01:33
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.

Adding custom camera feeds to CameraServer leads to crash on Linux
3 participants