Skip to content

Conversation

lnotspotl
Copy link
Member

@lnotspotl lnotspotl commented Sep 30, 2025

The purpose of this PR is to introduce a new depthai-specific cmake constant called DEPTHAI_INSTALL_PREFIX to facilitate dependency installation for projects linking against depthai, especially those on Windows. This is paramount for users who build depthai with BUILD_SHARED_LIBS turned ON as the folder containing generated .dll libraries either has to be added to PATH or libraries contained within have to be copied over to the very location of a dependent executable.

The depthai-core-example makes use of this new variable, a PR for that project is opened here.

Additionally, this PR fixes ALL examples built on Windows when depthai-core is built as a shared library.

@lnotspotl lnotspotl self-assigned this Sep 30, 2025
@lnotspotl lnotspotl added the testable PR is ready to be tested label Sep 30, 2025
@lnotspotl lnotspotl requested a review from Copilot September 30, 2025 03:08
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new CMake variable DEPTHAI_INSTALL_PREFIX to help downstream projects locate depthai dependencies, particularly on Windows when building with shared libraries. It also ensures all depthai examples work correctly on Windows with shared libraries by automatically copying required DLLs.

  • Adds DEPTHAI_INSTALL_PREFIX variable to the depthai CMake config for downstream dependency management
  • Refactors the add_runtime_dependencies macro into a shared helper file for reusability
  • Installs libusb runtime artifacts and copies DLLs for all examples to fix Windows shared library builds

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
cmake/depthaiConfig.cmake.in Adds DEPTHAI_INSTALL_PREFIX variable calculation and version information
cmake/depthaiHelpers.cmake New helper file containing the refactored add_runtime_dependencies macro
examples/cpp/CMakeLists.txt Includes helper file and adds libusb DLL copying for all examples
CMakeLists.txt Refactors macro to use helper file and adds libusb runtime artifact installation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +5 to +12
message(DEBUG "Adding runtime dependencies for ${depending_target} on ${dependency}. Imported configurations: ${imported_configs}")
foreach(cfg ${imported_configs})
message(DEBUG "Adding runtime dependencies for ${depending_target} on ${dependency} (${cfg})")
get_property(dll TARGET ${dependency} PROPERTY IMPORTED_LOCATION_${cfg})
message(DEBUG "Retrieved dll for ${cfg}: '${dll}'")
list(APPEND dlls $<$<CONFIG:${cfg}>:${dll}>)
endforeach()
message(DEBUG "Required dlls for ${depending_target} on ${dependency} are: ${dlls}")
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

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

The debug messages contain inconsistent terminology - some refer to 'core' while others use the actual target name. Line 23 should use '${depending_target}' instead of hardcoded 'core' for consistency.

Copilot uses AI. Check for mistakes.

@lnotspotl lnotspotl requested a review from moratom September 30, 2025 03:12
CMakeLists.txt Outdated

# Install vcpkg libraries that we link against dynamically
if(DEPTHAI_ENABLE_LIBUSB)
install(IMPORTED_RUNTIME_ARTIFACTS usb-1.0)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we also need to do that for dynamic calibration library? I also think this makes add_runtime_dependencies obsolete (at least that was the case when building deb packages for ROS)

Copy link
Member Author

Choose a reason for hiding this comment

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

You're making a really good point here. The issue was that when I tried installing the project, libusb-1.0.so would not be present as a runtime dependency but the dynamic_calibration library would. Turns out there was a bug in the add_runtime_dependencies macro that would override the required_dll_files variable every time it was invoked, thereby causing that only the last added runtime dependency would be added.

I guess this macro is still needed for libraries that we link against with PRIVATE visibility.

@lnotspotl lnotspotl requested a review from Serafadam September 30, 2025 18:23
Copy link
Collaborator

@moratom moratom left a comment

Choose a reason for hiding this comment

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

Test if the OpenCV is installed by choco and DepthAI core as a shared library, will it work? Can we make it work?

@lnotspotl
Copy link
Member Author

Choco installs OpenCV without modifying the PATH env variable, so we do need to copy its dlls over for all our examples. With our current cmake setup, this happens automatically and examples work even when depthai-core is compiled as a shared library.

image

@lnotspotl lnotspotl requested a review from moratom October 5, 2025 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testable PR is ready to be tested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants