Faulty include in ProjectM.hpp / projectM-4.pc

[JohannesKauffmann]

Describe the bug
In a consuming project, I can't include ProjectM.hpp as it includes Common.hpp like so: #include <libprojectM/Common.h>, but in the projectM install tree, Common.hpp is installed in ${includedir}/projectM-4/Common.hpp.

The CMake configuration for building ProjectM includes -DENABLE_CXX_INTERFACE=ON.

The only solution that comes to mind as of now would be to:

1. change the #include in ProjectM.hpp to #include <Common.hpp>
2. use CMAKE_INCLUDE_CURRENT_DIR in src/libprojectM/CMakeLists.txt
3. add ${includedir}/projectM-4 to INTERFACE_COMPILE_DEFINITIONS somehow, so this extra directory gets propagated to consuming targets

Any thoughts?

Desktop (please complete the following information):

• Application: [very early WIP trying to integrate projectM 4 into VLC]
• OS: [linux]
• Version [91bc04d]

[kblaschke]

If you want to integrate version 4 into VLC, I strongly recommend using the new C API. The C++ API is not meant to be used anymore and the CMake flag is mostly there for development testing purposes using projectM as an in-tree external project, hence the author warning you get in CMake when configuring the project.

Here's an issue I've created a while ago in the VLC issue tracker, with responses by the VLC project maintainer:

https://code.videolan.org/videolan/vlc/-/issues/26014

[JohannesKauffmann]

So far I hadn't seen any issues with using the C++ API, except for quering the playlist size, which seems to only be possible with the C API. However thinking about it some more, if major distro's aren't supposed to/going to package projectm with the C++ API, it might be better to use the C API instead, yes.

From the warning however it wasn't clear to me that the C++ API is only meant for in-tree projects; after all, the warning points to possible ABI problems. However, I can certainly see that following the ABI concerns, it would only be wise to only use the C++ API when bundling projectM directly with the app, thus the projectM install tree becomes irrelevant.

Would you agree that the current warning could use a clarification?

[kblaschke]

Sure, the wording hasn't changed since I introduced the option in the early phase of refactoring. The documentation is a bit clearer about the C++ headers, telling these can and will change at any time. I'd also be fine to remove this option completely and only provide a special target in the build tree that points to the proper include dirs, like "projectM::InternalAPI" which would only be available when using the lib via CMake 's "ExternalProject" module.

[kblaschke]

I'll close this issue for now, as the initially reported header is no longer present.

Plus, the documentation should be stating clear enough that the C API is the only officially supported and endorsed way of integrating projectM. We now give the guarantee to keep the C API forward compatible within a major release, but there is no such promise for the C++ code inside the library. This means that with every release, even patch releases, application developers using the C++ classes would have to change their code if they want to use a newer release, and thus could only support a single, specific version of libprojectM.

Another important reason to not use the C++ classes directly is that only the C API symbols are currently properly exported for use in Windows DLLs, while even with th C++ API enabled, not all required classes will be accessible. On top of that, there are several issues with passing STL types across library boundaries, which would for example prevent the use of a Release projectM.dll in an application built in debug mode, as the MSVC runtimes have different memory layouts due to additional debugging members in many types, e.g. std::string.

If you think that the documentation or the CMake option require additional clarification to make this even more clear, please fix the wording accordingly and create a PR. Any further requests on this matter, e.g. allowing to compile libprojectM inside another CMake project as a subdir, should go into a new feature request issue. Feel free to open one!