News:

Simutrans Chat Room
Where cool people of Simutrans can meet up.

New music backend for Simutrans: fluidsynth

Started by Roboron, August 29, 2020, 07:56:10 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

Roboron

Good news everyone! Since you may know already, I've recently discovered what was the problem with simutrans music and GNU/Linux.

However, I didn't like this solution. First, it's not enough to compile with sdl_mixer(2) but you also need to install fluidsynth (which doesn't come bundled with Simutrans). Second, setting an enviroment variable is not something you can expect from a new player, specially if they don't know that they have to do it! And third, on recent versions of sdl_mixer, loading times are BIG, and starting time of Simutrans is increased considerably.

So, I asked myself: If we have to use fluidsynth anyway, why don't we drop sdl_mixer and implement the whole thing with fluidsynth instead? And so I did.

I took some inspiration from the implementation made for OpenTTD, which also uses fluidsynth, but I've made the whole thing from scratch for Simutrans looking at the fluidsynth documentation and the other music backends.
The one thing I took from OpenTTD is the list of paths where soundfonts available via package managers are installed:

// Predefined list of paths to search for soundfonts
static const char * default_sf[] = {
/* Debian/Ubuntu/OpenSUSE preferred */
"/usr/share/sounds/sf2/FluidR3_GM.sf2",

/* RedHat/Fedora/Arch preferred */
"/usr/share/soundfonts/FluidR3_GM.sf2",

/* Debian/Ubuntu/OpenSUSE alternatives */
"/usr/share/sounds/sf2/TimGM6mb.sf2",
"/usr/share/sounds/sf2/FluidR3_GS.sf2",

nullptr
};


The implementation needs one of those soundfonts to work. So, right now, this implementation only has sense for GNU/Linux distributions (which was my goal anyway). BUT, it is using SDL2 audio driver to work, not pulseaudio/alsa, so it has the potential to work in any OS with SDL2 and fluidsynth available (which only leaves out Haiku, I guess?).

One unintended consequence of this backend is that Simutrans startup time is greatly improved (like, very greatly). Althought I have no idea why there is such a difference. I would love to see the SDL2 Linux builds using this in the future for MIDI playback (and sdl_mixer to burn in hell) :-)

Tested on Arch Linux x64. The following works:

  • Music plays at startup
  • Music can be stopped an resumed
  • Player can go to the next/previous track, random or not
  • Volumen can be changed
  • You can open two instances of Simutrans and both play music

EDIT: Current version attached. Includes fluidsynth code, Makefile changes, UI selector and a visual warning when no soundfont has been found.

TurfIt

[patch] FluidSynth
Done 9!!! years ago. wow. But no interest at all...   Also, to my ears, FluidR3 is horrid with the Simutrans MIDIs. YMMV.

Dwachs

Maybe it is time now? I wont do it, I play games only with sound effects and music off.
Parsley, sage, rosemary, and maggikraut.

prissi

Adding an uncommon (relatively) library adds another problem for me maintaining nightly builds etc. I can check it, but I would rather do after release. (I have to admit, that I usually mute the midi and sounds too.)

Yona-TYT

I've used it with debian for a long time and it certainly sounds horrible, I wonder if there is a much better alternative, I really miss midi since switching to linux. :-[

Roboron

#5
Quote from: TurfIt on August 29, 2020, 08:47:50 PMDone 9!!! years ago. wow

Oh my dog. Had this patch be accepted, my life would have been different. Such a lot of time wondering why the music wasn't playing :-(

I see some interesting ideas from your original patch which I have took (I hope you don't mind) and adapted to the current Simutrans source code, like the possibility for a user to specify a soundfont. This is what was left to complete the multi-platform support. I've made sure to prioritise this over predefined ones.

Quote from: prissi on August 30, 2020, 03:20:21 PMAdding an uncommon (relatively) library adds another problem for me maintaining nightly builds etc.

Another idea borrowed from @Turflt is to add an extra compilation flag (USE_FLUIDSYNTH_MIDI), which is also only effective when compiling with SDL2 as backend, see attached patch. You can keep producing nightly builds with USE_FLUIDSYNTH_MIDI set to 0 to avoid dealing with fluidsynth, and only care about it if you are going to release a stable version, which is what a normal user who might want to listen to the music will use anyway :-)

Moreover, after this patch you could also get rid of sdl_mixer backends, since they do not provide anything of value anymore...

Quote from: Yona-TYT on August 30, 2020, 03:21:46 PMit certainly sounds horrible

Well, it depends on the soundfont you are using. Changing the soundfont will change how the music sounds. I agree that FluidR3 sounds bad with some songs (specially the main theme, the high tones sound horrible). Other tracks which do not use those sound better...

TurfIt

Quote from: Roboron on August 30, 2020, 07:12:17 PM
Oh my dog. Had this patch be accepted, my life would have been different. Such a lot of time wondering why the music wasn't playing :-(
I didn't pursue as non existent reception to the idea back then, and the challenge of needing decent soundfonts, which remains today unfortunately...


Quote from: Roboron on August 30, 2020, 07:12:17 PM
I see some interesting ideas from your original patch which I have took (I hope you don't mind) and adapted to the current Simutrans source code, like the possibility for a user to specify a soundfont. This is what was left to complete the multi-platform support. I've made sure to prioritise this over predefined ones.
Not at all, obviously what I had needed updating. 
For soundfonts, I wonder if it would be better to have a dialog for choice like was added for truetype fonts. Better usability than yet another entry in simuconf.tab.
I also had the ability to select to driver, you've hardcoded sdl2. Not sure if this functionality is still needed. Did you test on Windows? Specifically if using the sdl2 driver for sound and fluidsynth results in volume anomalies when sounds are played?  IIRC having fluidsynth directly use the dsound driver fixed that issue on Win7 onwards...

Some other differences I note:
1) IMHO dr_load_midi() should just check the midi files (using fluid_is_midifile()) rather than load it as you have (fluid_player_add()). The later results in very slow Simutrans startup as it scans all the midi files (and fluidsynth loads the instrument patches contained in each for every file).
2) Volume setting  (0.8 * vol / 256) vs (vol / 640) hmm...
3) dr_stop_midi() really needs the all_notes_off.  You get hung notes that play forever otherwise. Can't remember what the _join is for, but also IMHO is makes sense to delete the player here too.
4) dr_midi_pos(), != PLAYING vs == DONE.   
5) you might want to check your editor.. tabs and spaces all over the place.... And tabs in the makefile where they don't belong.


Quote from: prissi on August 30, 2020, 03:20:21 PM
Adding an uncommon (relatively) library adds another problem for me maintaining nightly builds etc. I can check it, but I would rather do after release. (I have to admit, that I usually mute the midi and sounds too.)
Fluidsynth is a very old mature library. Should be easily available for all platforms.


Quote from: Roboron on August 30, 2020, 07:12:17 PM
Well, it depends on the soundfont you are using. Changing the soundfont will change how the music sounds. I agree that FluidR3 sounds bad with some songs (specially the main theme, the high tones sound horrible). Other tracks which do not use those sound better...
Try SimuSoundFont.7z

Roboron

Quote from: TurfIt on August 31, 2020, 01:29:56 AMFor soundfonts, I wonder if it would be better to have a dialog for choice like was added for truetype fonts

With TTF we have a clear idea on where to look for fonts (the fonts directory), but with soundfonts... We don't have that, we would need to use our own. Furthermore, a user might have installed lots of fonts to select from, but it is unlikely that more than two or three soundfonts are installed at the same time... So I don't know if it is worth. But I agree that simuconf.tab is vastly populated x'D

Quote from: TurfIt on August 31, 2020, 01:29:56 AMI also had the ability to select to driver, you've hardcoded sdl2

I actually first tried to implement a similar solution to yours. But in the end, I choose SDL2 over manually setting the driver because:

1. Simutrans preferred backend is SDL2, so we better take advantage of this.
2. Less code, and multi-platform.
3. We can't expect the user to know what audio driver he should use.
4. Automatically setting the audio driver did not work for me. It tried to use jack and it failed miserably.

The SDL2 audio driver in fluidsynth seems to be very recent (December 2019), so I guess you could't use it in the past to know for sure if it inherited those anomalies. I have yet to test on windows, but on linux it creates two audiostreams - one for sounds, the other for music -, so if the behaviour is the same on windows it is unlikely that it will happen. But I'll try later.

Quote from: TurfIt on August 31, 2020, 01:29:56 AMSome other differences I note:

1) IMHO dr_load_midi() should just check the midi files (using fluid_is_midifile()) rather than load it as you have (fluid_player_add()). The later results in very slow Simutrans startup as it scans all the midi files (and fluidsynth loads the instrument patches contained in each for every file).
2) Volume setting  (0.8 * vol / 256) vs (vol / 640) hmm...
3) dr_stop_midi() really needs the all_notes_off.  You get hung notes that play forever otherwise. Can't remember what the _join is for, but also IMHO is makes sense to delete the player here too.
4) dr_midi_pos(), != PLAYING vs == DONE.   
5) you might want to check your editor.. tabs and spaces all over the place.... And tabs in the makefile where they don't belong.

1) Okay, thank you. I've also added fluid_is_soundfont() to check if the soundfont is actually a soundfont.
2) I took those numbers from the OpenTTD implementation. And it worked, so I didn't investigate further. But so does yours, apparently, and louder, so I better apply them instead.
3) I have not noticed anything extrange without all_notes_off, but fine, added. But moving delete_fluid_player from dr_play_midi to dr_stop_midi ends in a segfault, not sure why. fluid_player_join is used to wait for the player to end (i.e. to reach the end of the track), so it doesn't make sense in this context.
4) Actually applying your logic solves the previous segfault, I'm sure why :-)
5) Oof, that's Kate again converting tabs to spaces or viceversa when I copy-paste things. I think I have it sorted now...

Quote from: TurfIt on August 31, 2020, 01:29:56 AMTry SimuSoundFont.7z

Those songs sounds slighty better with that. Thanks!

TurfIt

Quote from: Roboron on August 31, 2020, 12:02:36 PM
With TTF we have a clear idea on where to look for fonts (the fonts directory), but with soundfonts... We don't have that, we would need to use our own.
Actually we don't; Take a look at the path mess in dr_query_fontpath().


Quote from: Roboron on August 31, 2020, 12:02:36 PMFurthermore, a user might have installed lots of fonts to select from, but it is unlikely that more than two or three soundfonts are installed at the same time... So I don't know if it is worth. But I agree that simuconf.tab is vastly populated x'D
Well I had 24. Nows thank to your posting I went looking and found 18 more! Although so far only 4 seem worth keeping as complete GM banks. (Was trying to find a properly licensed and small size one that sounds ok that could possibly hopefully be redistributed with Simutrans).


Quote from: Roboron on August 31, 2020, 12:02:36 PM
1. Simutrans preferred backend is SDL2, so we better take advantage of this.
2. Less code, and multi-platform.
3. We can't expect the user to know what audio driver he should use.
4. Automatically setting the audio driver did not work for me. It tried to use jack and it failed miserably.
The installer still pushes GDI as default... I still prefer SDL1 given it's better performance (need all you can get to drive 4K software rendering), but SDL2 required for non-latin alphabet input.
3,4. Quite true.


Quote from: Roboron on August 31, 2020, 12:02:36 PM3) I have not noticed anything extrange without all_notes_off, but fine, added. But moving delete_fluid_player from dr_play_midi to dr_stop_midi ends in a segfault, not sure why. fluid_player_join is used to wait for the player to end (i.e. to reach the end of the track), so it doesn't make sense in this context.
Makes sense to make sure it's stopped before deleting.
Also, try a good piano soundfont with a piano'y midi, sustained notes persist in the synth after the player stops since the command to snuff never arrives.

Roboron

Quote from: Roboron on August 31, 2020, 12:02:36 PMBut I'll try later.

Actually, I failed. I tried compiling with MSYS2 + mingw64, but I get "ld.exe: cannot find -lfuildsynth" although I have installed mingw-w64-x86_64-fluidsynth. Could it be a problem with mingw?

Quote from: TurfIt on August 31, 2020, 04:40:28 PMTake a look at the path mess in dr_query_fontpath().

Well, yes, it is a mess xD But we got something. What locations do you propose if we do it?

Quote from: TurfIt on August 31, 2020, 04:40:28 PMWell I had 24. Nows thank to your posting I went looking and found 18 more!

You are crazy, sir.

Quote from: TurfIt on August 31, 2020, 04:40:28 PMThe installer still pushes GDI as default... I still prefer SDL1 given it's better performance (need all you can get to drive 4K software rendering), but SDL2 required for non-latin alphabet input.

Yeah, Japanese player often complain about that, so it's an important point to use SDL2 over SDL1. To be honest I don't know why there's a GDI version, I don't know what advantages has over the SDL version to make sense to release those two .-. Not like I care, anyway...

Quote from: TurfIt on August 31, 2020, 04:40:28 PMAlso, try a good piano soundfont with a piano'y midi

I'm now the one who is increasing his soundfont library xD

TurfIt

Looks like only dynamic linking is supported, and it brings in 45 gazillion other dll dependencies, and then it doesn't even run. So this looks dead in the water... :(

---
For font selection paths, I'd suggest the ones you have, and who knows for Windows. Perhaps hijack the simutrans font/ dir.
I did find a promising ancient tiny soundfont that could be distributed with Simutrans I think if desired, and it's already in many linux repos too.... PCLite.  Haven't listened to all the Simutrans MIDIs yet, but so far none that are jarringly bad. And for a 30MB font, sounds ok.


Also, it looks like someone has borked the Simutrans music support. Without this patch, while you can change with MIDI is playing in the sound dialog, if the song ends, Simutrans crashes.
Starting Simutrans results in song 26 playing instead of 1, yet the sound dialog shows 1 playing. If you select >, you get 2 playing and displayed, select <, you get 53 playing, but 52 displayed. Yeesh.

prissi

I think the midi would need more love by a programmer that actually listens to the Simutrans music at all. Unfortunately, I do not.

Ters

I use GDI because then I don't need SDL. I don't really care for sound or music, as I often listen to radio while playing, or even watch television (like Tour de France).

Roboron

Quote from: TurfIt on September 01, 2020, 12:15:42 AMLooks like only dynamic linking is supported

Is this something new or was it true 9 years ago? Are we in a dead end then with the windows support?

If so, I still would like this to be approved if only for the Linux music support. Simutrans's Windows default version doesn't have any problem playing MIDI music, unlike Simutrans Linux, so it wasn't necessary anyway (for Windows).

Quote from: TurfIt on September 01, 2020, 12:15:42 AMFor font selection paths, I'd suggest the ones you have

Actually I mean path for soundfonts. I guess we could look at /usr/share/soundfonts/ and /usr/share/sounds/sf2/ on Linux, while also having a soundfont directory in simutrans folder like we do for fonts. But that's not gonna happen anyway if this solution only support one OS.

Quote from: TurfIt on September 01, 2020, 12:15:42 AMif the song ends, Simutrans crashes.

Can't reproduce this bug. I have tested Simutrans SDL2(compiled from svn) and GDI(121.0 version) on Windows, and none crashed when a song ended.

Quote from: TurfIt on September 01, 2020, 12:15:42 AMStarting Simutrans results in song 26 playing instead of 1, yet the sound dialog shows 1 playing. If you select >, you get 2 playing and displayed, select <, you get 53 playing, but 52 displayed. Yeesh.

I've noticed the first thing, too. I guess someone didn't like Simutrans Main Theme (I can relate) and decided it was better to disguise song 26 as the main theme xD
But I can't reproduce the second one. If I select >, I get 2 playing and displayed. If I select <, I get 1 playing and displayed, as intended. Nothing seems to be wrong :-/

TurfIt

Quote from: Roboron on September 02, 2020, 04:32:24 PM
Is this something new or was it true 9 years ago? Are we in a dead end then with the windows support?
Can't remember. Doesn't look good for MinGW support...


Quote from: Roboron on September 02, 2020, 04:32:24 PM
If so, I still would like this to be approved if only for the Linux music support. Simutrans's Windows default version doesn't have any problem playing MIDI music, unlike Simutrans Linux, so it wasn't necessary anyway (for Windows).
Yes, just frustrated with this broken tools yet again, and some other Simutrans breakages...
Will be hard to test for me Linux only, I've never had success with Linux desktop environments... only server, and even then I've moved to BSDs.
While Windows default can play a MIDI, what comes out the speakers can hardy be described as "music"!


Quote from: Roboron on September 02, 2020, 04:32:24 PM
Actually I mean path for soundfonts. I guess we could look at /usr/share/soundfonts/ and /usr/share/sounds/sf2/ on Linux, while also having a soundfont directory in simutrans folder like we do for fonts.
That's what I meant; You have some paths to a default soundfont for linux in your patch, presumably that's where prepackaged fonts on Linux end up, but varying by distro of course, 'cause reasons..



Quote from: Roboron on September 02, 2020, 04:32:24 PM
But I can't reproduce the second one. If I select >, I get 2 playing and displayed. If I select <, I get 1 playing and displayed, as intended. Nothing seems to be wrong :-/
Selecting < when 26 (the fake 1) is playing.   Not after switching to 2...
I'm guessing a bunch of songs were added, and something's now overflowing.

prissi

#15
It was asked for a random sound to play. Probably random is not working at that point yet.

Wildmid compiles also under mingw (with some effort due to broken msys Cmake) and comes as a single 2 MB static library. So this may be a more portable alternative. It would even support BSD, Haiku and Amiga OS ...

Roboron

#16
Quote from: TurfIt on September 02, 2020, 04:57:01 PMWill be hard to test for me Linux only, I've never had success with Linux desktop environments...

If someday you want to test, the easiest way of doing it is to set up an Arch/Manjaro system and install my own package of simutrans-svn. It will get the source from GitHub, get the dependencies, apply some linux-specific configuration patches and compile the binary. You only need to press install x)

Quote from: TurfIt on September 02, 2020, 04:57:01 PMWhile Windows default can play a MIDI, what comes out the speakers can hardy be described as "music"!
¯\_(ツ)_/¯

Quote from: TurfIt on September 02, 2020, 04:57:01 PMThat's what I meant; You have some paths to a default soundfont for linux in your patch, presumably that's where prepackaged fonts on Linux end up, but varying by distro of course, 'cause reasons..

Oh. You said font so that confused me. Yes, those paths are were soundfonts are stored. I like more /usr/share/soundfonts than /usr/share/sounds/sf2, and it's a matter of taste, apparently xD Sadly the FHS specification doesn't cover soundfonts, but only two paths is relatively a small number of paths when dealing with such things in the linux world :^)

I have now checked what soundfonts are available via package manager and I found a 200+ MB (!) soundfont that is GPL and in the official repositories called FreePats General MIDI sound set. I have tried it and... oh boy... NOW that's music. That piano sounds nearly like a real piano! I'm delighted to listen to the Simutrans music now!

And it seems that it was updated the previous month and it's still improved. I'm with no doubt adding it to the top of the list, and also adding it as and optional dependency for my arch package if this patch get approved.

Quote from: TurfIt on September 02, 2020, 04:57:01 PMI'm guessing a bunch of songs were added, and something's now overflowing.

Actually, it's a lot easier than that. I have investigated the problems and I found the root causes of all them, which are not the same, and are unrelated.

1 - Starting Simutrans results in song 26 playing instead of 1

If you look at line 1281 of simmain.cc, you will find the following:

Code (simmain.cc) Select
// not muted => play random song
midi_play(-1);


According to the code, Simutrans should play a random song. But that's not what's happening! Why? Let's look at midi_play() in simsound.cc and we will find it calling the random generator sim_async_rand():

Code (utils/simrandom.cc) Select
static uint32 async_rand_seed = 12345678+dr_time();

// simpler simrand for anything not game critical (like UI)
uint32 sim_async_rand( uint32 max )
{
if(  max==0  ) {
return 0;
}
async_rand_seed *= 3141592621u;
async_rand_seed ++;

return (async_rand_seed >> 8) % max;
}


So a typical random generator, just using time as usual to generate a seed... But what time?

Code (sys/simsys_s2) Select
uint32 dr_time()
{
return SDL_GetTicks();
}


Aha! So that's the problem here. dr_time() will not return the actual time like you could expect from a time function, but the number of ticks since the SDL library initialization! And how much is that if you just started simutrans?? ZERO. Always.
So we are generating the same seed, everytime we try to play the first random song, and as such we get always the same "random" song!

I'm not confident enough to modify dr_time(), because getting the ticks instead of the time may be critical for other things that are not related to music and that I completely ignore, but we can just include <time.h>  and use that instead of dr_time() for music (async_rand_seed is just used for music and two or three "not game critical" things according to the comment). Voilà! First problem solved.

Song 26 (first) playing, yet the sound dialog shows 1 playing

I first thought this could be a problem of song not getting checked the first time the UI load. But sound_frame.cc seems ok. Then I went back and discovered something: the midi_play() function used on simmain.cc, it was only used there, and it was only called one time at the start. Suspicious.

Code (simcound.cc) Select
void midi_play(const int no)
{
if(  no > max_midi  ) {
dbg->warning("midi_play()", "MIDI index %d too high (total loaded: %d)", no, max_midi);
}
else if(  !midi_get_mute()  ) {
dr_play_midi( (no<0) ? sim_async_rand(max_midi) : no );
}
}


The problem is here. midi_play is starting playback but it is not updating current_midi, which is used by the UI to display the song playing. Solved!

3 - If you select >, you get 2 playing and displayed, select <, you get 53 playing, but 52 displayed.

Actually, there are no bugs in the code causing this. Indeed, I would say that this is the... intended behaviour. And that's it because... there's only 52 songs being loaded!

Code (Simutrans log) Select
  Reading MIDI file '/home/rober/.local/share/simutrans/music/47-Salty-Breeze.mid' - Salty Breeze
  Reading MIDI file '/home/rober/.local/share/simutrans/music/48-Techno-movement.mid' - Techno-movement
  Reading MIDI file '/home/rober/.local/share/simutrans/music/49-Last-Sunday.mid' - Last Sunday
  Reading MIDI file '/home/rober/.local/share/simutrans/music/51-Summer-Intersection.mid' - Summer Intersection
  Reading MIDI file '/home/rober/.local/share/simutrans/music/52-Dreamy-Oriental-Nights.mid' - Dreamy Oriental Nights
  Reading MIDI file '/home/rober/.local/share/simutrans/music/53-Snowy-Road.mid' - Snowy Road


It's clear what's happening there. Simutrans load all the songs and assign number from order of load. Song number 50 is actually song number 51, and so on. And where it is the actual song number 50? It is not loaded, because is commented in music.tab, which was a decision made by prissi two years ago, because it was apparently disturbing  :D

Quote from: prissiAdded to Standard (but No4950 commented out). Thanks for sahring!
Original post = https://forum.simutrans.com/index.php/topic,18461.0.html

How can we solve this? We could just delete song number 50 if it is not loved, but we can also move it to the end of the list (number 53) so there's no mismatch between songs and song titles. My attached patch suggest this approach, alongside with previous fixes ^^


What this patch changes

  • Added default.sf2 first on the list (which is actually what fluidsynth looks for if you run it standalone), followed by freepats-general-midi.sf2 (apparently only available for arch)
  • Remove the fluidsynth section from the Makefile for windows
  • Added a clarification in config.template indicating that USE_FLUIDSYNTH_MIDI will only be used for linux/macOS*
  • Added a clarification in simuconf.tab indicating that soundfont_filename is only available for linux/macOS*, and also recommending the freepats soundfont
  • Changed async_rand_seed() to use <time.h> time()  instead of dr_time() (fix first random song)
  • Fixed midi_play() to set the first current_midi
  • Swap Where-Thomassons-Lie(previously 50, now 53) with Snowy-Road(previously 53, now 50) in music.tab so there is no mismatch between songs/songs numbers (you also need to rename them in the folder)
  • That's all, I can split the last three if you don't want to implement the fluidsynth backend.

* Teorically, it should work on macOS too if you install fluidsynth, but I can't test it.

P.D.: Sorry if I seem a bit enthusiastic suggesting changes, please do criticize my decisions if you think/know they are wrong.

Yona-TYT

Quote from: Roboron on September 03, 2020, 01:16:26 AMI have now checked what soundfonts are available via package manager and I found a 200+ MB (!) soundfont that is GPL and in the official repositories called FreePats General MIDI sound set. I have tried it and... oh boy... NOW that's music. That piano sounds nearly like a real piano! I'm delighted to listen to the Simutrans music now!
Wow this looks promising.

prissi

wildmidi can use freepats too. (It is actually the default since 2006 according to config files.) Since wildmidi compiles also on windows, maybe wildmidi is the way to go.

As for random number initialisation, we could use the time() function, with second since 1970. That should be random enough. (And aparently on windows this was not a problem, since it returns ms since midnight.)

TurfIt

Quote from: Roboron on September 03, 2020, 01:16:26 AM
I have now checked what soundfonts are available via package manager and I found a 200+ MB (!) soundfont that is GPL and in the official repositories called FreePats General MIDI sound set.
"Currently the sound set is incomplete:" is a little concerning to recommend this. If instruments a midi call for are missing - silence.
Originally I was looking for something small that could be distributed with Simutrans, mainly for Windows since there are not easy 'packages' for it. But since the library is in bad shape for Windows... moot.
But, still consider the 'personal copy' soundfont as sounding reasonable, and smallish too. Even the Lite version at 30MB is quite decent with the Simutrans songs (not so much some others, where I much prefer the 400MB one I'd sent you).



Quote from: Roboron on September 03, 2020, 01:16:26 AM
Actually, it's a lot easier than that. I have investigated the problems and I found the root causes of all them, which are not the same, and are unrelated.
Thanks very much for this. I've not had time to look at any of this more...
Should probably commit those fixes now, not related to fluidsynth.


Quote from: Roboron on September 03, 2020, 01:16:26 AM
I'm not confident enough to modify dr_time(), because getting the ticks instead of the time may be critical for other things that are not related to music and that I completely ignore, but we can just include <time.h>  and use that instead of dr_time() for music (async_rand_seed is just used for music and two or three "not game critical" things according to the comment). Voilà! First problem solved.
Using time() to initialize the async rands is good. Modifying dr_time() would likely be quite bad - used for main game timing loop, leave as SDL_Ticks().


Quote from: prissi on September 03, 2020, 08:44:05 AM
As for random number initialisation, we could use the time() function, with second since 1970. That should be random enough. (And aparently on windows this was not a problem, since it returns ms since midnight.)
Very much a problem on Windows, that's where I noticed it.


Quote from: Roboron on September 03, 2020, 01:16:26 AM
Actually, there are no bugs in the code causing this. Indeed, I would say that this is the... intended behaviour. And that's it because... there's only 52 songs being loaded!
Yeesh. 1,2,3,4,6, yup, that's how I count!


Quote from: Roboron on September 03, 2020, 01:16:26 AM
P.D.: Sorry if I seem a bit enthusiastic suggesting changes, please do criticize my decisions if you think/know they are wrong.
Not at all. A push is needed.


---
Quote from: prissi on September 03, 2020, 08:44:05 AM
wildmidi can use freepats too. (It is actually the default since 2006 according to config files.) Since wildmidi compiles also on windows, maybe wildmidi is the way to go.
From a quick look, wildmidi is using GUS pats. Usually don't sound quite right with GM songs - they need tweaking to sound right on a GUS.
But the bigger 'problem' is wildmidi is simply rendering the midi to a memory buffer. The program (Simutrans) then has to spit out the audio stream. That means needing SDL_Mixer  (or something similar) added to Simutrans, which Roboron was trying to get away from. Fluidsynth can handle the audio output itself.


Roboron

Good news, everyone.

I have been able to compile Simutrans for Windows with Fluidsynth statically linked! You can test* it downloading the executable from my nextcloud: https://michan.noho.st/nextcloud/s/MWTQ28r3SNoHzaX

I'll post later about how I managed to do it, I need some rest now. It has been a hacky solution that took some hours to figure, and still I may consider asking for help on the mailing list to improve it.

* Remember to set soundfont_filename in simuconf.tab, otherwise you wont't hear anything.
** This version does not include previous MIDI fixes.

Roboron

Quote from: TurfIt on September 03, 2020, 12:25:41 PM"Currently the sound set is incomplete:" is a little concerning to recommend this. If instruments a midi call for are missing - silence.

I have yet to see a soundfont that is "completely complete". Even your 400 MB soundfont has missing instruments. I'm recommending it because it is the best I've found and it is GPL. But whatever my recommendation is, the user can choose whatever soundfont he wants. I'm also not suggesting to include it alongside Simutrans; it's too big for that.

Quote from: TurfIt on September 03, 2020, 12:25:41 PMOriginally I was looking for something small that could be distributed with Simutrans, mainly for Windows since there are not easy 'packages' for it. But since the library is in bad shape for Windows... moot.

Welp, now that we have "sorted" that problem we may further discuss this. You have mentioned PCLite soundfont but I've not been able to find it doing a quick search, do you have any links? I would like to listen to it.

Quote from: prissi on September 03, 2020, 08:44:05 AMwildmidi can use freepats too. (It is actually the default since 2006 according to config files.) Since wildmidi compiles also on windows, maybe wildmidi is the way to go.

Actually, I've deal with SDL_Mixer a bit more and I discovered that Timidity++ does work, unlike I thought. It just doesn't work with Soundfonts, but with GUS patches, and the timidity++ folder is not where the Arch Wiki told me it should be:

Quote from: SDL_Mixer READMETo play MIDI files using Timidity, you'll need to get a complete set of
GUS patches from:
http://www.libsdl.org/projects/mixer/timidity/timidity.tar.gz
and unpack them in /usr/local/lib under UNIX, and C:\ under Win32.

Looking at wildmidi, it seems that it also doesn't have support for Soundfonts, but GUS patches. So, it doesn't give much benefit over current SDL_Mixer (apart from no being **** confusing, I guess).

Now, let's get back to the explanation:

How I linked Fluidsynth statically to Simutrans (MSYS2)

1 - As you already know, I installed mingw-w64-x86_64-fluidsynth, but the fluidsynth library is not found by the linker.
2 - I got the Fluidsynth source from https://github.com/FluidSynth/fluidsynth and compiled it following the wiki https://github.com/FluidSynth/fluidsynth/wiki/BuildingWithCMake#building-with-msys2-on-windows. This gave me a fluidsynth-2 library which I renamed to fluidsynth, and it linked without errors. However, at runtime, it asks for all Fluidsynth dependencies (sdl2.dll, readline.dll, etc).
3 - I realized that what I've got in step 2 was not a static library and went to research a bit more. I found that I could build fluidsynth with CMake parameter -DBUILD_SHARED_LIBS=0, which sounds exactly like what I need. I also disabled the libraries we don't need (everything but sdl2), and I got the final command to call CMake:

cmake -DBUILD_SHARED_LIBS=0 -Denable-readline=0 -Denable-winmidi=0 -Denable-waveout=0 -Denable-libsndfile=0 -Denable-network=0 -Denable-dsound=0 -G "MSYS Makefiles" ..

4 - Now this gave me a proper fluidsynth.a file. I tried again... and ld failed again, but with another error:

[spoiler]C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x7c): undefined reference to `__imp_fluid_settings_setnum'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0xf2): undefined reference to `__imp_fluid_is_midifile'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x191): undefined reference to `__imp_fluid_player_get_status'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x1a3): undefined reference to `__imp_fluid_player_stop'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x1b5): undefined reference to `__imp_fluid_synth_all_notes_off'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x1c2): undefined reference to `__imp_delete_fluid_player'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x1cf): undefined reference to `__imp_new_fluid_player'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x1ef): undefined reference to `__imp_fluid_player_add'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x202): undefined reference to `__imp_fluid_player_play'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x24d): undefined reference to `__imp_fluid_player_stop'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x25f): undefined reference to `__imp_fluid_synth_all_notes_off'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x271): undefined reference to `__imp_delete_fluid_player'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x28d): undefined reference to `__imp_fluid_player_get_status'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x2bd): undefined reference to `__imp_fluid_player_stop'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x2cf): undefined reference to `__imp_fluid_synth_all_notes_off'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x2dc): undefined reference to `__imp_delete_fluid_player'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x2e9): undefined reference to `__imp_delete_fluid_audio_driver'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x2f6): undefined reference to `__imp_delete_fluid_synth'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x303): undefined reference to `__imp_delete_fluid_settings'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x33e): undefined reference to `__imp_new_fluid_settings'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x35c): undefined reference to `__imp_fluid_settings_setstr'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x369): undefined reference to `__imp_new_fluid_synth'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x382): undefined reference to `__imp_new_fluid_player'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x3a6): undefined reference to `__imp_new_fluid_audio_driver'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x3c4): undefined reference to `__imp_fluid_is_soundfont'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x3d8): undefined reference to `__imp_fluid_synth_sfload'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: build/default/music/fluidsynth.o:fluidsynth.cc:(.text+0x482): undefined reference to `__imp_fluid_synth_sfload'
[/spoiler]

It looks like ld is not able to find fluidsynth.h in the system? I don't know why is that, if it the code threw no error at compile time .-. I just modified fluidsynth.cc to include "fluidsynth.h" instead of <fluidsynth.h> and copied the header file manually. It worked, but not at all, since ld threw new undefined references:

C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:/msys64/mingw64/lib\libfluidsynth.a(fluid_settings.c.obj): in function `new_fluid_settings':
C:/msys64/home/Rober/fluidsynth-2.1.4/src/utils/fluid_settings.c:274: undefined reference to `g_rec_mutex_init'


5 - To solve this, I had to add -lglib-2.0 to LDFLAGS. Then recompiled, new undefined references, new LDFLAG, and so on. Until no undefined references were left:

LDFLAGS += -lfluidsynth -lglib-2.0 -lintl -liconv

Fortunately since our project already requires sdl2 that was not necessary xD Now after all this, make successfully compiles an executable with fluidsynth (and dependencies) linked statically.

However, I feel like there's room for improvement here. Why is #4 failing to locate header's file? Shouldn't be -lglib-2.0 -lintl -liconv in #5 already statically linked to fluidsynth.a so I don't have to link them yet again? I'm afraid I don't know enough to know how to solve this, so maybe you can help me, before reaching to the mailing list.

TurfIt

Quote from: Roboron on September 03, 2020, 09:39:36 PM
I have yet to see a soundfont that is "completely complete". Even your 400 MB soundfont has missing instruments. I'm recommending it because it is the best I've found and it is GPL. But whatever my recommendation is, the user can choose whatever soundfont he wants. I'm also not suggesting to include it alongside Simutrans; it's too big for that.
Maybe the freepats sf2 is doing something my ancient timidity++ player can't handle, but it does not render the Simutrans songs correctly at all. Will need to try it with fluidsynth if I (you!) get it working.


Quote from: Roboron on September 03, 2020, 09:39:36 PM
Welp, now that we have "sorted" that problem we may further discuss this. You have mentioned PCLite soundfont but I've not been able to find it doing a quick search, do you have any links? I would like to listen to it.
http://www.personalcopy.com/sfarkfonts1.htm


Quote from: Roboron on September 03, 2020, 09:39:36 PM
It looks like ld is not able to find fluidsynth.h in the system? I don't know why is that, if it the code threw no error at compile time .-. I just modified fluidsynth.cc to include "fluidsynth.h" instead of <fluidsynth.h> and copied the header file manually. It worked, but not at all, since ld threw new undefined references:
Linker errors have nothing to do with header files. After you've build your custom fluidsynth library, where is it/you installing it? If somewhere other than the main include/lib directories (eg. /msys64/mingw32/include/) the Simutrans build needs the directories added to it's search paths.


Quote from: Roboron on September 03, 2020, 09:39:36 PM
LDFLAGS += -lfluidsynth -lglib-2.0 -lintl -liconv
Explicit linking of these extra libraries used by fluidsynth is normal; They don't get linked into the fluidsynth library, but into Simutrans.exe itself.
Look at the sdl2-config for an example:

#    --libs|--static-libs)
      echo -L${exec_prefix}/lib  -lmingw32 -lSDL2main -lSDL2 -mwindows  -lmingw32 -ldxerr8 -ldinput8 -lshell32 -lsetupapi -ladvapi32 -luuid -lversion -loleaut32 -lole32 -limm32 -lwinmm -lgdi32 -luser32 -lm  -mwindows -Wl,--no-undefined -pipe
      ;;

All those extra libraries...
And in pkg-config format:

Libs: -L${libdir}  -lmingw32 -lSDL2main -lSDL2 -mwindows
Libs.private: -lmingw32 -ldxerr8 -ldinput8 -lshell32 -lsetupapi -ladvapi32 -luuid -lversion -loleaut32 -lole32 -limm32 -lwinmm -lgdi32 -luser32 -lm  -mwindows -Wl,--no-undefined -pipe
Cflags: -I${includedir}/SDL2  -Dmain=SDL_main

So for your custom fluidsynth library, you need your own fluidsynth-config script, or fluidsynth.pc file, or just add the required in the Simutrans Makefile / config.default.

Hopefully you end up with something that the Github actions can handle...

TurfIt

#23
I can compile, but no sound output after applying the fluidsynth patch. Sound effects dead too, not just music.

1) Grab fluidsynth source - fluidsynth-2.1.4.tar.gz
2) cmake -DBUILD_SHARED_LIBS=0 -Denable-readline=0 -Denable-winmidi=0 -Denable-waveout=0 -Denable-libsndfile=0 -Denable-network=0 -Denable-dsound=0 -G "MSYS Makefiles" .
3) make libfluidsynth
4) copy built includes, and lib to mingw dirs
5) apply fluidsynth simutrans patch
6) modify LIBS += -lfluidsynth -lglib-2.0 -lintl -liconv -lws2_32   << note I needed the winsock lib added here too.
7) make
8 ) edit simuconf.tab for soundfont selection
9) run sim.exe and no sound at all.  The .exe you provided does work (and confirms freepats sounds like .....)

progress, yet not!

EDIT: Well, that was the most idiotic asinine thing... Not sure if that's me, or the system, but... run trunk sim.exe, works. run your fluidsynth sim.exe, works. run my fluidsynth sim.exe, and the win10 mixer decides to set the application volume to 0. wut?
In the process, I can confirm the dsound driver works too. And that there's yet another bug in Simutrans midi handling.  if dr_init_midi() returns failure, other midi functions are still called. Fluidsynth does not like that and promptly crash closes Simutrans.

Roboron

#24
Quote from: TurfIt on September 04, 2020, 01:32:47 AMLinker errors have nothing to do with header files. After you've build your custom fluidsynth library, where is it/you installing it? If somewhere other than the main include/lib directories (eg. /msys64/mingw32/include/) the Simutrans build needs the directories added to it's search paths.

I was copying them manually to /lib and /include directories. But TIL I can use "-DCMAKE_INSTALL_PREFIX=$MINGW_PREFIX" with cmake and then "make install" will take care of it. Works now.

Quote from: TurfIt on September 04, 2020, 01:32:47 AMExplicit linking of these extra libraries used by fluidsynth is normal

Good to know this is the right way.

Quote from: TurfIt on September 04, 2020, 01:32:47 AMSo for your custom fluidsynth library, you need your own fluidsynth-config script, or fluidsynth.pc file, or just add the required in the Simutrans Makefile / config.default.

Hopefully you end up with something that the Github actions can handle...

I've been playing with it today, and I got something which integrates into the autoconf + /.configure workflow (is this what GitHub actions use?). If it finds fluidsynth, it makes the windows version use it (it doesn't matter the backend) and link the extra libraries (I also needed to link -lole32 for this implementatio, but got rid of sdl2 for building fluidsynth). For other systems, requires SDL2. I have splitted this patch in two, since I think the fluidsynth-related code is nearly complete, while the MAKEFILE configuration needs some serious review (I'm a total noob on this).

Quote from: TurfIt on September 04, 2020, 12:53:39 PM-lws2_32   << note I needed the winsock lib added here too.

You shouldn't. It is already required for mingw builds :/

Quote from: TurfIt on September 04, 2020, 01:32:47 AMhttp://www.personalcopy.com/sfarkfonts1.htm

I have to agree it is good enough, and light enough, to be included alongside the game. I approve.

Quote from: TurfIt on September 04, 2020, 12:53:39 PMIn the process, I can confirm the dsound driver works too.

We'll use it instead of SDL2 for windows builds, then. I've already applied the necessary changes.

Quote from: TurfIt on September 04, 2020, 12:53:39 PMAnd that there's yet another bug in Simutrans midi handling.  if dr_init_midi() returns failure, other midi functions are still called. Fluidsynth does not like that and promptly crash closes Simutrans.

I have tried not setting a soundfont to make dr_init_midi() fail, but I can't reproduce this. No matter if I let the game running or directly try to control music, Simutrans won't crash. How did you do it?

Attached file includes both patches and a script to prepare the system* to compile Simutrans with fluidsynth support (based on this script from prissi).

* Of course you need to apply the patches first, and then run autoconf and ./configure, I have commented those.

TurfIt

Quote from: Roboron on September 04, 2020, 09:03:32 PM
I was copying them manually to /lib and /include directories. But TIL I can use "-DCMAKE_INSTALL_PREFIX=$MINGW_PREFIX" with cmake and then "make install" will take care of it. Works now.
I don't get why cmake defaults to a c:\program files (x86)\.... prefix. Useless PITA.


Quote from: Roboron on September 04, 2020, 09:03:32 PM
while the MAKEFILE configuration needs some serious review (I'm a total noob on this).
It does. The Simutrans Makefile in general is completely FUBAR.


Quote from: Roboron on September 04, 2020, 09:03:32 PM
You shouldn't. It is already required for mingw builds :/
Link order matters. e.g. I don't need ole32 with your revised Makefile, nor ws2_32 anymore. Like I said FUBAR.   LD_FLAGS != LIBS either... further complicating things.


Quote from: Roboron on September 04, 2020, 09:03:32 PM
I have to agree it is good enough, and light enough, to be included alongside the game. I approve.
I was hoping to make it an optional download, like the wenquanyi text font is. But that's needs a default, which on Windows is problematic (I'm assuming all Linux distros have atleast a simple soundfont package that could be a Simutrans prereq). There is c:\Windows\System32\drivers\gm.dls, and Fluidsynth can be compiled with DLS support, but it sounds worse than the w32_midi backend file.  With Fluidsynth DLS, it needs a ton more libraries bloating Simutrans.exe by 4.5MB!  So... that's not good.
And the Makefile mess for DLS:

+= -lfluidsynth -linstpatch-2  -lgobject-2.0 -lglib-2.0 -lgthread-2.0 -lpcre -lintl -liconv -ldsound  -lffi -lsndfile -lflac -lvorbisenc -lvorbis  -lopus -logg



Quote from: Roboron on September 04, 2020, 09:03:32 PM
We'll use it instead of SDL2 for windows builds, then. I've already applied the necessary changes.
I think it's better to stick with SDL2 driver if the backend is SDL2. Use dsound only for GDI builds. (Although I'm not sure the purpose of maintaining GDI any longer...)


Quote from: Roboron on September 04, 2020, 09:03:32 PM
I have tried not setting a soundfont to make dr_init_midi() fail, but I can't reproduce this. No matter if I let the game running or directly try to control music, Simutrans won't crash. How did you do it?
Make it fail sooner!  The checks for "if(  player  )" in my original patch solve this. I'll attach my current state of your patch soon(ish).

Roboron

Quote from: TurfIt on September 05, 2020, 09:41:20 PMI don't get why cmake defaults to a c:\program files (x86)\.... prefix. Useless PITA.

¯\_(ツ)_/¯

Quote from: TurfIt on September 05, 2020, 09:41:20 PMI was hoping to make it an optional download, like the wenquanyi text font is.

I... didn't know that was an optional download, lol. It's not mentioned anywhere :/

Quote from: TurfIt on September 05, 2020, 09:41:20 PMBut that's needs a default, which on Windows is problematic. There is c:\Windows\System32\drivers\gm.dls, and Fluidsynth can be compiled with DLS support, but it sounds worse than the w32_midi backend file.  With Fluidsynth DLS, it needs a ton more libraries bloating Simutrans.exe by 4.5MB!  So... that's not good.

I have also considered using gm.dls for Windows if no soundfont was found. But I was not able to compile libinstpatch with MINGW (and it doesn't seem to be available via pacman), so I gave up. We may consider some alternatives:

1. Can we somehow fallback to w32_midi backend on windows if fluidsynth backend fails?

2. We could introduce an alert windows at the start that:
  2.1. Would alert the player that no soundfont was found and that therefore no music will be played
  2.2. Would suggest the player to download the recommended (PCLite) soundfont
  2.3. (Opt) Have a check option "don't show again"
  2.4. (Opt) If we want to do it more straightforward, we could include a download button to do 2.2. automatically
  2.5. (Opt) Also a button to restart the music system after 2.2. without restarting Simutrans?

3. Not exclusive with the previous one, we may come back to the idea of a soundfont selector in sound settings
  3.1. With an error message more helpful than "Invalid MIDI Index!" (in this case, "Soundfont not found, download and select a soundfont".
  3.2. With a similar layout to the font selector, but checking simutrans/soundfonts (plus linux paths)
  3.3. We would also need to implement 2.5 here, and maybe 2.4

What do you think?

Quote from: TurfIt on September 05, 2020, 09:41:20 PMI'm assuming all Linux distros have atleast a simple soundfont package that could be a Simutrans prereq

Yes, as mentioned Fluid_R3 seems to be available everywhere, so it could be included as an (optional) dependency.

Quote from: TurfIt on September 05, 2020, 09:41:20 PMI think it's better to stick with SDL2 driver if the backend is SDL2. Use dsound only for GDI builds. (Although I'm not sure the purpose of maintaining GDI any longer...)

I'm also not sure of that purpose, and the only answer I've had is "not to use SDL2" (if this is a benefit, I fail to see why). But some purpose it may has: recently, Simutrans Extended dropped support for deprecated backends (allegro, sdl, sdl_mixer) but it kept GDI ¯\_(ツ)_/¯ https://github.com/jamespetts/simutrans-extended/pull/263

Anyway, that's not my decision to make, so I have to think on the other backends. Although I'm doing this not only to improve MIDI playback, but to deprecate sdl_mixer(2) backend and not to think about it again :^)

Back to the topic, I don't care wheter we use dsound or SDL2 with the SDL2 version. I just saw easier to just check if the system is windows or is not. Not because the ifdef, but also because:

1 - We can always compile fluidsynth with sdl2 off and dsound on
2 - We can always include -ldsound in the makefile and make use of it. Otherwise (SDL2) we are including it but not using it.
3 - If we compile fluidsynth with sdl2 on but we then use GDI, I think we still need to dinamically link -lSDL2, and that's bad.

So, for automation purposes, it's better to check if we are for windows and then use dsound, than to check wheter we are using GDI or SDL2. Yes, everything would be way easier if we only used SDL2, but......

Quote from: TurfIt on September 05, 2020, 09:41:20 PMMake it fail sooner!  The checks for "if(  player  )" in my original patch solve this. I'll attach my current state of your patch soon(ish).

Nice! Another thing I should have copied xD

prissi

The GDI interface is easier to debug, especially when grafics are concerned. Occasionally the IME support did not work on well on either SDL2 or GDI (currently GDI though).

Roboron

#28
I have keep working on this on and off, and today I have finally ended the work on the GUI. The following patch has (in addition to the work made before):

1. A new settings window to select a soundfont, which search for soundfonts on the music directory. After loading a soundfont, music will start playing if not already playing.
2. A warning dialog will pop-up warning the user if no soundfont was found at the start.
3. Changes and fixes to the sound menu. Unmute music button will be disabled if no soundfont is selected, along with next/previous tracks. Status of those buttons will be also updated along with track info, since the the soundfont dialog can unmute the midi.
4. I've also made possible to save the selected soundfont to the settings.xml file, and restore it instead of the value on simuconf.tab, so the previous selected soundfont is loaded.

#2 can be improved, since it won't go away until you close the start menu, and sometimes it can appear behind it 😕 Any ideas?

I've based the soundfont selector on the font selector - mostly renamed it, stripped away what I didn't need, and added what I needed. I may have left something unnecessary still, so a review will be welcome - it's my first time working with GUI code.

I've also had to add some extra strings. Is there any automatic tool to update the base texts file, or is it done manually? Anyway, those strings are:

Quote"Select a soundfont"
"Soundfonts are searched on the music directory."
"Soundfont not found. Please select a soundfont below"
"Music initialized but no soundfont found!\n\nMusic won't play until you load a soundfont from the sound options menu."
"Music playing disabled/not available" <- This existed already, but it was not managed by translator::translate, I added that.

After some changes to fluidsynth.cc I've stopped experiencing the crashes commented before. But I'm sure TurfIt will have something to say/add/improve about this.

Feedback is welcomed and appreciated. I'm of the opinion that those changes make this a good enough solution for the general user, and I'm looking forward to see this backend become the default for linux soon, removing the need of mixer_sdl and their issues entirely. I'll let you with a brief demo of the functionality below.



EDIT: Ups, previous patch didn't include the loadsoundfont_frame. Now it does. Also, here you have a binary for Windows to test. Compiled with mingw.

prissi

Do not worry about windows. After reading this, I am sure whenever I want to compile it in two months, or next year, it will be broken again. (Had this experience too often.) Furthermore, Windows has a working midi driver, which does not require advanced user interaction (like downloading and installing soundfonts). Same for MacOS.

NB default cmake does not work under msys2, since it claims to be unix. (WIN32 is not defined!) Official cmake cannot built mingw makefile. (Or I am too stupid, but I tried 3 days andgot nothing to work.) For that reason I will also not include something depending on msys2 Cmake.

Of course, under Linux things are different. I would suggest to add fluidsynth configure.ac and then enabled when properly installed (or display a warning etc.) I mean, the official SImutrans is SDL2 without mixer.

Where is the latest version of the patch to look at?

Offtopic: But GDI can handle Japnese and other 2 byte languages since ages, SDL2 just adds another layer in between and has troubles with latin2 languages, since it works with scancodes for "single keystroke letters".

Roboron

Quote from: prissi on February 06, 2021, 01:15:06 PMNB default cmake does not work under msys2, since it claims to be unix. (WIN32 is not defined!) Official cmake cannot built mingw makefile. (Or I am too stupid, but I tried 3 days andgot nothing to work.) For that reason I will also not include something depending on msys2 Cmake.

(I don't know what NB is). I think you need to use mingw cmake to build "MinGW Makefiles" (that makes sense). In MSYS2 you have to use either mingw32 or ming64 shell with their corresponding cmake. More information can be found at https://github.com/FluidSynth/fluidsynth/wiki/BuildingWithCMake#building-with-msys2-on-windows (but you also have the mingw-setup.sh adapted for fluidsynth at the top of this thread).

Quote from: prissi on February 06, 2021, 01:15:06 PMI would suggest to add fluidsynth configure.ac and then enabled when properly installed

Already done some months ago:

Quote from: Roboron on September 04, 2020, 09:03:32 PMI got something which integrates into the autoconf + /.configure workflow (is this what GitHub actions use?). If it finds fluidsynth, it makes the windows version use it (it doesn't matter the backend) and link the extra libraries (I also needed to link -lole32 for this implementatio, but got rid of sdl2 for building fluidsynth). For other systems, requires SDL2.

Quote from: prissi on February 06, 2021, 01:15:06 PMWhere is the latest version of the patch to look at?

I always update the first post with the last patch I've uploaded.

TurfIt

Quote from: Roboron on February 06, 2021, 05:12:37 PM
(I always update the first post with the last patch I've uploaded.
I never look there for the latest, should always remain the original...

Anyway, looks quite good. I cleaned it up a bit - spaces for indentation, gah! Please don't do that again...
Removed double calls to translator - some gui elements automatically do this, no need to explicitly add.
Forced the no soundfont warning dialog to open always in the upper left.
Reverted fluidsynth.cc closer to the original with the if (player) checks. Rather leave those in since they fixed crashes in the past...
I'm tempted to leave the soundfont selection dialog available even if not compiled with fluidsynth support. Could display a message "You're missing this feature" to shame those who would produce a .exe without the ability for passable music!

If the attached still looks good to you, then let's just commit it be done.


Quote from: prissi on February 06, 2021, 01:15:06 PM
Do not worry about windows. After reading this, I am sure whenever I want to compile it in two months, or next year, it will be broken again. (Had this experience too often.) Furthermore, Windows has a working midi driver, which does not require advanced user interaction (like downloading and installing soundfonts). Same for MacOS.

NB default cmake does not work under msys2, since it claims to be unix. (WIN32 is not defined!) Official cmake cannot built mingw makefile. (Or I am too stupid, but I tried 3 days andgot nothing to work.) For that reason I will also not include something depending on msys2 Cmake.
cmake is annoying, but does work to build the fluidsynth library under Msys2 MinGW.
cmake -DCMAKE_INSTALL_PREFIX=$MINGW_PREFIX -DBUILD_SHARED_LIBS=0 -Denable-aufile=0 -Denable-dbus=0 -Denable-ipv6=0 -Denable-jack=0 -Denable-ladspa=0 -Denable-midishare=0 -Denable-opensles=0 -Denable-oboe=0 -Denable-oss=0 -Denable-readline=0 -Denable-winmidi=0 -Denable-waveout=0 -Denable-libsndfile=0 -Denable-network=0 -Denable-pulseaudio=0 Denable-dsound=1 -Denable-sdl2=0 -G "MSYS Makefiles" ..
To reduce the number of dependent librarys. Alternatively, a Msys2 package is available, mingw-w64-x86_64-fluidsynth, however it's only setup for dynamic linking, and required a ton of extra libraries, not used by Simutrans implementation.

prissi

#32
It still leaves the problem, that on MacOS and Windows, one needs to download a soundfont. (Or the installer needs to be changed to do that.) But that would be an external link, which may change. Honestly, for windows I think why bother to fix something, which is not broken (the windows midi is bad, not simutrans pre se.) Thus, on windows one can replace the MS MM midi player by something like http://coolsoft.altervista.org/en/virtualmidisynth and get better sound for all programs. This is probably the better solution for the windows guys caring about midi sound.

But since fluidsynth is only built when installed, I would say please submit it. (The chances are low, that it is installed with the pre-environment.)

Roboron

Quote from: prissi on February 07, 2021, 01:07:15 PMHonestly, for windows I think why bother to fix something, which is not broken

Nobody said it was a fix for windows. It is a fix for linux, but work being done, it adds some value and it was trivial to enable it for other systems, if only optionally. I agree that making it the default without an out-of-the-box solution (like shipping a default soundfont) is not desirable, but that's not the case.

Quote from: TurfIt on February 07, 2021, 03:52:12 AMAnyway, looks quite good. I cleaned it up a bit - spaces for indentation, gah! Please don't do that again...

Agh, my bad. Everytime I switch text editors I forget to check if they are respecting my tabs (I've never used the space for indenting)... This time I learned the first thing I need to do is to enable showing tabs & spaces so this doesn't happen again (hopefully).

Quote from: TurfIt on February 07, 2021, 03:52:12 AMI'm tempted to leave the soundfont selection dialog available even if not compiled with fluidsynth support. Could display a message "You're missing this feature" to shame those who would produce a .exe without the ability for passable music!

That sounds a bit evil. But maybe...

Quote from: TurfIt on February 07, 2021, 03:52:12 AMIf the attached still looks good to you, then let's just commit it be done.

Sure, I see nothing wrong with it :-)

P.D.: Can I work next on the removal of sdl2_mixer or is anyone against it?

prissi

Why should SDL_mixer be removed. Maybe there are systems, where fluidsynth is not available but SDL_mixer is. It is enough to prefer fliudsynth for building and only rely on SDL_mixer, if including fluidsynth fails.