Use fluidlite as fluidsynth replacement, add SF3 support

[korli]

Fluidlite is a "very light version of FluidSynth designed to be hardware, platform and external dependency independant".

I uploaded packages for it and am proposing a patch to switch to it:

• add build features for libvorbis and fluidlite
• use these libs and headers for libmidi.
• include <fluidlite.h> instead of <fluidsynth.h>

The functionality should be the same as now, but I could only basically test. The bootstrap build (which doesn't include libmidi.so) isn't affected.

• libvorbis is required to support SF3 (SF2 files compressed with ogg vorbis).
• To be noticed, the fluidlite packages include this commit from Pete added in hrev45742.
• I didn't manage to have a single build feature with the 2 libs, in fact because fluidlite is only a static library, linking against libvorbisfile.so had to be added.
• SF3 support could help to use better quality soundfounds for a same disksize (MuseScore provides FluidR3Mono_GM.sf3 for instance).
• Fluidsynth seems to be stagnant (4 years without a release, well that's more than Haiku :) ).

Comments welcome! I plan to push the change for real world test next week.

diff --git a/build/jam/BuildFeatures b/build/jam/BuildFeatures
index 0b75ec3..660ee57 100644
--- a/build/jam/BuildFeatures
+++ b/build/jam/BuildFeatures
@@ -256,6 +256,37 @@ if [ IsPackageAvailable ffmpeg_devel ] {
 }
 
 
+# Fluidlite
+if [ IsPackageAvailable fluidlite_devel ] && [ IsPackageAvailable libvorbis_devel ] {
+	ExtractBuildFeatureArchives fluidlite :
+		file: devel fluidlite_devel
+			library: $(developLibDir)/libfluidlite.a
+			headers: $(developHeadersDir)
+		;
+
+	EnableBuildFeatures fluidlite ;
+} else {
+	Echo "Fluidlite support not available on $(TARGET_PACKAGING_ARCH)" ;
+}
+
+
+# Libvorbis
+if [ IsPackageAvailable libvorbis_devel ] {
+	ExtractBuildFeatureArchives libvorbis :
+		file: base libvorbis
+			runtime: lib
+		file: devel libvorbis_devel
+			depends: base
+			library: $(developLibDir)/libvorbisfile.so.3
+			headers: $(developHeadersDir)
+		;
+
+	EnableBuildFeatures libvorbis ;
+} else {
+	Echo "Libvorbis support not available on $(TARGET_PACKAGING_ARCH)" ;
+}
+
+
 # Freetype
 if [ IsPackageAvailable freetype_devel ] {
 	ExtractBuildFeatureArchives freetype :
diff --git a/src/kits/midi/Jamfile b/src/kits/midi/Jamfile
index d6bb5c2..d91a03a 100644
--- a/src/kits/midi/Jamfile
+++ b/src/kits/midi/Jamfile
@@ -9,11 +9,20 @@ if $(TARGET_PLATFORM) != haiku {
 }
 
 UsePrivateHeaders midi ;
-UseLibraryHeaders fluidsynth ;
+
 
 local architectureObject ;
 for architectureObject in [ MultiArchSubDirSetup ] {
 	on $(architectureObject) {
+		if ! [ FIsBuildFeatureEnabled fluidlite ] {
+			continue ;
+		}
+
+		UseBuildFeatureHeaders fluidlite ;
+		Includes [ FGristFiles MidiSynth.cpp MidiSynthFile.cpp
+			SoftSynth.cpp Synth.cpp ]
+			: [ BuildFeatureAttribute fluidlite : headers ] ;
+
 		SharedLibrary [ MultiArchDefaultGristFiles libmidi.so ] :
 			Midi.cpp
 			MidiGlue.cpp
@@ -30,7 +39,8 @@ for architectureObject in [ MultiArchSubDirSetup ] {
 			be
 			midi2
 			media
-			[ MultiArchDefaultGristFiles libfluidsynth.so ]
+			[ BuildFeatureAttribute libvorbis : library ]
+			[ BuildFeatureAttribute fluidlite : library ]
 			[ TargetLibsupc++ ]
 			;
 	}
diff --git a/src/kits/midi/SoftSynth.h b/src/kits/midi/SoftSynth.h
index ccd7cb4..da9c7a8 100644
--- a/src/kits/midi/SoftSynth.h
+++ b/src/kits/midi/SoftSynth.h
@@ -19,7 +19,7 @@
 	This version of SoftSynth is a wrapper libfluidsynth.so.
  */
 
-#include <fluidsynth.h>
+#include <fluidlite.h>
 #include <Midi.h>
 #include <SoundPlayer.h>
 #include <Synth.h>

[Pete]

Not too impressed by the fact I haven't been able to compile it... Downloaded the zip from GitHub (dated today, apparently), unpacked it, ran "cmake ." and then "make". Got a parse error in fluid_synth.c -- seems that a 'C' file has used "for(int i;..." which gcc doesn't like! Maybe I did something wrong -- not that familiar with cmake --but it looked OK to start.

Are we sure it's a true drop-in replacement? Reverb etc. work as before?

[korli]

Replying to Pete:

Not too impressed by the fact I haven't been able to compile it... Downloaded the zip from GitHub (dated today, apparently), unpacked it, ran "cmake ." and then "make". Got a parse error in fluid_synth.c -- seems that a 'C' file has used "for(int i;..." which gcc doesn't like! Maybe I did something wrong -- not that familiar with cmake --but it looked OK to start.

The recipe at Haikuports has a small patch for gcc2, will be upstreamed.

Are we sure it's a true drop-in replacement? Reverb etc. work as before?

The sources are based on fluidsynth, just truncated to the part doing the basic functionality. I don't know how reverb works.

I just got the hpkg, and I see that (as you said) it's a static library. How is that to be accessed by apps that want libfluidsynth.so?

Apps that want libfluidsynth.so would have to depend on lib:libfluidsynth, which they already should do. Haiku won't embed and support libfluidsynth.so in the base system in the future. Fluidlite is meant as a *Midikit* replacement for fluidsynth, nothing more.

[Pete]

Replying to korli:

Replying to Pete:

Not too impressed by the fact I haven't been able to compile it...

The recipe at Haikuports has a small patch for gcc2, will be upstreamed.

Thanks -- found it. Now compiles OK.

Are we sure it's a true drop-in replacement? Reverb etc. work as before?

The sources are based on fluidsynth, just truncated to the part doing the basic functionality. I don't know how reverb works.

Checked it out. Reverb and Chorus seem to work fine.

I just got the hpkg, and I see that (as you said) it's a static library. How is that to be accessed by apps that want libfluidsynth.so?

Apps that want libfluidsynth.so would have to depend on lib:libfluidsynth, which they already should do. Haiku won't embed and support libfluidsynth.so in the base system in the future. Fluidlite is meant as a *Midikit* replacement for fluidsynth, nothing more.

But that's rather retrograde isn't it? My stuff has used libfluidsynth.so since the beginning, and has always assumed and expected it to just "be there".

What's wrong with making it a shared library? I just tweaked CMakeLists.txt to build a shared library, installed it in non-packaged/lib and linked libfluidsynth.so to that. Ran my MusicWeaver Synth module on that and it works just fine! I suppose calling it fluidsynth is a bit wrong because it doesn't have midifile functionality and so on. But it should be a shared library anyway, so the user/developer has the choice of just using it or requiring the full fluidsynth system.

[korli]

Well I would expect you to bundle libfluidsynth.so with your apps if they actually need it :) The alternative is to have them depend on the fluidsynth hpkg from Haikuports (might be a different version) Sure fluidlite can be enhanced to provide a shared library, Haiku will probably still stick with the static lib at least until it happens upstream.

[Pete]

Replying to korli:

Well I would expect you to bundle libfluidsynth.so with your apps if they actually need it :)

Why...? Any more than I should have to bundle any other library that's always been packaged with Haiku -- libjpeg, say. The alternative is to have them depend on the fluidsynth hpkg from Haikuports (might be a different version) I doubt there are "different versions". As I said, my stuff works perfectly with fluidlite. I do my own midifile processing.

Sure fluidlite can be enhanced to provide a shared library, Haiku will probably still stick with the static lib at least until it happens upstream.

But you're already patching ther source. What's the objection to the two-line fix to CMakeLists.txt? Like this (sorry it's not canonical -- these are the two files I have):

--- CMakeLists.txt_ORIG 2016-08-04 08:09:44.000000000 -0700
+++ CMakeLists.txt      2016-08-05 14:33:08.094896128 -0700
@@ -96,12 +96,13 @@
 include_directories(${LIBOGG_INCLUDE_DIRS})
 include_directories(${LIBVORBIS_INCLUDE_DIRS})
 
-add_library(${PROJECT_NAME} STATIC ${SOURCES})
+add_library(${PROJECT_NAME} SHARED ${SOURCES})
 
 configure_file(fluidlite.pc.in fluidlite.pc @ONLY)
 
 set_property(TARGET ${PROJECT_NAME} PROPERTY C_STANDARD 99)
-install(TARGETS ${PROJECT_NAME} ARCHIVE DESTINATION lib)
+install(TARGETS ${PROJECT_NAME} ARCHIVE DESTINATION lib
+  LIBRARY DESTINATION lib)
 install(FILES ${HEADERS} DESTINATION include)
 install(FILES ${SCOPED_HEADERS} DESTINATION include/fluidsynth)
 install(FILES fluidlite.pc DESTINATION lib/pkgconfig)

I really don't understand why you want to make unneeded hassle for third-party folks... The code is going to be there; why should one have to duplicate it?

[pulkomandy]

When we provide things in the Haiku base package or its dependencies, it puts on us, Haiku developers, the burden of keeping these bundled libraries stable over the years. They become part of our ABI and we need to keep them around.

This results in us shiping 4 or 5 different versions of ICU, same for libpng, libjpeg, etc.

To avoid this, we try to remove these and let app use the official APIs instead (MIDI Kit, translators, etc). This way, we have a single entry point to the API which we can keep stable, and we can replace the backend as we see fit. This is a more future proof approach.

This of course doesn't prevent providing packages for the 3rd party stuff, but for Haiku itself, it's better to limit runtime dependencies as much as possible.

[korli]

Applied.