Skip to content
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

Don't stop playing until zero voices are active and reverb has a chance to fade out #1159

Merged
merged 1 commit into from
Sep 24, 2022

Conversation

topaz
Copy link
Contributor

@topaz topaz commented Sep 20, 2022

In an attempt to alleviate 12-year-old #24 without a deep understanding of FluidSynth internals, this PR adds a player.extend-playback setting (and corresponding -x / --extend-playback CLI option) which takes a number of milliseconds to wait after the last MIDI event before stopping playback. This PR makes no attempt to solve any of the more complex problems like automatic silence detection; -x 2000 is more than enough for my use cases, and so I hope this change is useful to others as well.

Reviewers beware: I'm not 100% fluent in C, knew almost nothing about FluidSynth internals a week ago, and have very likely caused a mess of things, so please be careful when reviewing this PR. In particular, I'm not sure whether claiming -x was appropriate, whether I've filled in documentation in all the right places/ways, whether the added logic is even in the right place, whether I've caused weird behavior in edge cases involving stopping/restarting/reloading a player or playing multiple files, or how to write tests for any of those situations.

Copy link
Member

@derselbst derselbst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all: Thanks for looking into this.

However, this approach is unsuitable. Adding an additional setting and extra commandline argument is an absolute overkill for this problem and most importantly doesn't solve it.

The better solution would be to keep rendering audio until fluid_synth_get_active_voice_count() returns zero. Additionally, a static number of seconds could be added after voice count dropped to zero to allow reverb to complete. Two seconds should be enough.

@topaz
Copy link
Contributor Author

topaz commented Sep 21, 2022

I think I've made the changes you requested. For my use cases, waiting for zero active voices plus two seconds for reverb leaves almost 2 seconds of silence; when reverb is turned way up, 2 seconds isn't nearly enough and the audio still cuts out. I'm not sure how to handle it better beyond putting back a setting that lets the user choose the amount of extra time.

@topaz topaz changed the title Add support for player.extend-playback to avoid truncating end of audio Don't stop playing until zero voices are active and reverb has a chance to fade out Sep 21, 2022
@topaz topaz requested a review from derselbst September 21, 2022 13:08
Copy link
Member

@derselbst derselbst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that looks better already, thanks!

Regarding the reverb / grace time issue, you could try whether a simple weighting of the hardcoded value would give better results, like so:

fluid_synth_get_reverb_roomsize(player->synth) * fluid_synth_get_reverb_level(player->synth) * FLUID_PLAYER_STOP_GRACE_MS

roomsize and level will in range of about [0;1], so the grace time would represent the maximum value in this case (probably you need to increase it then, 2 sec was just a wild guess).

I think having absolutely no silence in between is not possible that way, but I don't consider it to be a use-case either. Also, I would question whether we should respect the case of "turning reverb all the way up". Just having a reasonably well working default behavior would be nice. Perhaps you could have a try.

In case it turns out to be not that simple, I would like to have a try myself. In this case pls. give me some more information of how you test this, e.g. which reverb settings, which soundfont and possibly also with MIDI files you use.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

5.0% 5.0% Coverage
0.0% 0.0% Duplication

@topaz
Copy link
Contributor Author

topaz commented Sep 22, 2022

I'm happy to do the modeling, at least initially.

In the interest of maybe doing the more complete solution after all, is there somewhere we can currently (or add a feature to) query the volume of the audio currently coming out of the synth? It seems like we could just wait until it drops below some threshold. I don't know enough about audio synthesis to even know whether that's a sane question to ask.

@mmontag
Copy link

mmontag commented Sep 22, 2022

I think this would be a nice addition to fluidsynth.

Just for fun...I have my own hacky solution to the general problem:
https://github.com/mmontag/chip-player-js/blob/master/src/players/MIDIFilePlayer.js#L236
It just waits for samples to stay below value X for a duration Y, but it helps a lot with the whole "gapless playback" experience.

@topaz
Copy link
Contributor Author

topaz commented Sep 22, 2022

I've run a few tests measuring the timestamp of fluid_synth_get_active_voice_count() == 0 and the last timestamp containing significant audio using values of synth.reverb.level in 0, 0.1, 0.2, ..., 1 and synth.reverb.room-size in 0, 0.1, 0.2, ..., 1. In general, it seems like audio ends before fluid_synth_get_active_voice_count() == 0 unless reverb is turned way up.

Using a MIDI file that just hits a bunch of notes on a bunch of instruments all at once and then immediately stops:

  • With the FluidSynth-provided VintageDreamsWaves-v2.sf2, audio extends past zero active voices only when synth.reverb.level >= 0.7 and synth.reverb.room-size == 1.
  • With this SoundFont, audio extends past zero active voices around when synth.reverb.room-size >= 0.8 or when synth.reverb.room-size >= 0.5 and synth.reverb.level >= 0.7.

Using a real song I've been testing with, audio extends past zero active voices only around when synth.reverb.room-size >= 0.9.

Except in cases when reverb is turned way up, the ideal value for FLUID_PLAYER_STOP_GRACE_MS is actually negative - that is, audio stops before the active voice count is zero.

To try to come up with a method to handle cases where the ideal value for FLUID_PLAYER_STOP_GRACE_MS is positive, I did some modeling of the difference between the timestamp when fluid_synth_get_active_voice_count() == 0 and the last timestamp when significant audio is produced. I found this model to be a decent fit for the worst-case dataset from above; it recommends a FLUID_PLAYER_STOP_GRACE_MS of 1000 * 7.51577 * synth.reverb.level ^ 0.326144 * synth.reverb.room-size ^ 6.6097 - 0.381694. (This produces negative values, which predict audio ended some time in the past and should be treated as zeroes in practice.) However, since my more realistic SoundFont+MIDI had a very worst case (synth.reverb.level = synth.reverb.room-size = 1) of FLUID_PLAYER_STOP_GRACE_MS=3.5 seconds, this model overestimates the added time by about four seconds; using a model calibrated to this use case would instead underestimate the added time in other situations.

I measured the various timestamp-of-last-audio values using a script I wrote called measure-reverb.pl; if you want to try it, you'll need to build FluidSynth using this commit but add printf("no voices active at msec=%d\n", msec); after src/midi/fluid_midi.c line 2189 and then set FLUID_PLAYER_STOP_GRACE_MS to some very large value like 30000 in src/midi/fluid_midi.h. It tries to detect silence by looking through the raw 16-bit signed PCM data for 100-frame chunks with a RMS of < 16, which seemed like roughly when Audacity stopped even rendering the waveform. (This method was constructed by cobbling together a vast assortment of Google searches and my own limited knowledge of audio stuff, and so I have no idea whether it is actually a sane approach.)

So, it doesn't seem like estimating FLUID_PLAYER_STOP_GRACE_MS using just the values of synth.reverb.room-size and synth.reverb.level is reasonable. It seems like the best plan is to either:

  • use something like the silence-detecting technique I used in measure-reverb.pl to give the synth object the ability to report whether its last chunk contained any significant audio
  • decide that high reverb is not a common enough use case to handle and rely on just fluid_synth_get_active_voice_count() == 0 to handle most situations

Copy link
Member

@derselbst derselbst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your investigations! Since you're happy with this initial approach, I'm fine as well. To take care of this "silence detection" part, I've opened #1165. Your input is welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants