- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 23.5k
 
Support CameraFeed format on macOS #106777
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
ab9927a    to
    19c603f      
    Compare
  
    19c603f    to
    003c5ea      
    Compare
  
            
          
                modules/camera/camera_macos.mm
              
                Outdated
          
        
      | ERR_FAIL_INDEX_V((unsigned int)p_index, device.formats.count, false); | ||
| ERR_FAIL_COND_V_MSG(capture_session != nullptr, false, "Feed is active."); | ||
| selected_format = p_index; | ||
| emit_signal("format_changed"); | 
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.
The format_changed signal is emitted from camera_feed.cpp, so emitting it from camera_macos.mm would result in double emission.
https://github.com/godotengine/godot/blob/master/servers/camera/camera_feed.cpp#L199
https://github.com/godotengine/godot/blob/master/servers/camera/camera_feed.cpp#L222
https://github.com/godotengine/godot/blob/master/servers/camera/camera_feed.cpp#L255
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'm not clear on when format_changed should be emitted, the Linux driver emits the signal whenever a format is selected:
godot/modules/camera/camera_feed_linux.cpp
Lines 348 to 350 in a39a83a
| parameters = p_parameters.duplicate(); | |
| selected_format = p_index; | |
| emit_signal(SNAME("format_changed")); | 
while
camera_feed.cpp emit the signal when current camera frame size is different from the previous. Could someone clarify that whether format_changed should be emitted as soon as a format is selected for the camera feed?
    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.
Currently, the format_changed signal is emitted within set_ycbcr_images on the following line when the image size differs from the previous call, so combined with the call within set_format, it gets emitted twice per frame - we should choose one or the other. It's a dilemma which one should emit it, but I imagine what an app using CameraFeed would want to do when receiving the format_changed signal. Emitting from set_ycbcr_images allows receiving image data with the changed format. If emitted from set_format, there's a possibility that image data isn't available yet. Comparing these two, I think emitting from set_ycbcr_images would be better.
https://github.com/godotengine/godot/blob/master/modules/camera/camera_macos.mm#L185
94974b3    to
    1487dfa      
    Compare
  
    97939ae    to
    a153d20      
    Compare
  
    1f82a8f    to
    dd48972      
    Compare
  
    ee2e1e7    to
    310ccdc      
    Compare
  
    0551489    to
    6d9913a      
    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.
Tested locally on macOS 26 (MBP 16" 2024 with M4 Max), it works as expected.
However, some formats in the list look broken:
![]()  | 
![]()  | 
|---|
The following formats have an appearance like this, with green diagonal lines and what seems to be an incorrect stride (the image does update, but looks skewed):
- 1760x1328
 - 1328x1760
 - 1552x1552
 - 1080x1920 (1920x1080 is fine)
 
ae36848    to
    30959dc      
    Compare
  
    | 
           30959dc try to handle padded image data. Please let me know if it fixes skewed image for some formats.  | 
    
30959dc    to
    83c1d2f      
    Compare
  
    
          
 Works great now 🙂 I can get all formats to display. However, in https://github.com/shiena/godot-camerafeed-demo, switching formats occasionally leads to a black screen until I resize the window (the webcam display appears as a tiny square in the top-right corner). Sometimes, switching formats will still result in a black screen until I restart the project. No errors or warnings are printed in the output log. This is presumably a bug in the demo rather than in this PR though. cc @shiena  | 
    
| 
           I didn't take into account that camera feed format could change while activated (current implementation just remember selected format index to use before next feed activation), maybe there are still parts that can be improved.  | 
    
83c1d2f    to
    76a317e      
    Compare
  
    
          
 With 76a317e and shiena/godot-camerafeed-demo@a10d76c, camera feeds should be able to change format while still active. The problem of disappeared camera preview after stopping and starting the feed without changing format seems to be caused by rotation of camera preview in godot-camerafeed-demo. The last issue that I'm aware of is that, after changing feed format, it does not take effect immediately. On my machine with a USB webcam, camera feed still output a frame or two in previously configured format, even when camera feed is deactivated then activated again.  | 
    
6447ccc    to
    e3f15cf      
    Compare
  
    32ad606    to
    fac938b      
    Compare
  
    1a24340    to
    c1f9190      
    Compare
  
    c1f9190    to
    f926356      
    Compare
  
    

Implement
CameraFeedMacOS::set_formatandCameraFeedMacOS::get_formats.When activating the feed, if
selected_formatis specified, try locking the device to set active format for it, and unlocking the device upon deactivated.