-
Notifications
You must be signed in to change notification settings - Fork 731
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
NVIDIA VisionWorks and stereo_image_proc #380
base: melodic
Are you sure you want to change the base?
NVIDIA VisionWorks and stereo_image_proc #380
Conversation
…dd NVIDIA VisionWorks support for stereo algorithm acceleration in ROS stereo_image_proc. This includes: * Adding a library for visionworks functionality, linked against NVIDIA libovx and libnvx * Adding supporting calls in processor.h/processor.cpp to switch to this functionality * Editing dynamic-reconfigure options * Adding supporting documentation This software should auto-detect the presence of installed VisionWorks code and, if detected, compile and link against said code. If the requirements are not met, then stereo_image_proc will not link in the new code and will continue to run in the traditional stereo_image_proc method.
@zlacelle hi, we're getting to all this. This is fabulous, great work! 2 asks before review:
|
@SteveMacenski OK, can do. I'll rebase and give it a try. For the rebase, I'll just re-branch off of melodic and create a new pull request. Is there another way you'd like it done? For the separate nodelet, my plan is to just copy the entire stereo_image_proc nodelet code as is--I'm not going to be able to test it in hardware with any substantial changes. In the future, with this approach, I think it would be good to remove all non-VisionWorks code and have the nodelet exclusively make use of visionworks. |
Hi, I'm not sure what you mean by not being able to test - has this not been validated? I think to give you some background on my motivation: stereo image proc is used by a bunch of people and works on robust APIs that themselves haven't changed in a long time. I wouldn't want to break anything for anyone, or have it magically start using GPU resources without asking, I'd like it to be an intentional and deliberate action for someone to say "use GPU", which could be done via a parameter, however that would involve changing the core stereo image proc with a pretty new GPU-vendor specific implementation. I'd much rather see a separate nodelet with just the GPU capabilities that has the same API so someone just changes a name and poof it works. Then once we're all comfortable with it working as expected, we might reintegrate it into the main stereo image proc as a parameter, but likely it'll live as a separate I'm also not certain how this CMakeLists work will scale with deploying debians from the build farm, I think it tries to link at compile time to vision works rather than at run time, but you'd have to inform me your expected behavior. |
Hi, Yes, everything has been validated on a TX2 with a Bumblebee. What I mean to say is that the hardware I was using to test is no longer set up and readily available, so any major code changes (like fully removing non-GPU code from the GPU nodelet) would make me uncomfortable. But the existing code is tested. I understand completely not wanting to modify the existing nodelet and APIs. I will make the separate nodelet you're mentioning, which should be just build system changes. |
@SteveMacenski Rebased, will work on separate module for stereo_image_proc and associated libraries. |
@zlacelle This PR was made prior to me taking over maintainership of this repo. Is this still something you need? If so, could you please rebase to the latest |
Changes and additions from Zach LaCelle at The MITRE Corporation to add NVIDIA VisionWorks support for stereo algorithm acceleration in ROS stereo_image_proc.
This includes:
This software should auto-detect the presence of installed VisionWorks code and, if detected, compile and link against said code. If the requirements are not met, then stereo_image_proc will not link in the new code and will continue to run in the traditional stereo_image_proc method.