For Soundfonts bigger 2GiB, num_samples becomes negative. When being passed to safe_fread() it's promoted to long long and when being passed to fread(), it's cast to size_t. Works fine in twos-complement, but still is not nice.
Proposing a new event queue for the sequencer, based on prior discussion:
https://lists.nongnu.org/archive/html/fluid-dev/2019-12/msg00001.html
With this change fluidsynth will require a C++98 compliant compiler.
Consider this as RFC, feedback is welcome.
The "pain-points" from the discussion:
#### 1. It is slow.
Done (thanks to heap sort), see runtime of `test_seq_event_queue_sort`.
#### 2. A meaningful ordering for events with the same tick has not been considered.
Done, see comments in `fluid_seq_queue.cpp`.
#### 3. Complicated implementation
Now uses one single event queue, which requires C++98. Implemented similarly to std::priority_queue by using heap sort.
The "queue" I use currently is of type `std::deque`. `deque` does not provide preallocation. `std::vector` does provide it. However, `std::deque` has the huge advantage that appending additional elements is cheap. For `std::vector` appending new elements would require to reallocate all the memory and copy it to the new array. So,
* either use `std::deque` with the risk that memory allocation may occur during `fluid_sequencer_send_at()`, or
* use `std::vector` with a preallocated pool of events and make `fluid_sequencer_send_at()` when the `vector` runs out of capacity.
Comments?
#### 4. Events that have been processed are deleted and gone.
After having thought about this more, this is the correct behavior. After events have been dispatched, they must be released to free underlying memory, see point 3. For the very rare case that a client (e.g. fluid_player) may require those events in the future, the client should be responsible for storing the events somewhere.
#### 5. The sequencer supports the system timer as alternative time source.
The conclusion from the mailing list was that the system timer can be removed. This has been done.
#### 6. Time Scaling
Time scaling can now be used for arbitrary tempo changes. The previous implementation was capable of that as well, however, the time-scale was limited to 1000. The only limitation for the scale is now >0, see `test_seq_scale`.
### Other Points
* `fluid_sequencer_remove_events()` turned out to be broken before, as it did not remove all events from the queue. This has been fixed, see `test_seq_event_queue_remove`.
* Consider the following code executed by `threadA`:
```c
fluid_sequencer_send_at(event0);
fluid_sequencer_set_time_scale(); // new scale
fluid_sequencer_send_at(event1);
```
The new scale will be definitely applied to `event1`. However, if another concurrently running `threadB` executes `fluid_sequencer_process()`, it was previously not clear, whether the new scale was also applied to event0. This depends on whether `event0` was still in the `preQueue`, and this depends on `event0.time` and the tick count that `fluid_sequencer_process()` is being called with. This has been changed. As of now, events are queued with their timestamp AS-IS. And only the latest call to `fluid_sequencer_set_time_scale()` is being considered during `fluid_sequencer_process()`. This makes the implementation very simple, i.e. no events need to be changed and the sequencer doesn't have to be locked down. On the other hand, it means that `fluid_sequencer_set_time_scale()` can only be used for tempo changes when called from the sequencer callback. In other words, if `threadA` executes the code above followed by `fluid_sequencer_process()`, `event0` and `event1` will be executed with the same tempo, which is the latest scale provided to the seq. Is this acceptable? The old implementation had the same limitation. And when looking through the internet, I only find users who call `fluid_sequencer_set_time_scale()` from the sequencer callback. Still, this is a point I'm raising for discussion.
Since sizeof(long) == 4 even on 64 bit Windose, big files cannot be
loaded natively via the ANSI C file API. This change makes fluidsynth's
file callback API use long long, which is guaranteed to be at least 64
bit wide.
This PR allows the reverb to pre-allocate the memory needed for the maximum sample-rate of the synth. That means that on sample rate change there are no memory allocation and the function fluid_revmodel_samplerate_change() should always return FLUID_OK.
The PR addresses discussion in #608.
Stopping `dsound `before stopping `audio thread` avoid dsound playing transient glitches between the time audio task is stopped and dsound will be released. These short trailing glitches are particularly audible when captured by a reverb connected on output.
For some reason, the configure command must be specified explicitly in the `gentables` build step. Otherwise, the ARM target compiler will be used to build `make_tables` rather than the host compiler.
Now that microsoft/vcpkg#10485 has been completed, an ARM CI build can be added to AppVeyor. Also, the build status table in the README has been updated.
Supplying --verbose to the fluidsynth executable now prints debug messages to stdout. Debug messages are still being printed by default when fluidsynth was compiled in debug mode.
Responsibility for calling fluid_sequencer_unregister_client() in case of FLUID_SEQ_UNREGISTERING events has been moved to fluid_sequencer_send_now(). In other words, a FLUID_SEQ_UNREGISTERING event now really unregisters the client, no matter how the client's callback function looks like.
Avoids leaking the sequencer clients if implementations do not unregister them explicitly.
Also fixes another memory leak if fluid_sequencer_register_fluidsynth() clients were unregistered with fluid_sequencer_unregister_client() rather than by sending an unregistering event.
During the creation of a jack audio driver, it is checked whether the sample-rate of the settings object matches jack's rate. If not, it was adjusted previously via fluid_synth_set_sample_rate(). Due to the deprecation of that function and removal of real-time capability of the synth.sample-rate setting, a regression was introduced in 5fbddcecc3 causing the synth's sample-rate to be not updated.
This workaround obtains the synth via the settings instance and for now calls the deprecated sample-rate set function.
See discussion in #591 for details. Basically an incorrect size was
being allocated for the CoreAudio buffer list for a device. It was being
allocated by a VLA (which already did not quite fit the semantics of the
list) and the length calculated could be 0 (instead of the size of the
struct with no buffers elements) causing undefined behaviour.
This corrects it to allocate the amount of memory required by the
CoreAudio framework function and adds a check for the size retrieval and
for the dynamic allocation. This change passed UBSan in my test where
before the change it did not.
Fixes#591
This fixes a regression introduced in 907ec27a9e
When rendering a voice, there are 3 cases to consider: silent, playing,
and finished. When optimizing away the memset, I incorrectly assumed that
a voice cannot switch between playing and silence (like crazy) while
rendering FLUID_MIXER_MAX_BUFFERS_DEFAULT. Apparently this does not
hold true, esp. when rendering at sample rates ~96kHz.
This adds new LFO modulators:
- these modulators are computed on the fly, instead of using lfo lookup table. Advantages:
- Avoiding a lost of 608272 memory bytes when lfo speed is low (0.3Hz).
- Allows to diminish the lfo speed lower limit to 0.1Hz instead of 0.3Hz.
A speed of 0.1 is interesting for chorus. Using a lookup table for 0.1Hz
would require too much memory (1824816 bytes).
- Make use of first-order all-pass interpolator instead of bandlimited interpolation.
- Although lfo modulator is computed on the fly, cpu load is lower than using
lfo lookup table with bandlimited interpolator.
Also adds a stereo unit controlled by WIDTH macro. WIDTH [0..10] value define a stereo separation between left and right.
- When 0, the output is monophonic.
- When > 0 , the output is stereophonic.
WIDTH is currently fixed to maximum value to provide maximum stereo effect.
`GTimeVal` has been deprecated in glib 2.62 . While switching to `g_get_monotonic_time()`, I realized that we could simply reuse `fluid_utime()` for that purpose.
This provides a less branchy and therefore more instruction-cache-friendly version of fluid_ct2hz(), which also significantly reduces the number of floating-point comparisons.
- memset should clear all the memory
- end position should be at the end of sample data
Closes#576
Signed-off-by: Stefan Westerfeld <stefan@space.twc.de>
The synthesizer is using stdint types for a long time, perhaps for consistency, it would be worth to get rid of the GLIB macros for integer to pointer conversion and viceversa, and use just a type cast for that purpose.
The parameters (roomsize, level, etc.) of the reverb effects unit are initialized at the very end of `new_fluid_synth()` with `fluid_synth_set_reverb_full_LOCAL()`.
This however only adds an update-event to the `rvoice_mixer` queue.
The call to `fluid_synth_process_event_queue()` in `new_fluid_synth()` should make sure, that this event is dispatched and triggers the actual update.
However, the event is not dispatched immediately, because the rvoice event queue has not been committed yet, that is, a call to `fluid_rvoice_eventhandler_flush()` is missing.
So, although a reverb param initialization event has been queued, the reverb params still are garbage initialized (on Windows to some `-6.2774385622041925e+66`).
The next call to through the synth's public API will flush the queue and finally dispatch the update event, but when will it happen?
1. If the soundfont is specified as command-line argument, this call will happen before the audio driver starts rendering.
2. If the soundfont is loaded via shell command `load`, the audio driver will first start rendering audio, after updating the reverb.
Case 1. is trivial, everything works as it should.
Case 2. is interesting. Since the synth already started rendering audio by using that uninitialized reverb unit, the reverb engine's internal buffer is completely filled up with noise. Before outputting that signal to sound card, the sound is clipped to `1.0f`. That's the click we hear at the beginning. And because the reverb is so loud, the rendered audio signal stays 1.0f for quite a long time (...or always, can't tell).
Why is it not reproducible on Linux? Because GCC and Clang (AFAIK) leave all values uninitialized after allocating memory. And because malloc() often return zero-initialized memory, the reverb params seem to be nicely zero-initialized. MSVC however, always initializes memory with garbage, which is why we "hear" this resonance disaster.
Solution: Just update the reverb params via public API, which implicitly calls `fluid_rvoice_eventhandler_flush()`.
Fixes#563.
Fixes a use-after-free when the MIDI player is deleted before the audio
driver, because the synthesis thread is still actively making callbacks
on the sample timer, which is deleted by the player though.
Enabling the option supresses the default welcome message and
some other text output that normally gets printed to stdout.
It also slightly changes the way the welcome message and argument
errors are handled: in case of an argument error, the welcome message
is never printed.
Previously, sample timers were deleted in fluid_player_stop() which caused a use-after-free when at the same time the sample timers were advanced by the synthesizer thread. This was incorrectly addressed in 5d3f727547 . Deleting sample timers is now done in delete_fluid_player(). A broken application could still crash if it does not respect the order of object creation though. At least now, this issue is properly documented.
In an out of memory situation, fluid_synth_t::voice and fluid_synth_t::channel may not be fully initialized, causing a NULL dereference and heap corruption in delete_fluid_synth().
By default, libinstpatch silently adds all SF2 default modulators to the
converted DLS voices. Since fluidsynth respects the modulators provided
by libinstpatch, those modulators would conflict with the default
modulator list managed by fluidsynth. This is only noticeable, if the
user used fluidsynth's API to manipulate default modulators.
When compiled with compatibility for WinXP (toolset v141_xp), read() may return 0 (EOF) rather than '\n' which led to an early exit of the shell after a single user input.
Vorbis compressed SF3 samples are always loaded individually and stored
in the sample cache in uncompressed form. When dynamic-sample-loading is
not active (the default), then the uncompressed samples did not get
unloaded when unloading the SF3 font.
This fix makes sure that those samples are also freed. For bulk loaded
samples, the sample->data pointer is always the same as the
font->sampledata pointer. For individually loaded samples, the sample->data
pointer always points to a different memory region. So we can use that
information to determine if and when to unload the samples one by one.
Fixes#530Fixes#528
The old buffering code assumes that synth->cur is between 0 and FLUID_BUFSIZE.
However since fluid_synth_process() can render more than one buffer at a time
this isn't always true, and the new code handles other values properly.
Closes#527
This set of changes implements audio drivers for Android, OpenSLES and Oboe. The changes in the original sources are kept minimal so that it should be easily maintained.
When fluidsynth is run as a service using systemd, make sure
the service is considered started only when it is ready to process events.
In order to do so:
- Add an optional runtime dependency to libsystemd to the fluidsynth executable
- Change the systemd service type to "notify"
- Have fluidsynth notify systemd that the service is started after the server is started
- Have fluidsynth notify systemd that the service is stopping after joining the server thread
possibly due to glibc-2.29 commit 424c4f60ed6190e2ea0e72e0873bf3ebcbbf5448
pow is using fused-multiply-add (fma) if available.
That caused the fluidsynth binaries to differ depending on
the build machine's CPU.
This was visible among others in
grep '7.58577575029183[56]e-04' fluidsynth-2.0.3/build/fluid_conv_tables.c
See https://reproducible-builds.org/ for why this is matters.
Closes#512.
-Name of an instrument zone shouldn't prefixed by the preset zone
name.
This is due to the fact that instrument zone are common to all
preset zone. Consequently, an instrument zone cannot identfied
via a preset zone.
-This new change, simply prefixe a zone as this:
instrument zone name prefixed by iz:
preset zone name prefixed by pz:
Normalized value of MIDI sources (before unipolar or bipolar mapping) is always in the range [0..+1], regardless of MIDI event (even for FLUID_MOD_PITCHWHEEL).
Actually some basic modulators check are done at noteon time (in fluid_voice_add_mod()). That means that we know if a modulator (modx) is invalid only when a MIDI noteon is received and only for a preset modx belongs to. This is not appropriate.
This moves the modulator checking at soundfont loading time. Enhancements are:
1) A better verbose modulator integrity check, for any soundfont loaded at appropriate time.
1.1) All modulators are checked (preset zone (local/global), instrument zone (local/global).
1.2.1) Modulators check are enforced to source src1 and src2 (for non-CC and CC sources) (following SF specs (except for CC LSB) ( see comment in fluid_synth_cc_LOCAL()).
Modulators CC sources checking is coherent with the actual behaviour in fluid_synth_cc_LOCAL() in regard of modulation triggering.
1.2.2) Also, identical modulator in the same zone are detected.
1.2.3) Any invalid modulator(sources invalid, or modulator identical) is removed at loading time with a warning message displaying the cause and name of the modulators.
2) This fix a bug in noteon, in the case of identical modulators in global preset zone.
Assuming 2 identical modulator (m1 and m2) in a preset global zone, the actual noteon doesn't check this case (the actual code detect identical modulator in all others zones (instrument (local or global), preset(local)) but not preset global).
3) NoteOn is faster.
3.1)There is no more modulators checks at noteon making this more efficient.
3.2) As there are no identical modulator in the same zone, there is no more identity modulator check (i.e local zone against local zone), (i.e global zone against global zone). This result in a faster code and the bug described in (2) is gone.
4) Modulators sources checking as been added in API functions fluid_synth_add_default_mod() and fluid_voice_add_mod(). Please
- When primary source input (src1) is set to General Controller
'No Controller', output will be forced to 0.0 at synthesis time
(see fluid_mod_get_value()).
That means that the minimum value of the modulator will be always 0.0.
We need to force amount value to 0 to ensure a correct evaluation of the
minimum value later (see fluid_voice_get_lower_boundary_for_attenuation()).
- When secondary source input (src2) is set to General
Controller 'No Controller', output will be forced to +1.0 at synthesis time
(see fluid_mod_get_value()).
That means that this source will behave unipolar only. We need to force the
unipolar flags to ensure the modulator will behave correctly later in
fluid_voice_get_lower_boundary_for_attenuation().
This change removes the warning message if retrieving the soundfont file
modification time fails in the sample cache loader. The warning made sense
while we didn't have mtime support on all platforms, but after switching
to GLibs g_stat, it's no longer needed.
To reduce the number of calls to fluid_get_file_modification_time and also
get rid of a possible race condition, getting the mtime has been moved to
fluid_samplecache_load.
This change also fixes#483 because it removes the bogus warning messages
if a soundfont was loaded from memory by abusing the filename to store a
pointer.
- v variable is renamed min_val for clarity.
- This fix a bug when calculating the possible minimun
value (min_val) of a modulator. min_val should only
be dependant of:
- signs of mapped sources (ie bipolar [-1..+1] or unipolar [0..+1])
- sign of amount.
- absolute value of amount.
If we move UNKN_ID to the bottom of the enum, in the for() cycle inside chunkid() if the value is not found then the "i" variable will be already UNKN_ID.
I tried to check the generated ASM code when WITH_FLOAT macro is defined and when it is undefined. In both cases I noticed that there are some points where the math number are converted from float to double and viceversa. Actually this happens whether the FPU is present or not, perhaps for granting the right precision in digits. I also noticed few points that could be simplified a bit by using integer math, so I also included them.
* Since fluid_voice_get_actual_key() returns an integer value, the value 60 could be subtracted directly as integer, the code generated looks a bit better.
* FLUID_NOISE_FLOOR needs to be cast to fluid_real_t to generate a bit more efficient code.
* DC_OFFSET needs a cast to fluid_real_t to generate a bit more efficient code.
* "last_fres" and "output_rate" are already fluid_real_t, so they do not need type cast.
* "chan_add", "par1_add" and "par2_add" are integer types, so they can be added without FPU. "chan_mul", "par1_mul" and "par2_mul" are already fluid_real_t, so they do not need type cast. Instead, the constant 0.5 needs cast to fluid_real_t to be more efficient.
Libreadline is already used into fluid_istream_readline().
In my opinion, there is no need to have this duplicated code, so I would suggest to move the call to add_history() into fluid_sys.h
Closes#460.
Since 'data' is an integer value clipped between +/-8192 and 'nrpn_scale' is a char value, this calculation could be done with integers and then return the fluid_real_t value.
Addresses #455.
Instead of saving the name of the driver, it would be worth to save to pointer to the selected definition instead.
In this way, the function for deleting the driver does not need to search its name by parsing all the list, but it just needs to call the pointer into the saved definition (less code). This fix can be applied to MIDI drivers too.
I also moved the FLUID_FREE(allnames) inside the "if(allnames != NULL)" block, nothing bad should happen even by keeping that instruction outside, but actually there is no need to call the free if allnames is NULL.
