MuseScore icon indicating copy to clipboard operation
MuseScore copied to clipboard

Feature request: thirdparty libs should be linkable from system installed

Open Lucretia opened this issue 3 years ago • 16 comments
trafficstars

Add cmake options to use thirdparty libs

A lot of hte libs in thirdparty/* should have flags in cmake to enable the use of system packages, i.e.:

  • flac
  • fluidsynth
  • freetype
  • lame
  • opus
  • singleapp (on Gentoo it's dev-qt/qtsingleapplication)
  • stb

It tries to install headers from opus currently:

 * Detected file collision(s):
 * 
 *      /usr/include/opus/opus_types.h
 *      /usr/include/opus/opus_defines.h
 *      /usr/include/opus/opus_multistream.h
 *      /usr/include/opus/opus_custom.h
 *      /usr/include/opus/opus_projection.h
 *      /usr/include/opus/opus.h
 * 
 * Searching all installed packages for file collisions...
 * Detected file collision(s):
 * 
 *      /usr/include/opus/opus_types.h
 *      /usr/include/opus/opus_defines.h
 *      /usr/include/opus/opus_multistream.h
 *      /usr/include/opus/opus_custom.h
 *      /usr/include/opus/opus_projection.h
 *      /usr/include/opus/opus.h
 * 
 * Searching all installed packages for file collisions...

Lucretia avatar May 08 '22 21:05 Lucretia

I'm assigning this a priority and Beta 1 release. Will need conversations with the devs here about when to do it.... however, any community members who would like to take it on would be appreciated!

Tantacrul avatar May 15 '22 14:05 Tantacrul

We have decided to move this to 4.x.

We'll be reopening it when 4.0 is released.

Tantacrul avatar Sep 01 '22 13:09 Tantacrul

I'm interested in building 4.x, but would prefer to link to system libs (esp. freetype and opus). My distribution (linux) has a build template for 3.6 which seems to use USE_SYSTEM_FREETYPE=ON however I can't see that option anywhere in the current cmake configs. Could someone clarify the status of support for building with system libs? Also, do I understand correctly that webengine is no longer needed to build 4.x?

adigitoleo avatar Jul 15 '23 05:07 adigitoleo

Webengine is no longer needed indeed. (Instead, you do need QtNetworkAuthorization.) The status about using system libs has not changed since this issue was created; the USE_SYSTEM_FREETYPE option is currently not functional.

cbjeukendrup avatar Jul 15 '23 07:07 cbjeukendrup

See also #16948

cbjeukendrup avatar Jul 15 '23 07:07 cbjeukendrup

I tried to work on making musescore link to system fluidsynth, and I noticed that musescore uses a lot of private functions from the fluidsynth's src... When one installs FluidSynth, by default it includes only:

include
├── fluidsynth
│   ├── audio.h
│   ├── event.h
│   ├── gen.h
│   ├── ladspa.h
│   ├── log.h
│   ├── midi.h
│   ├── misc.h
│   ├── mod.h
│   ├── seqbind.h
│   ├── seq.h
│   ├── settings.h
│   ├── sfont.h
│   ├── shell.h
│   ├── synth.h
│   ├── types.h
│   ├── version.h
│   └── voice.h
└── fluidsynth.h

Here's an example of an error I got:

/build/source/src/framework/audio/internal/synthesizers/fluidsynth/sfcachedloader.h:35:10: fat
floader/fluid_sfont.h: No such file or directory
   35 | #include <sfloader/fluid_sfont.h>
After applying this rather simple patch:
diff --git i/CMakeLists.txt w/CMakeLists.txt
index 621c62126f..e984112959 100644
--- i/CMakeLists.txt
+++ w/CMakeLists.txt
@@ -123,6 +123,7 @@ option(MUE_COMPILE_USE_UNITY "Use unity build." ON)
 option(MUE_COMPILE_USE_CCACHE "Try use ccache" ON)
 option(MUE_COMPILE_USE_SHARED_LIBS_IN_DEBUG "Build shared libs if possible in debug" OFF)
 option(MUE_COMPILE_USE_SYSTEM_FREETYPE "Try use system freetype" OFF) # Important for the maintainer of Linux distributions
+option(MUE_COMPILE_USE_SYSTEM_FLUIDSYNTH "Try use system fluidsynth library" OFF) # Important for the maintainer of Linux distributions
 
 # === Debug ===
 option(MUE_ENABLE_LOGGER_DEBUGLEVEL "Enable logging debug level" ON)
diff --git i/src/framework/audio/CMakeLists.txt w/src/framework/audio/CMakeLists.txt
index 6293730784..20ca5e6625 100644
--- i/src/framework/audio/CMakeLists.txt
+++ w/src/framework/audio/CMakeLists.txt
@@ -69,7 +69,6 @@ elseif(OS_IS_WASM)
     )
 endif()
 
-add_subdirectory(${PROJECT_SOURCE_DIR}/thirdparty/fluidsynth fluidsynth)
 
 set(MODULE_SRC
 
@@ -224,22 +223,35 @@ set(MODULE_SRC
     ${CMAKE_CURRENT_LIST_DIR}/internal/synthesizers/synthresolver.h
     )
 
-set(FLUIDSYNTH_DIR ${PROJECT_SOURCE_DIR}/thirdparty/fluidsynth/fluidsynth-2.1.4)
-set (FLUIDSYNTH_INC
-    ${FLUIDSYNTH_DIR}/include
-    ${FLUIDSYNTH_DIR}/src
-    ${FLUIDSYNTH_DIR}/src/external
-    ${FLUIDSYNTH_DIR}/src/utils
-    ${FLUIDSYNTH_DIR}/src/midi
-    ${FLUIDSYNTH_DIR}/src/rvoice
-    ${FLUIDSYNTH_DIR}/src/sfloader
-    ${FLUIDSYNTH_DIR}/src/bindings
-    ${FLUIDSYNTH_DIR}/src/synth
-    ${FLUIDSYNTH_DIR}/src/drivers
-    )
+if (MUE_COMPILE_USE_SYSTEM_FLUIDSYNTH)
+    find_package(PkgConfig)
+    pkg_check_modules(FLUIDSYNTH REQUIRED fluidsynth)
+    if (FLUIDSYNTH_FOUND)
+        message(STATUS "Found fluidsynth: ${FLUIDSYNTH_VERSION}")
+        message(STATUS "Found fluidsynth include dirs: ${FLUIDSYNTH_VERSION}")
+    else()
+        message(FATAL_ERROR "Set MUE_COMPILE_USE_SYSTEM_FLUIDSYNTH=ON, but system fluidsynth not found")
+    endif()
+endif()
 
+if (NOT FLUIDSYNTH_FOUND)
+    add_subdirectory(${PROJECT_SOURCE_DIR}/thirdparty/fluidsynth fluidsynth)
+    set(FLUIDSYNTH_DIR ${PROJECT_SOURCE_DIR}/thirdparty/fluidsynth/fluidsynth-2.1.4)
+    set (FLUIDSYNTH_INCLUDE_DIRS
+        ${FLUIDSYNTH_DIR}/include
+        ${FLUIDSYNTH_DIR}/src
+        ${FLUIDSYNTH_DIR}/src/external
+        ${FLUIDSYNTH_DIR}/src/utils
+        ${FLUIDSYNTH_DIR}/src/midi
+        ${FLUIDSYNTH_DIR}/src/rvoice
+        ${FLUIDSYNTH_DIR}/src/sfloader
+        ${FLUIDSYNTH_DIR}/src/bindings
+        ${FLUIDSYNTH_DIR}/src/synth
+        ${FLUIDSYNTH_DIR}/src/drivers
+        )
+endif()
 set (MODULE_INCLUDE
-    ${FLUIDSYNTH_INC}
+    ${FLUIDSYNTH_INCLUDE_DIRS}
     )
 
 set(MODULE_LINK

@cbjeukendrup Are you aware of this private functions usage? I was wondering whether you'd interesting of avoiding it.

doronbehar avatar Aug 20 '23 11:08 doronbehar

Yes, I'm aware of it (part of it was added by me recently), and if we could avoid it in a relatively painless way that would be great, but I'm not sure if that's doable. Using those defsfont functions seems the only way to simply load a soundfont without instantiating a synth.

cbjeukendrup avatar Aug 20 '23 11:08 cbjeukendrup

Using those defsfont functions seems the only way to simply load a soundfont without instantiating a synth.

Maybe upstream would agree to expose these functions?

doronbehar avatar Aug 20 '23 15:08 doronbehar

That's certainly worth asking. Especially since FluidSynth still seems to be maintained / developed actively. I'll create an issue (or even a PR, why not) in their repo.

cbjeukendrup avatar Aug 20 '23 19:08 cbjeukendrup

Another attempt report, for flac this time: I tried to make musescore use a system's flac library, using this patch:

Patch
diff --git c/CMakeLists.txt w/CMakeLists.txt
index 621c62126f..96ca7b88b9 100644
--- c/CMakeLists.txt
+++ w/CMakeLists.txt
@@ -123,6 +123,7 @@ option(MUE_COMPILE_USE_UNITY "Use unity build." ON)
 option(MUE_COMPILE_USE_CCACHE "Try use ccache" ON)
 option(MUE_COMPILE_USE_SHARED_LIBS_IN_DEBUG "Build shared libs if possible in debug" OFF)
 option(MUE_COMPILE_USE_SYSTEM_FREETYPE "Try use system freetype" OFF) # Important for the maintainer of Linux distributions
+option(MUE_COMPILE_USE_SYSTEM_FLAC "Try use system flac" OFF) # Important for the maintainer of Linux distributions
 
 # === Debug ===
 option(MUE_ENABLE_LOGGER_DEBUGLEVEL "Enable logging debug level" ON)
diff --git c/build/module.cmake w/build/module.cmake
index 32ad9033e3..12229a7e7e 100644
--- c/build/module.cmake
+++ w/build/module.cmake
@@ -143,3 +143,5 @@ set(MODULE_LINK ${QT_LIBRARIES} ${MODULE_LINK})
 set(MODULE_LINK ${CMAKE_DL_LIBS} ${MODULE_LINK})
 
 target_link_libraries(${MODULE} PRIVATE ${MODULE_LINK} )
+
+set_target_properties(${MODULE} PROPERTIES LINK_FLAGS "${MODULE_LDFLAGS}")
diff --git c/src/framework/audio/CMakeLists.txt w/src/framework/audio/CMakeLists.txt
index 6293730784..25b06b5e0a 100644
--- c/src/framework/audio/CMakeLists.txt
+++ w/src/framework/audio/CMakeLists.txt
@@ -268,7 +268,18 @@ if (MUE_ENABLE_AUDIO_EXPORT)
 
     add_subdirectory(${PROJECT_SOURCE_DIR}/thirdparty/lame lame)
     add_subdirectory(${PROJECT_SOURCE_DIR}/thirdparty/opusenc opusenc)
-    add_subdirectory(${PROJECT_SOURCE_DIR}/thirdparty/flac flac)
+    if (MUE_COMPILE_USE_SYSTEM_FLAC)
+        find_package(PkgConfig)
+        pkg_check_modules(FLAC REQUIRED flac)
+        if (NOT FLAC_FOUND)
+            message(FATAL_ERROR "Set MUE_COMPILE_USE_SYSTEM_FLAC=ON, but system FLAC not found, built-in will be used")
+        endif()
+    endif()
+    if (FLAC_FOUND)
+        set(MODULE_LDFLAGS "${FLAC_LDFLAGS}")
+    else()
+        add_subdirectory(${PROJECT_SOURCE_DIR}/thirdparty/flac flac)
+    endif()
 
     set(MODULE_LINK ${MODULE_LINK} lame opusenc flac)
 endif()

And the build fails with:

/nix/store/fm1xq4f3g48k1mmwl6qg9jc3ipgfsk1x-binutils-2.40/bin/ld: cannot find -lflac: No such file or directory

This one should be easier to solve, I'm just not familiar with the MODULE_LINK interface used here - I need to pass -L/nix/store/...-flac-1.4.3/lib to the LINK_FLAGS property of target audio target, but the MODULE_LDFLAGS I added doesn't work...

doronbehar avatar Aug 21 '23 07:08 doronbehar

MODULE_LINK is passed to target_link_libraries(${MODULE} PRIVATE ${MODULE_LINK} ), so its interface is the same as that of CMake's target_link_libraries.

The reason that the line set(MODULE_LINK ${MODULE_LINK} lame opusenc flac) does not work, must be that flac refers to nothing if MUE_COMPILE_USE_SYSTEM_FLAC is true. Instead, pkg_check_modules sets some variables like FLAC_FOUND, FLAC_LIBRARIES, FLAC_INCLUDE_DIRS. Those should be used instead of just flac (which stays meaningless to CMake and ld, as far as I can tell). In FindSndFile.cmake, there is an example of how we use pkg_check_modules.

cbjeukendrup avatar Aug 22 '23 18:08 cbjeukendrup

FLAC_LIBRARIES, FLAC_INCLUDE_DIRS. Those should be used instead of just flac (which stays meaningless to CMake and ld, as far as I can tell).

That makes sense. I started to use FLAC_LIBRARIES, set by pkg_check_modules, instead of flac. But it still fails, but differently:

/nix/store/fm1xq4f3g48k1mmwl6qg9jc3ipgfsk1x-binutils-2.40/bin/ld: src/framework/audio/libaudio.a(unity_3_cxx.cxx.o):(.data.rel.ro._ZTV11FlacHandler[_ZTV11FlacHandler]+0x1b8): undefined reference to `FLAC::Encoder::Stream::seek_callback(unsigned long)'
/nix/store/fm1xq4f3g48k1mmwl6qg9jc3ipgfsk1x-binutils-2.40/bin/ld: src/framework/audio/libaudio.a(unity_3_cxx.cxx.o):(.data.rel.ro._ZTV11FlacHandler[_ZTV11FlacHandler]+0x1c0): undefined reference to `FLAC::Encoder::Stream::tell_callback(unsigned long*)'
/nix/store/fm1xq4f3g48k1mmwl6qg9jc3ipgfsk1x-binutils-2.40/bin/ld: src/framework/audio/libaudio.a(unity_3_cxx.cxx.o):(.data.rel.ro._ZTV11FlacHandler[_ZTV11FlacHandler]+0x1c8): undefined reference to `FLAC::Encoder::Stream::metadata_callback(FLAC__StreamMetadata const*)'
/nix/store/fm1xq4f3g48k1mmwl6qg9jc3ipgfsk1x-binutils-2.40/bin/ld: src/framework/audio/libaudio.a(unity_3_cxx.cxx.o):(.data.rel.ro._ZTV11FlacHandler[_ZTV11FlacHandler]+0x1d0): undefined reference to `FLAC::Encoder::File::init(_IO_FILE*)'
/nix/store/fm1xq4f3g48k1mmwl6qg9jc3ipgfsk1x-binutils-2.40/bin/ld: src/framework/audio/libaudio.a(unity_3_cxx.cxx.o):(.data.rel.ro._ZTV11FlacHandler[_ZTV11FlacHandler]+0x1d8): undefined reference to `FLAC::Encoder::File::init(char const*)'
/nix/store/fm1xq4f3g48k1mmwl6qg9jc3ipgfsk1x-binutils-2.40/bin/ld: src/framework/audio/libaudio.a(unity_3_cxx.cxx.o):(.data.rel.ro._ZTV11FlacHandler[_ZTV11FlacHandler]+0x1e0): undefined reference to `FLAC::Encoder::File::init(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)'

And more... In FindSndFile.cmake, find_library is used. Do you think that is needed here as well? I wonder why using LIBSNDFILE_PKGCONF prefixed variables wasn't good enough.

doronbehar avatar Aug 23 '23 10:08 doronbehar

I'm not sure why the output from pkg_check_modules is not used directly, but it seems to be the case in many places where pkg_check_modules is used, so perhaps that's the way to do it... Maybe the reason is that pkg_check_modules only gives the lib dir and the filename of the library, and that then find_library is used to find the full path to the library. But I'm not sure if that's entirely true.

Perhaps that is the cause of the link failure, but that's difficult to say... In this case, I don't think we are using private functions or doing something unusual.

cbjeukendrup avatar Aug 23 '23 16:08 cbjeukendrup

Gentoo user working on building MuseScore 4.1.1 here. A prior ebuild had a patch which replaced a few third-party libraries with system libraries, which appears to be what you're looking for. As mentioned above, I did observe that MuseScore uses fluidsynth internals, so it's probably not possible to use system fluidsynth without changes there.

This patch is for v4.1.1, but should be very close to what you want (some cmake logic will be required, I assume). musescore-4.1.1-unbundle-deps.patch.txt

Theofghy avatar Oct 20 '23 04:10 Theofghy

Thanks for the help @Theofghy :) Your usage of PkgConfig:: seems to help a lot, and I think that I had missing the flac++ lookup in my previous attempt, which probably also helped. However, I noticed a few issues that disallow part of your patch to be ported here to upstream:

  • Your patch relies on lame library including a lame.pc file, that is available only on Gentoo.
  • To make the opusenc part of the patch work, I needed to instruct cmake to look up for opus library (not opusenc). This is working now so it seems.
  • Your googletest part of the patch is probably working, but (without going too much into details) I had trouble keeping the default behavior of using the bundled version.

All of the changes that worked for me, are available in #19795 .

doronbehar avatar Oct 21 '23 09:10 doronbehar

Just to summarize, after #19795, the following libraries will lack support for using a system version:

  • [ ] lame - requires finding a good way to find it on system - upstream lacks a .pc file.
  • [ ] fluidsynth - requires asking fluidsynth maintainers to install in their package header files that are only availble on their repo.
  • [ ] googletest - needs further debugging (won't put too much details here).
  • [ ] KDDockWidgets - latest upstream version had headers renamed - need to accommodate to that. https://github.com/musescore/MuseScore/issues/24866 lists issues with the current, outdated version bundled in the sources...
  • [ ] rtf2html - haven't tried to support system version of it yet.

doronbehar avatar Oct 21 '23 10:10 doronbehar

I just had a look at upstreaming my dusty Haiku patches, but it seems like they don't apply anymore so it's almost like having to port again… anyway, now I get "platform not supported" from some thirdparty/fluidsynth header, while we already have a working port of it.

Having to port dependencies twice is really painful.

mmuman avatar Feb 15 '25 22:02 mmuman

my dusty Haiku patches

Which patches do you mean exactly? Also, what do you mean by "porting dependencies"?

doronbehar avatar Feb 16 '25 05:02 doronbehar

Which patches do you mean exactly?

These that make it build on the Haiku Operating System. Seems CMakeList has been cut into multiple files… and probably some are not relevant anymore.

Also, what do you mean by "porting dependencies"?

Everything in thirdparty/ (the new fashion is to use git submodules but eh), which is either a plain copy or a modified fork that's not been upstreamed, for which we already have recipes and patches in HaikuPorts that often haven't been upstreamed either. I don't want to have to apply our patches over yours, we already have way too much work.

mmuman avatar Feb 16 '25 09:02 mmuman

`lame` - requires finding a good way to find it on system - upstream lacks a `.pc` file.

Just checked, svn trunk actually has one it seems, so it'd just be a matter of asking for a new release (last one is from 2017…).

`fluidsynth` - requires asking `fluidsynth` maintainers to install in their package header files that are only availble on their repo.

What do you need from those headers? Maybe you just need to ask them to expose more stuff. cf. The Platform Problem.

mmuman avatar Feb 16 '25 12:02 mmuman

These

Unfortunately, these patches seem to be from the MuseScore 3 era, and none of them are relevant anymore.

In https://github.com/musescore/MuseScore/pull/17808, someone added support for FreeBSD, so you could use that PR as inspiration for where to look.

cbjeukendrup avatar Feb 16 '25 13:02 cbjeukendrup

I already started but as I said, I don't want to have to patch fluidsynth again (I already rebased our patch over their git and made a PR). Besides, patching it there will only make it even more work when you need to upgrade the copy.

mmuman avatar Feb 16 '25 13:02 mmuman

Okay, so we just need to wait until https://github.com/FluidSynth/fluidsynth/pull/1490 is merged and FluidSynth creates a new release (they seem to do so reasonably often) and then we'll just update our copy of it (that shouldn't be a big deal).

cbjeukendrup avatar Feb 16 '25 14:02 cbjeukendrup