-
-
Notifications
You must be signed in to change notification settings - Fork 152
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
Implement FluidSynth integration #262
Comments
Well laid-out plan @dreamer ! 📑 |
Regarding part of point 4, I assume you mean using separate threads for midi synthesis. Fluidsynth already uses other threads internally, meaning that it doesn't burden the dosbox thread. In general, the patch seems simple enough to me that it can just be merged. Further cleanup can proceed later on as a specific task. Most importantly, it'll allow more users to switch to dosbox-staging, who will then be invested in the project's success. |
We need proof to confirm this assertion. It's not hard, just generating flamegraphs and some testing - but that's part of development work needed to push the patch through the finish line. Also, I am not going to merge patch in a state where CI does not verify the build using multiple compilers on multiple OSes. And that's the state the patch is in ATM :(. |
From the fluidsynth pages:
This is unlike munt, which has no notion of threading. |
Great point @bluddy , I agree. Regarding the flamegraphs @dreamer mentioned, are you able to generate them? I haven't had time to dig in yet; but if you can build and run fluidsynth (standalone) on a linux box, here are notes on how to generate them: https://github.com/brendangregg/FlameGraph/blob/master/README.md |
Without going into details, this is the way I generate flamegraphs (Linux only), before starting: the FlameGraph repo mentioned earlier needs to be cloned (for the scripts inside).
Resulting SVG to be opened in the browser (it has js to allow for easier browsing and filtering the graph). I need to describe somewhere how to interpret the graph, but overall: there will definitely be a huge plateau of unrecognized stacks (that's dynrec-generated CPU emulation, we're not interested in that) and a tower, that can be clearly recognized as main dosbox "Normal" loop; narrow towers on top of normal loop are ok - plateaus on top of normal loop are bottlenecks. (The thing I'm not sure about is how to be 100% sure flamegraph does not include child threads - but for CD-DA this doesn't seem to be a problem). If flamegraphs with FluidSynth integrated will look similar to graphs when game is playing music via CD-DA emulation - that's good. if fluidsynth will show up as plateaus covering ~5% of runtime stacks or more - that's bad and we'll need to investigate further. edit I guess it would be helpful to list the games, that offer music playback both via CD-DA and MIDI to start preparing test cases. I think some candidates might be: HoMM2, Settlers 2, and System Shock. |
Wow, this looks completely different than what I get on x86_64 - can you paste plain svg somewhere? |
Work on this feature started on branch |
A recent platform-specific timer adjustment for fluidsynth: joncampbell123/dosbox-x@e00cf22 |
These are only used when fluidsynth is doing audio output itself. They are audio driver parameters. When rendering audio into a buffer (with http://www.fluidsynth.org/api/index.html#UsingSynth This is how it's done in dosbox-core. The fluidsynth patch that's been floating around for a while now for vanilla dosbox does not do this, and thus there these parameters are important. |
Thanks for the comparison @realnc, and good to know these adjustments won't be needed. The approach of feeding dosbox's mixer with samples is win-win-win: fewer LOC to maintain, uses a single host-agnostic audio interface abstracted by SDL, and less runtime complexity and overhead. |
Initial, working version of FluidSynth integration can be tested on branch Testers: you need to compile it yourself. Our CI does not provide precompiled snapshots with FluidSynth integration (yet). FluidSynth 2.x is available in many distro repositories, but it's still missing from a few notable ones. Do not ask me for support as of yet - you're on your own. Do not get married to new fluid settings as inherited from dosbox-core - we will change them (not sure exactly how yet). I tested it on Ubuntu 20.04 and Windows 10 and it seemed to work fine, but code is not good enough quality-wise and we have no user documentation, so it won't be merged to master just yet. |
The first part of FluidSynth 2.x support was just merged via #539 :) But I'm not closing this feature request just yet - we need to polish it a little bit, add more documentation, implement some missing bits, we have 1 small bug… but as of now, dosbox-staging finally has a built-in MIDI synth. To testers: recreate your config file - there's a new I am especially interested in learning from testers using wide range of SoundFonts:
If you're compiling the code yourself, then FluidSynth support should work on any OS. |
i think this is from SVN
i use a Roland SC-55 soundfont, i cant remember where i got it from i dont have fluidsynth running all the time
that said, i dont start fluidsynth with dosbox-staging edit: |
some dos midi software useful for testing? http://dosmid.sourceforge.net/ also GSPLAY 1.0 untested but others report using: |
@kcgen The 8GB flash drive (ext4) also fails this test (probably due to slow writing). Regarding the configuration you proposed, I had to slightly modify it. The laptop, in addition to a weak HDD and CPU i5-450M, also has poor intel integrated graphics (Gen5) compatible only with OpenGL 2.1.
Here's a recording. To be sure, I repeated this test in various configurations on edit: I put autosave.gm1 and heroes2.cfg on tmps and made symbolic links. Even with that, it's not 100% correct. |
I completely forgot: fsynth wouldn't be the exception. Munt should be treated the same way. Or any other MIDI synth, like BASSMIDI (if you decide to add that at some point.) This makes sense even from a philosophical point of view. MIDI synths were always "asynchronous" devices back in the day. If you had an MT-32, a SoundCanvas, a Wave Blaster, or whatever else, the game would just send MIDI events over the MPU-401 port. The sound was rendered and played by those devices on their own. Even if the game went into an infinite loop, freezing completely, they would not stutter, hiccup, underrun, or anything like that. They would always play the sound cleanly, without any sound dropouts. Replicating that 100% async behavior makes sense. |
Maybe there are other files (besides the big
Other thoughts: Regarding slow CPU and GPU -- given the game plays flawless when moved to tmpfs, I think your CPU and GPU are fine. dosbox doesn't need much horse-power to hit 30000 cycles (my $30 raspberry pi plays HOMM2 flawlessly w/ MIDI at 30k cycles); so I think the problem is isolated to your system's memory and IO subsystems. |
@kcgen I will still be testing. edit: I built and tested the newest DOSBox-ECE-r4367 with fluidsynth-1.1.11. The sound is clear, smooth without any distortion. Same procedure with complete cleaning of the cache. I don't know if this will explain anything, logs from perf top (approximate). Fluidsynth in dosbox staging without interpolate, reverb, chorus, etc. There are many differences.
|
The configuration is the same.
Cache cleaned every time.
To make the difference visible, I increased the amplitude of the sound (-90dB). |
Hi @grapeli, ECE applies the FluidSynth patch as originally authored by @realnc, who explained that the patch makes FluidSynth write its generated audio out to the host OS using FluidSynth's own audio drivers for alsa / PulseAudio / CoreAudio (macOS) / Windows Audio. This audio output flow can happen asynchronously and in parallel to whatever dosbox is doing internally. This differs from dosbox-staging's integration of @realnc's patch, which modified FluidSynth's audio output pathway to follow the same path as all the other emulated audio devices in dosbox, which is: raw PCM samples -> dosbox's mixer -> SDL -> host audio interface. All of this is driven in a 1ms synchronous and serialized loop. Due to the lack of threading and output asynchronousity in the latter implementation, timing becomes absolutely critical. That is, all the PCM samples (from all the emulated audio sources) need to be computed on the spot in time to fill 1ms worth of buffer, then mixed, and then written to SDL.. all within 1ms. If there's any lag (including lag caused by other parts of the emulator not related to audio), there will be an audible gap in the audio. |
@kcgen I thought ECE applies an older version of the patch, still using (now being deprecated) FluidSynth 1.x? @grapeli Regarding |
Good point -- maybe ECE's patch is prior to @realnc's updates? I'm guessing it still using 1.x looking at configure.ac:AC_CHECK_HEADER(fluidsynth.h, have_fluidsynth_h=yes, have_fluidsynth_h=no, )
configure.ac:AC_MSG_CHECKING(for FluidSynth support)
configure.ac:if test x$have_fluidsynth_lib = xyes -a x$have_fluidsynth_h = xyes ; then
configure.ac: AC_DEFINE(C_FLUIDSYNTH,1)
configure.ac: LIBS="$LIBS -lfluidsynth"
configure.ac: AC_MSG_WARN([fluidsynth MIDI synthesis not available]) That said, the patch is definitely using (and requires) FluidSynth's own audio output driver as opposed to the channel -> mixer -> SDL pathway (as there is no channel callback or add_samples() call): adriver = new_fluid_audio_driver(settings, synth);
if (!adriver) {
LOG_MSG("MIDI:fluidsynth: Can't create audio driver");
delete_fluid_synth(synth);
delete_fluid_settings(settings);
return false;
} Regarding
Speed-wise, these two are effectively the same: the 6 MiB doesn't fit in most L2 caches so can take a hit paging it in while the unordered map might have to lookup up two values instead of one - but has very high odds of fitting entirely in L1 cache. But in either case, these lookup times are dwarfed by the deliberate delay inserted into the IO handler code to prevent it going too fast and likely causing getting divide-by-zeros and such in the DOS program: But there's another thing at play here: the unordered map is an STL template customized at compile time. When you build with profile instrumentation, then the map code itself becomes instrumented (which is great!) but at the same time, lookups become much slower due to that instrumentation. Where as the 6 MiB raw array isn't going to have that instrumentation. So that's going to act as "ballast weight" in the benchmark that disappears in a non-instrumented build. (and indeed, @dreamer and I both extensively benchmarked this change having the same concern and results were effectively identical). |
@kcgen @dreamer edit:
https://hg.libsdl.org/SDL/file/c6ed576e4ba5/src/audio/disk/SDL_diskaudio.h |
I checked dosbox-staging under Windows 10 (same hardware). Configuration similar, soundfont used the same (FluidR3_GM.sf2), cold cache, HoMM2. Then the x86-32 version under wine-5.19 under linux. The Linux version of dosbox-staging doesn't work (for me) properly with the built-in fluidsynth. I can rule out Alsa, I put it under Jack and it's the same. |
WTF, this is a PR build, not a build from our master branch… We explicitly filter development builds to include only those from master branch… WTF, GitHub REST API? (as for FluidSynth, functionally it's the same as 93c7d6f / current master branch)
@grapeli |
@dreamer I'm testing a master with fluidsynth built from contrib. I wrote what sound font I use, I will write again - FluidR3_GM.sf2. I have sent over 5 samples showing what kind of problems it has with the sound output. I can send another one. It is no different. I am using Archlinux. |
Jack is not listed as a supported audio driver on SDL Wiki - the list says "it's partial", but I don't know if it's partial in regards to platforms or available backends. @grapeli sorry, I haven't noticed you listed the soundfont and I thought you are reporting some new issue. Arch has Fluid 2.x, so you don't need to build from contrib - it's better to use the library as provided by your distro. |
@dreamer With fluidsynth-2.1.5-2 from the repository, the sound is also interrupted. |
What tool are you using tho create this? This is extremely insightful; thanks for sharing this @grapeli! Yet the build from Even though our Very interesting none the less. |
@kcgen https://ffmpeg.org/ffmpeg-filters.html#showspectrum-1 https://ffmpeg.org/ffmpeg-filters.html#showvolume
No Linux version with built-in fluidsynth (with contrib, with the repository, with fluidlite) will produce the correct sound (with a cold cache). All these audio interruptions (in my video) can be detected with ffprobe. You don't need to use audacity. or |
I tried to verify it again. Once again it turns out that my conclusions are too hasty. Sound is choppy in HoMM2 similar to Linux. It turns out that something else is heard on the headphones (speakers) and something else is obtained in the file with the captured sound (Ctrl-F6). I was fooled.
The repeated wine test also fails. |
@grapeli thanks for retest; I suggest you hold your horses a little bit until #640 will be in - that one improves the situation for me (so far tested with HoMM2 on Linux, using my old laptop). Another thing, that seemingly improves the situation (but it might be placebo, I did no measurements yet) was changing to Interestingly, I noticed that GOG release was toying with mixer prebuffer value for this game… And yeah - your experience with sound issues happening outside of dosbox-staging (only when testing with headphones but not when recording) are similar to mine. |
@grapeli, @dreamer - this is so subtle and tricky; but fantastic you are both reproducing it. Wish we could get to the bottom of what's sitting between our output stream to SDL (the recording) and you headphones. Maybe its time for |
Midi sound interruptions with dosbox-staging built-in fluidsynth occur: Never when the HoMM2 directory is mounted on tmpfs. I tested dosbox-staging in various variants. With static SDL2, with I use statically built SDL2 a lot. For example, a niche WM - notion, I use needs a patch to make SDL2 work properly in full screen. edit: A broken fullscreen in SDL2 was finally fixed five days ago (under notion, xmonad and probably other WM). |
With #734 finished, I think it's time to close this issue :) FluidSynth 2.x integration is not perfect yet, but it already provides the best out-of-the-box experience compared to any MIDI solution I tested with any DOSBox fork so far :) We picked good defaults, support soundfont lookup in standard directories, gain configuration that's easy to correlate to a specific font, have updated documentation to reflect the changes, and more. To enable FluidSynth MIDI, user needs to set: [midi]
mididevice = fluidsynth
mpu401 = intelligent
[fluidsynth]
soundfont = filename.sf2 # file will be looked in current working dir, then
# in 'soundfonts' dir in config dir, and a number
# of global directories that might store .sf2 files.
# alternatively:
soundfont = ~/my-soundfonts/foobar.sf2 # relative, absolute paths and tilde-expansion
# is supported
# alternatively:
soundfont = quiet-font.sf2 150 # gain is adjusted to 150%
soundfont = loud-font.sf2 30 # gain is reduced to 30% Empty The volume of FluidSynth MIDI channel ( DOSBox Staging will look for @kcgen, @grapeli, @realnc, @nemo93, @bluddy |
…recording with config. Add way to start keymapper from config as well. Imported-from: https://svn.code.sf.net/p/dosbox/code-0/dosbox/trunk@4416
The patch integrating FluidSynth was developed a long time ago (first revision 9 years ago, last revision 3 years ago - it really waits that long for inclusion upstream?). It is currently being distributed via ECE.
In the past, I was very hesitant to integrate FluidSynth patch for a number of reasons:
What changed?
Over last 1-2 months (?), point (5) stopped being a problem, fluidsynth 2.x is now available in most repositories, including brew and vcpkg. That takes out reason (5), and it's huge.
Points (4) and (2) are just a matter of putting in some work - not a big deal. I still want to address (1), but after looking in details at the patch - this work can largely happen in parallel. (3) can be assessed only during testing.
Of course, integration of Fluid needs to be optional - the lib is not old nor tested enough to be propagated to all repositories, so some users will want to build without it. But the patch already kind-of handles it.
When?
Testing effort required would push 0.75.0 release too far away, so I won't merge any new MIDI-related changes before 0.75.0 release (here are tasks planned ATM for 0.75.0).
But if someone has free cycles and wants to push this work forward: the patch is imported on a side branch already - cherry-pick f786b9f and start working on fixes to problems I listed in (2) and (4) ;) If nobody will step up, I'll start work on this sometime after 0.75.0.
The text was updated successfully, but these errors were encountered: