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

Enable protobuf support #81

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Alextopher
Copy link

Submitting on behalf of the photonvision project.

In order to seamlessly support object detection through OpenCV DNN module we ask that wpilib enable protobuf support. Protobuf support is required to load DNN models in the .onnx file format.

While waiting for 2025 versions we ask that a 2024 v4.8.0-5 could be released with this change in the meantime.

CC @mcm001

@PeterJohnson
Copy link
Member

This is not a trivial change. We can't just enable it, because it will result in symbol conflicts between the system protobuf picked up by the cmake build and the protobuf library integrated into wpilib. We'd either be introducing a circular build dependency on allwpilib when upgrading opencv, or require us to move the protobuf build to thirdparty-protobuf and have this and the allwpilib build pull down those libraries.

@mcm001
Copy link

mcm001 commented Oct 9, 2024

It looks like Opencv has a BUILD_PROTOBUF flag, which will make opencv build a libprotobuf static library, yeah.

@PeterJohnson
Copy link
Member

That doesn't help, as in static libraries the symbols are still exposed to people linking against opencv.

@mcm001
Copy link

mcm001 commented Oct 9, 2024

Yeah sorry poorly phrased, I was trying to say that. If we were able to make sure opencv shared libraries did not import/export protobuf symbols would we be OK?

@Alextopher
Copy link
Author

Alextopher commented Oct 10, 2024

What about

WITH_PROTOBUF=ON
BUILD_PROTOBUF=OFF

image

Edit: This doesn't solve any problems as it would require pulling a thirdparty-protobuf dependency in.

@PeterJohnson
Copy link
Member

We build and use both shared and static libraries, so we need both to work, and it’s impossible to hide/embed the protobuf static library symbols.

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

Successfully merging this pull request may close these issues.

3 participants