lmms 1.2.0-rc7 fails to build with fluidsynth 2.0.0

[dvzrv]

I'm currently rebuilding a bunch of software against fluidsynth 2.0.0 on Arch Linux.
Sadly that introduced an API change.

LMMS currently fails to build with it:
lmms-1.2.0rc7-fluidsynth2.0.txt

Any help in integrating/patching the sf2_player would be greatly appreciated!
Plus, I think this should probably be fixed before releasing 1.2.0!

[PhysSong]

Thank you for reporting! I'll look into this.

[PhysSong]

There are two types of errors:

• Missing macros
  Add reverb and chorus settings FluidSynth/fluidsynth#385 have introduced new settings for reverb and chorus. It also removed old macros. I think we can either hardcode their values or use fluid_settings_getnum_default.
• Incomplete types
  Refactor Soundfont loader API FluidSynth/fluidsynth#334 made some type definitions private. Since they provide getters/setters now, we may use that. A tricky part is iteration_next because they also changed the function prototype.

I'll work on it in a few days and provide a fix.

[dvzrv]

@PhysSong thanks for the fast reply!

Please note: During rebuilding against fluidsynth 2.0.0, I realized, that nearly none of the projects implementing it, are compatible to the new API.
Keep in mind: Ideally LMMS should be compatible to 1.x and 2.x. Linux distributions will only be able to use the new API, once all projects have switched (or are able to implement both APIs).

[PhysSong]

@dvzrv Can you test if my fluidsynth2 branch compiles with FluidSynth 2.0?

[dvzrv]

@PhysSong will try to do that later today! Thanks! :)

[trebmuh]

Please, note that a new 2.0.1 version as been released yesterday.

[schnitzeltony]

Have sent similar for carla (see falkTX/Carla#765) and looked into https://github.com/PhysSong/lmms/tree/fluidsynth2. Wouldn't it be better to ask fluidsynth on first instantiation for default values instead of pinning them?

[trebmuh]

For reference, done by @schnitzeltony as well : Calf & fluidsynth 2.0.

[PhysSong]

Wouldn't it be better to ask fluidsynth on first instantiation for default values instead of pinning them?

@schnitzeltony Is that about hard-coded macro values? I can do that, but I think it will decrease readability a lot.

[schnitzeltony]

Since I have not contributed (much) here: it is just a suggestion.

Is that about hard-coded macro values? I can do that, but I think it will decrease readability a lot.

Yes - advantage would be that if fluidsynth changes defaults, these changes are taken. Had the similar issue at carla so I decided to collect all defaults required at one place.

[PhysSong]

Okay. I'll try that and see if it looks okay.

[PhysSong]

@dvzrv Updated the fluidsynth2 branch(with a force-push). Tested with both 1.1.9 and 2.0.1, worked fine. Also addressed #4640 (comment) .

[schnitzeltony]

Thanks for addressing my comment. I think hard-coding defaults for fluidsynth 1.x is OK - future changes in 1.x are unlikely. Have not yet build tested (will do this evening), but would be happy to see this merged: lmms is the last on my list...

[PhysSong]

PR opened: #4678
Can anyone test that?

[dvzrv]

On 2018-10-24 22:02:50 (-0700), Hyunjin Song wrote: Can anyone test that?

Will try to do it this evening! Sorry this takes so long.

…

-- https://sleepmap.de

[dvzrv]

@PhysSong I can confirm that your branch builds with fluidsynth 2.0.0 and 1.1.11!
Thanks so much and sorry again for the delay!

[jorgicio]

PR opened: #4678
Can anyone test that?

Tested the patch in Gentoo, and working fine with fluidsynth 2.0.0. Thanks.