Skip to content

[macOS] Fix missing frame_changed signal to CameraFeed #104809

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

shiena
Copy link
Contributor

@shiena shiena commented Mar 30, 2025

fixed #104808
I will add the missing emit of the frame_changed signal in the CameraFeed class on macOS.

@shiena shiena requested a review from a team as a code owner March 30, 2025 17:36
@TokageItLab TokageItLab added this to the 4.x milestone Mar 30, 2025
@shiena shiena changed the title Add missing frame_changed signal to CameraFeed on macOS Fix missing frame_changed signal to CameraFeed on macOS Mar 31, 2025
@shiena shiena changed the title Fix missing frame_changed signal to CameraFeed on macOS [macOS] Fix missing frame_changed signal to CameraFeed Apr 4, 2025
@shiena
Copy link
Contributor Author

shiena commented Apr 12, 2025

The frame_changed signal is documented but not implemented. Because this signal doesn't arrive, the datatype, width, and height remain unknown, preventing decoding and aspect ratio determination.

https://docs.godotengine.org/en/stable/classes/class_camerafeed.html#class-camerafeed-signal-frame-changed

@shiena
Copy link
Contributor Author

shiena commented Jun 8, 2025

When updating frames in camera feed, we always call one of set_rgb_image / set_ycbcr_image / set_ycbcr_images / set_external. Therefore, we're moving the signal emission from platform-specific CameraFeedXXX classes to the base CameraFeed class.

@shiena shiena force-pushed the fix/missing_frame_changed branch from 8a7a2b8 to 1493082 Compare June 8, 2025 03:59
@shiena
Copy link
Contributor Author

shiena commented Jun 14, 2025

Currently, the CameraFeed::frame_changed signal is essential due to the following non-functional scenarios:

  • CPU-based image processing libraries like OpenCV
  • Frame change detection when CameraFeed is reactivated after being deactivated (i.e., active → inactive → active cycle)

@shiena shiena force-pushed the fix/missing_frame_changed branch 4 times, most recently from 25da7db to e276b5d Compare June 21, 2025 18:19
@shiena shiena force-pushed the fix/missing_frame_changed branch 3 times, most recently from b1c7d7f to a152857 Compare July 7, 2025 03:11
@BastiaanOlij
Copy link
Contributor

I think from a high level this makes sense. I don't really have the ability to test this.

@stuartcarnie is this maybe something you can verify just to make sure?

My only coding remark is that it's better to create a constant for the StringName itself, have a look at other places in the code where we do this. It's more efficient than constructing new StringNames all over. But that really is a nitpick.

@shiena shiena force-pushed the fix/missing_frame_changed branch from a152857 to af78d59 Compare August 8, 2025 01:17
@shiena
Copy link
Contributor Author

shiena commented Aug 8, 2025

@BastiaanOlij
Changed constants to StringName type.

@shiena shiena force-pushed the fix/missing_frame_changed branch from af78d59 to 8b4babd Compare August 8, 2025 01:25
@shiena shiena force-pushed the fix/missing_frame_changed branch from 8b4babd to ca2071d Compare August 8, 2025 01:31
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.

CameraFeed.frame_changed signal is not emitted on macOS
3 participants