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

request replacement for deprecated CameraBoardSocket::RGB, LEFT, RIGHT, and CENTER. #142

Open
diablodale opened this issue Jun 12, 2023 · 7 comments

Comments

@diablodale
Copy link
Contributor

Commit eecb8de removed important functionality. I request a replacement method/process/api for an application to use so that the app can programmatically identify color cameras and depth mono pairs of cameras in retail OAK sensors.

  • My applications need to be able to open and use the color RGB camera.
  • My applications need to be able to open and use one mono camera, two mono cameras, or combined as a virtual depth.

dai::CameraBoardSocket::RGB is deprecated. it now has an uncertain future. My applications need an alterative programmatic method to definitely access and denote each camera on the sensor. This api needs to be within depthai-xxxx itself...only it knows itself, its firmware, hardware, sensors attached, etc. Customers writing solutions for OAK should not need to build their own database lookup tables or probing mechanisms.

Naturally, the enum is a great solution for retail OAK sensors since it is a compile-time constant that is unvarying, simple, and definitive. Personally...I would revert the deprecation and instead add specialized/new methods for custom OAK boards which have unknown sensors in unknown places/sockets.

Setup

  • all os, platforms
  • depthai-shared eecb8de

Repro

  1. Write any depthai-core application that uses CameraBoardSocket::RGB. Or use the many tests+examples included with depthai-core.
  2. config and build your application for Debug

Result

Many warnings (may be elevated to errors)

error C4996: 'dai::CameraBoardSocket::RGB': was declared deprecated
error C4996: 'dai::CameraBoardSocket::LEFT': was declared deprecated
error C4996: 'dai::CameraBoardSocket::RIGHT': was declared deprecated

Expected

No warnings or errors. Code should continue to build and work with no flaws

@themarpe
Copy link
Collaborator

Check updated examples:

  • My applications need to be able to open and use the color RGB camera. - use getConnectedCameraFeatures() & check for "COLOR" camera. Use any you see fit.
  • My applications need to be able to open and use one mono camera, two mono cameras, or combined as a virtual depth. - Addressed with naming. Use 'left' & 'right' to default to those two imagers.

Reason for the change is that board socket does not correlate to the "semantic" of a camera, especially in FFC scenarios or newer devices. The semantic can be inferred either from name, or from type or other features that are retrievable.

This was also a reason for the deprecation - it'll go away in the future, hopefully replaced with something better, but it is api that does not hold correct.

Better will likely be a replacement of sorts: "getDefaultCameraBoardSocket(dai::Camera::RGB)" which returns a socket (perhaps optional) or name, etc... getDefaultCamera(dai::Camera::LEFT) -> None or "left" for instance.
This can then be feed into either:

  • Constructor of Color/Mono/Camera
  • camera.setName(getDefaultCamera(dai::Camera::LEFT))
  • ...

@diablodale
Copy link
Contributor Author

diablodale commented Jun 12, 2023

I don't see any improvements or replacements in examples

main
https://github.com/luxonis/depthai-core/blob/125feb8c2e16ee4bf71b7873a7b990f1c5f17b18/examples/ColorCamera/rgb_preview.cpp#L19

develop
https://github.com/luxonis/depthai-core/blob/5f75ae74ade6ae12bd56c71c0149c0482f23c18f/examples/ColorCamera/rgb_preview.cpp#L19

The latter would require startups/teardowns of pipelines starting at A -> ? letter to find somewhere a color camera. Naturally, this is unworkable.

I encourage continued compatibility such that today's RGB, LEFT, RIGHT, CENTER continue to work. It would be the job internal code within depthai-core to map that into the given't device's color camera. for example....

if (value <= easyEnums)
    // map easy enums to technical socket
   value = getDefaultCameraFromEnum(easyEnum)
else
   // nothing, it is an abstracted technical socket letter

doThings(value)

[EDITED] Meanwhile, if your examples imply that you have no replacement or methodology currently planned, then I recommend you don't [[deprecate]]. Only deprecate when you have a transition plan and give that transition plan with the deprecation. Otherwise, deprecating before you have an alternative causes churn for coders and project managers to investigate the issue, find a workaround, find there is no workaround, then workaround the deprecation (which I've done myself).

@themarpe
Copy link
Collaborator

themarpe commented Jun 19, 2023

I encourage continued compatibility such that today's RGB, LEFT, RIGHT, CENTER continue to work. It would be the job internal code within depthai-core to map that into the given't device's color camera. for example....

if (value <= easyEnums)
    // map easy enums to technical socket
   value = getDefaultCameraFromEnum(easyEnum)
else
   // nothing, it is an abstracted technical socket letter

doThings(value)

This'd be a good approach - but might fall short in cases where there is no device info available. Likes of CalibrationHandler, etc...

That usage would still be deprecated then, but conditionally. So would have to fall under a runtime check.


Current replacement mechanism is setCamera. left, right, color would be the names corresponding on the usual devices. (Some examples do use them, some do not). Main ones being stereo ones: https://github.com/luxonis/depthai-core/blob/5f75ae74ade6ae12bd56c71c0149c0482f23c18f/examples/StereoDepth/depth_preview.cpp

We can extend the "standard naming" (left, right, color) to always apply, even for devices where that naming differs, by automatically falling down to "best candidate" (likes of FFC, etc...)

@diablodale
Copy link
Contributor Author

I saw you recently released a version with these new APIs. Your build has many warnings on the deprecation. I recommend the SDK code's be updated to not deprecate itself and to use the deprecation feature where you can put the new api usage.
https://en.cppreference.com/w/cpp/language/attributes/deprecated.

Naturally, breaking API changes are strongly discouraged as it leads to difficult by customers like me. That is unless you were jumping from depthai v2 -> depthai v3 and then there are many opportunities for breaking changes.

@diablodale
Copy link
Contributor Author

diablodale commented Jun 27, 2023

..and what is the plan for inspecting values of EepromData? version=7 of it needs CameraBoardSocket. How can a program inspect values for the RGB camera? The new setCamera() api does not address this topic.

Keep in mind that EepromData has no field to identify an rgb camera. There is only CameraBoardSocket. So when a program needs to inspect rgb (or any other camera's) properties, then the program needs that enum or some other way to discover the sensor. luxonis/depthai-core#285

For example, my app needs to know if a camera is fixed or auto focus for many reason including luxonis/depthai-core#326. I am still required to inspect rgbCamera->second.lensPosition == 0 to determine this. A subset of new sensors might have the free-form text field productName with a FF at the end, doesn't help the thousands of previous sensors in the marketplace. Naturally, string scraping a std::string field for "FF" is error-prone and a hack for both Luxonis and customers. Naturally, a bool fixedFocus; is one of many fields on a camera struct that would help fill this gap. If we could even identify which CameraBoardSocket contains an rgb camera.

Is CameraFeatures the plan?

@themarpe
Copy link
Collaborator

CameraFeatures is the plan - that will be the main "capabilities" of imaging portion of the device.

To discover rgb camera, you may use supportedTypes, where first option describes the type of imager (eg: CameraSensorType::COLOR). There may be many as FFC cases contain imagers that cannot be identified upfront (likes of OV9282 vs its color variant OV9782). This is rare & in progress of being addresses as well, but requires HW changes.

Likewise for autofocus capabilities, which is WIP.

@themarpe
Copy link
Collaborator

We'll implement replacements using setName (to be able to 'left', 'right', 'center', 'rgb', ..., across all devices) and add means to query for these sockets:

Device::getCameraSocket(string cameraName);

Modifications include:

// CameraFeatures
vector<string> additionalNames;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants