Skip to content

Conversation

@yuhao1118
Copy link
Contributor

Fix UsbCameraImpl on macOS

Changes

  1. Removed redundant m_mode member variable from UsbCameraImpl class

    • The local m_mode was shadowing the parent class's m_mode from SourceImpl
    • This prevented proper read/write access to the parent's m_mode, causing incorrect GetVideoMode return values
  2. Improved FPS validation logic in deviceCheckModeValid

    • Previously only checked the first matching VideoMode for FPS compatibility
    • Now checks all VideoModes with matching resolution and pixel format
    • This fixes cases where multiple VideoModes with different FPS ranges exist for the same resolution/pixel format

Testing

  • Verified that GetVideoMode now returns correct values
  • Confirmed that FPS validation works correctly for all supported video modes

@yuhao1118 yuhao1118 requested a review from a team as a code owner April 14, 2025 15:44
@github-actions github-actions bot added component: cscore CameraServer library labels Apr 14, 2025
@PeterJohnson PeterJohnson changed the title fix: UsbCameraImpl on macOS [cscore] Fix USB video mode handling on macOS Apr 14, 2025
@yuhao1118 yuhao1118 force-pushed the fix/usbcamera-impl-macos branch from b820648 to 2cc1108 Compare April 15, 2025 03:09
@yuhao1118
Copy link
Contributor Author

@peterjohns Hi, can I request for a CR about this PR? Thanks!

@sciencewhiz
Copy link
Contributor

Hi, can I request for a CR about this PR? Thanks!

With the FRC Championship happening right now, the WPILib team has other priorities, so it might be a little while.

Copy link
Member

@ThadHouse ThadHouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Great.

macOS support was definitely written a bit hacky. Mainly because its basically broken if used from robot programs in sim, due to thread requirements. But for UI tools, it should work fairly well.

@ThadHouse ThadHouse merged commit 26e2991 into wpilibsuite:main Apr 21, 2025
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: cscore CameraServer library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants