Skip to content

FluidSynth/Lite: Stuff to fix #2300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
5 of 7 tasks
Ghabry opened this issue Aug 24, 2020 · 27 comments
Closed
5 of 7 tasks

FluidSynth/Lite: Stuff to fix #2300

Ghabry opened this issue Aug 24, 2020 · 27 comments
Milestone

Comments

@Ghabry
Copy link
Member

Ghabry commented Aug 24, 2020

Audio issues:

  • Investigate wrong CC command (Drums) Backport a fix?
  • Stuck instruments when BGM_Stop -> BGM_Play or Exit Game -> Game browser -> Start game
  • Further performance investigations

Better integration:

Other:

  • Buildfix for Fluidsynth v1.1.11
@fdelapena
Copy link
Contributor

fdelapena commented Aug 24, 2020

Additional system soundfont search paths:

Fedora/CentOS/RHEL fluidsynth has a default soundfont concept, if there is a different soundfont not being Fluid R3:

/usr/share/soundfonts/default.sf2

Arch (soundfont-fluid), Fedora/CentOS/RHEL (fluid-soundfont-gm):

/usr/share/soundfonts/FluidR3_GM.sf2

Debian/Ubuntu, openSUSE/SLED/SLES (fluid-soundfont-gm):

/usr/share/sounds/sf2/FluidR3_GM.sf2

@mateofio
Copy link
Contributor

mateofio commented Aug 25, 2020

I just tried this by playing hh3 with a debug build in Windows 10 using fluidlite. I used scc1t2.sf2 as easyrpg.soundfont

I played a bit of the intro, then went to "end game" from the menu. Now Player is hung with a black screen upon returning to the title.

The intro is playing a midi, the title screen mp3.

 	[External Code]	
>	Player.exe!WASAPI_WaitDevice(SDL_AudioDevice * this) Line 316	C
 	Player.exe!SDL_RunAudio(void * devicep) Line 758	C
 	Player.exe!SDL_RunThread(void * data) Line 289	C
 	[Inline Frame] Player.exe!RunThread(void *) Line 90	C
 	Player.exe!RunThreadViaCreateThread(void * data) Line 99	C
 	[External Code]	

Looks like some kind of deadlock in the audio subsystem.

Unfortunately this isn't reproducable :(

@mateofio
Copy link
Contributor

Another issue,

When I play my midis in foobar2000, using BASSMIDI and scc1t2.sf2 they sound fine.

When I play with Player using fluidlite and the same soundfont, I hear a faint crackling sound, like you would from an vinyl record player.

@mateofio
Copy link
Contributor

mateofio commented Aug 25, 2020

This midi clearly shows the percussion issue in fluidlite:

https://github.com/fmatthew5876/hh3-rm2k/blob/master/Music/HH3%20Castle%20of%20Life.mid

The drums are replaced by some other low sounding instrument.

It also displays a high amount of the crackle effect described previously.

@fdelapena
Copy link
Contributor

fdelapena commented Aug 25, 2020

When I play with Player using fluidlite and the same soundfont, I hear a faint crackling sound, like you would from an vinyl record player.

Same here with stock FluidSynth 2.1.1 in Player (fails to build with 1.11.1 because uses 2.x API). It was happening already with the previous interpolation mode (7). @elsemieni's uploaded rendered tests were also affected.
I've tried the FluidSynth 2.1.1 CLI tool with same args (gain 0.6, sampling rate 44100, chorus off, reverb off) and it plays without noise. Same with 1.11.1 CLI.

Other stuff I've found:

  • Apart of the stop/play not stopping notes, after the loopstart, some midis with active notes get stuck after returning to the loopstart position. Not sure how RPG_RT handles the note length, if it respects them or they makes an immediate note off for all of them. I didn't check if @tyrone-sudeium's MIDI: native midiout system #2302 improved this.

  • The jumpy notes issue mentioned by @elsemieni still happens when the freq is not 44100 (the pitch fix didn't have effect on this) and it was happening already on Emscripten with fmmidi (same sequencer) when the host uses 48000 Hz.

By the way, there's a synth.midi-bank-select option to select how bank select commands affect percussion, but not sure if allows to workaround the fluidlite bug.

A note on a possible optimization for some low memory ports (but with decent I/O maybe), synth.dynamic-sample-loading might help.

@Ghabry
Copy link
Member Author

Ghabry commented Aug 25, 2020

  • make it build with fluidsynth 1.1 and test if there are the same drum bugs

@Ghabry
Copy link
Member Author

Ghabry commented Aug 25, 2020

When I compare it with fluidsynth or fluidsynth-sans-glib (yeah this one works) then the samples generated by FluidLite sound really bland. With fluidsynth(-sans-glib) I feel the intensity and the bass.

So for platforms that have std::thread (unused in our case but is a dependency) sans-glib can be used.

  • Fluidsynth: Windows (needs patch!!! pthread is hardcoded!!!), Linux, macOS, Android, Switch
  • FluidLite: 3DS, Wii, Vita, emscripten

Except if somebody wants to provide a patchset for the patchset that removes std::mutex and std::thread :P

@Ghabry
Copy link
Member Author

Ghabry commented Aug 25, 2020

BUG: This code

https://github.com/EasyRPG/Player/blob/master/src/audio_sdl_mixer.cpp#L430-L436

Is now handled by EP_MIDI_FREQ

@Ghabry
Copy link
Member Author

Ghabry commented Aug 25, 2020

When deleting these lines:

  • FluidLite sounds less dull (but still worse)
  • The audio glitches are gone (problem with our resampler? Though we never had such problems with other audio samples before . Otherwise you would hear audio glitches in almost all BGMs.)

@Ghabry
Copy link
Member Author

Ghabry commented Aug 25, 2020

Audio recording of both:
midi.zip

@fdelapena
Copy link
Contributor

fdelapena commented Aug 26, 2020

I feel both sound fine now without the noise or degraded quality 👍.

Just two things:

  • The first has broken percussion yet (recorded without applying the patch?).
  • It's fairly hearable the jumpy uncoordinated notes issue here (on both, it's likely a midisequencer issue, it affected fmmidi too).

@Ghabry
Copy link
Member Author

Ghabry commented Aug 26, 2020

Is the "percussion" the drums? Guess I have to delete the channel to figure this out ;).

This is what I meant with "it sounds dull/worse". The "power" and the bass are missing

@Ghabry
Copy link
Member Author

Ghabry commented Aug 26, 2020

I found the bug in FluidLite. The Drums sound now beautiful. Basicly no difference to FluidSynth imo 👍

Wait for my patch ;)

@fdelapena
Copy link
Contributor

The jumpy uncoordinated notes issue has been fixed at #2478.

@Ghabry
Copy link
Member Author

Ghabry commented Mar 16, 2021

My fix also fixed FmMidi btw, because they all share the same code. :)

@fdelapena
Copy link
Contributor

(...)
So for platforms that have std::thread (unused in our case but is a dependency) sans-glib can be used.
(...)
Except if somebody wants to provide a patchset for the patchset that removes std::mutex and std::thread :P

Not verified, but threading seems to be an option in CMakeLists.txt for this one.

@fdelapena
Copy link
Contributor

solidsynth by @sykhro, used in the vgmtrans project is a freshly maintained patch for upstream FluidSynth without glib, includes MSVC support. Latest update is 12h since writing this, rebased to FluidSynth 2.1.8

Though the most recent commit seems to have issues with the generated patch:

https://github.com/vgmtrans/solidsynth

FluidSynth 2.2.0 is coming soon and introducing C++98 compiler requirement, so it is expected the patch going at least a couple of lines smaller.

@sykhro
Copy link

sykhro commented Mar 20, 2021

solidsynth by @sykhro, used in the vgmtrans project is a freshly maintained patch for upstream FluidSynth without glib, includes MSVC support. Latest update is 12h since writing this, rebased to FluidSynth 2.1.8

Though the most recent commit seems to have issues with the generated patch:

https://github.com/vgmtrans/solidsynth

FluidSynth 2.2.0 is coming soon and introducing C++98 compiler requirement, so it is expected the patch going at least a couple of lines smaller.

Oh whoops, failed to generate the patch and I didn't even notice, will fix soon.
The patch set passed all tests but hasn't been tested much in production yet, btw

@fdelapena
Copy link
Contributor

Thanks sykhro, no rush with this. And FluidSynth 2.2.0 has been released as expected.

@sykhro
Copy link

sykhro commented Apr 2, 2021

No worries; actually, thanks for the info, I'll try to rebase on top of 2.2.0 but I don't expect it to be very smooth

@Ghabry
Copy link
Member Author

Ghabry commented Jul 16, 2021

For us the solidsynth patchset is likely overkill.
We could survive with a patch that completely patches out all the thread-safety stuff, which is 90% of the glib-code, simply because we do not need thread safety at all.

@Ghabry
Copy link
Member Author

Ghabry commented Jul 16, 2021

I created now my own patch for fluidsynth. It uses a glib.h as a shim, this makes it usable with much less patching.

https://github.com/FluidSynth/fluidsynth/compare/master...Ghabry:no-glib?expand=1

Though the patch is far worse than from solidsynth. It has zero thread synchronization (we do not need this) and non-VIO based APIs are likely broken because I did not reimplement stat.

But it plays MIDIs for me 👍

@Ghabry
Copy link
Member Author

Ghabry commented Jul 17, 2021

copied the alloca patch from solidsynth over to fix the Windows build (MSVC does not support variable length arrays, they are a GNU extension): Fluidsynth sounds much better than Fluidlite. Exactly like using the Windows synth directly.

So I would say Fluidlite is good enough for the non-major platforms (smaller) but for the major platforms I vote to use fluidsynth with my patch, works great :)

@Ghabry
Copy link
Member Author

Ghabry commented Jul 17, 2021

Here precompiled exe for comparison:

  • Player.exe: Compiled with Fluidsynth
  • Player_fluidlite.exe: Compiled with Fluidlite (crashes on shutdown)

Player.zip

@Ghabry
Copy link
Member Author

Ghabry commented Jul 17, 2021

And here for comparison some audio. In the later half of the track you will notice a difference of Fluidlite vs. the others.

audio.zip

@fdelapena
Copy link
Contributor

fdelapena commented Jul 18, 2021

And here for comparison some audio. In the later half of the track you will notice a difference of Fluidlite vs. the others.

Cool, as expected it sounds better with FluidSynth because Windows native MIDI seems to use 22 KHz internally 👍.

Because MIDI synth is performance hungry, all non-desktop ports could use 22 KHz for synth (fluidsynth and fmmidi), even if they get doubled later (resampled) for mixing, because this operation is cheaper than rendering.

Ghabry added a commit to Ghabry/easyrpg-player that referenced this issue Feb 18, 2022
Search order is now (in Game directory and Root fs):
1. Command line (--soundfont)
2. Environment (SDL_SOUNDFONTS)
3. easyrpg.soundfont
4. More hardcoded filenames

Related EasyRPG#2300
@Ghabry Ghabry added this to the 0.7.1 milestone May 29, 2022
@Ghabry
Copy link
Member Author

Ghabry commented May 29, 2022

All major issues here are resolved.

Until we decide to bundle something with Jenkins (which is not a Player bug) it will be "Bring your own Soundfont" :)

@Ghabry Ghabry closed this as completed May 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants