-
Notifications
You must be signed in to change notification settings - Fork 32
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
Improvements for StereoDepth, ColorCamera #23
Conversation
the device will check if the node's outputs are connected in order to enable
/** | ||
* Overrides the baseline used for depth calculation. In centimeters | ||
*/ | ||
float baselineOverrideCm = 0; | ||
/** | ||
* Overrides the camera horizontal field of view used for depth calculation. In degrees | ||
*/ | ||
float fovOverrideDegrees = 0; |
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.
With #19 in works, should we rather have a way of specifying these settings "globally" instead of eeprom. This would also cover the usecase of the new sourceFrame
information added to ImgFrame
by @szabi-luxonis
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 main use-case for these fields would be running Stereo on frames from host, where the source images are (optionally) already rectified, but they were taken from other boards / setups, so the baseline/FOV from the onboard EEPROM is not applicable.
There's also possible to create multiple instances of the StereoDepth.
Should we then configure this in some other way?
(There might be possible to pass this info via sourceFrame
of ImgFrame
from host, but right now the StereoDepth node on device needs this info at init.)
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.
@saching13 should we then make a common structure describing actual camera information, in same way as is saved on EEPROM. That way user can either flash eeprom with it, or just supply it as part of pipeline information.
For virtual frames, thats something to think about - maybe we treat all negative instance numbers as virtual, and users can modify that as desired. That would mean that modifying this data in runtime might not be possible though. (Depending if that would be requried)
Thougths, @saching13 @szabi-luxonis ?
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 agree @themarpe. We can use the same structure in #19 and load the calibration information there and send it to the device for that instance of the stereo node. This would be the best way I think.
If the images are already rectified, then we can just make the rectification matrixes as identity when passing it.
float baselineOverrideCm = 0
On this I think we should no longer use the only x axis. We should just make it a 3x1 vector. This is also already there in the EEPROM structure. We can reuse that.
float fovOverrideDegrees
can also be set in that structure. but since we will be using calibration information. the override can be mentioned using a bool instead.
And for baseline also we have measured translation and translation from calibration. we should have an override to say which one to use.
And as of virtual frames. if the size, position and shape keeps changing during execution then
one option I can think of is to do stereo it on the source frame and crop on the returned depth(some resources are wasted).
second option would be to send the details along with inside the frame.
The third would be to have an interrupt type that sends updated information.
Another option would be to use the source camera calibration information and update the intrinsic based on crop information. (We tested this for center crop and resize. should also test for other arbitrary crops)
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.
Ok, can remove these for now, and will reimplement later when we have #19 in.
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.
But let's have bools in stereoProperties as useFov
useMeasuredTranslation
to provide the users with both options incase their calibration was not that good.
will be refactored when the new calibration structure is integrated
remove for now noiseReductionStrength (not yet implemented)
Even though they are limited to max 63, we'll simplify the fraction on device side, to be able to call setIspScale with (720, 1080) instead of (2, 3)
No description provided.