-
Notifications
You must be signed in to change notification settings - Fork 259
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
Conversation
There was a problem hiding this 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.
24a467b
to
d6e64e9
Compare
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. |
There was a problem hiding this 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.
Kudos, SonarCloud Quality Gate passed! |
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. |
I think this would be a nice addition to fluidsynth. Just for fun...I have my own hacky solution to the general problem: |
I've run a few tests measuring the timestamp of Using a MIDI file that just hits a bunch of notes on a bunch of instruments all at once and then immediately stops:
Using a real song I've been testing with, audio extends past zero active voices only around when Except in cases when reverb is turned way up, the ideal value for To try to come up with a method to handle cases where the ideal value for 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 So, it doesn't seem like estimating
|
There was a problem hiding this 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!
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.