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

kvssink .so with static linkage to cproducer #1179

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

Conversation

mateuszboryn
Copy link

Issue #, if available: #1178

Description of changes:
Adding cmake option BUILD_GSTREAMER_PLUGIN_STATIC to allow building GST plugin as .so with statically linked deps.

@@ -12,6 +12,7 @@ include(GNUInstallDirs)
option(BUILD_GSTREAMER_PLUGIN "Build kvssink GStreamer plugin" OFF)
option(BUILD_JNI "Build C++ wrapper for JNI to expose the functionality to Java/Android" OFF)
option(BUILD_STATIC "Build with static linkage" OFF)
option(BUILD_GSTREAMER_PLUGIN_STATIC "If building GStreamer plugin, build it as a static library" ${BUILD_STATIC})
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont think it is still clear to me from the description why we need a new option. We could repurpose BUILD_STATIC for this. What is the issue when using BUILD_STATIC vs BUILD_GSTREAMER_PLUGIN_STATIC? Because I do not see any other changes other that this new flag.

Copy link
Author

Choose a reason for hiding this comment

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

With existing implementation, if BUILD_STATIC is ON, then libgstkvssink is always built as a static library (which makes little sense for gstreamer usage). This new option allows to set BUILD_STATIC to ON, while building libgstkvssink as a shared library.
It's not perfect, but at least allows building libgstkvssink.so that has libcproducer built-in.

Copy link
Contributor

Choose a reason for hiding this comment

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

The naming here is confusing and in the comments it says "If building GStreamer plugin, build it as a static library"

But what you're saying is you don't want to build it as a static library, rather you want to build as shared object and the shared object should in turn have it's producer c dependency statically linked in. We should be explicit about what it does. I am on board to have a build option that statically links in the producer library, but the plugin itself is still a shared object so we need the naming of such an option should be clear.

@sirknightj sirknightj added the build Changes to CMakeLists.txt label Jun 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Changes to CMakeLists.txt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants