-
-
Notifications
You must be signed in to change notification settings - Fork 22.8k
Add CameraServer
feeds_updated
signal, and document async behavior
#108165
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
eba8332
to
36446f6
Compare
a5aa0f3
to
3792213
Compare
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 we should rework the API instead of documenting workarounds. (again, not directed to you @shiena)
CameraServer.monitoring_feeds
race condition and document async behavior
3792213
to
72aee69
Compare
…behavior - Add SafeFlag to prevent concurrent camera thread initialization - Set monitoring_feeds to true only after device enumeration completes - Document that monitoring_feeds requires waiting a few frames - Add code examples for GDScript and C# showing proper usage pattern This ensures camera feeds are properly initialized before being accessed and prevents potential crashes from concurrent thread creation.
72aee69
to
f905263
Compare
f120205
to
5a6d1ea
Compare
733ff30
to
28431d0
Compare
modules/camera/camera_linux.cpp
Outdated
@@ -169,12 +171,16 @@ inline void CameraLinux::set_monitoring_feeds(bool p_monitoring_feeds) { | |||
|
|||
CameraServer::set_monitoring_feeds(p_monitoring_feeds); | |||
if (p_monitoring_feeds) { | |||
camera_thread.start(CameraLinux::camera_thread_func, this); | |||
if (!activating.is_set()) { |
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 isn't fully thread safe. But I'm noticing on reviewing SafeFlag
that it's improperly implemented, and cannot be made truly thread safe. For the moment, this is probably fine. I'll address my SafeFlag
concerns some other time.
CameraServer.monitoring_feeds
race condition and document async behaviorCameraServer
feeds_updated
signal, and document async behavior
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.
Looks good from my perspective now.
Thanks for doing all the adjustments!
This still needs a review from @godotengine/xr though.
62e4dfc
to
3a3f3c9
Compare
fixed: #108136
This ensures camera feeds are properly initialized before being accessed and prevents potential crashes from concurrent thread creation.