SF2 chorus and reverb bug

[mikobuntu]

Edit @tresf 2014-10-27 (Added steps to reproduce)

Steps to reproduce

1. Download lmms-test.sf2, a soundfont with chorus and reverb support.
2. Create a new project, add a new SF2 Player instrument to Song Editor
3. Open SF2 Player dialog. Browse to lmms-test.sf2 (pink folder icon)
4. Add notes to Song Editor piano roll for SF2 Player instrument. Play sounds using the play button. Notice no reverb. No chorus.
5. File, Export
6. Play sounds using play button. Notice reverb and chorus. They can only be disabled by clicking and unclicking appropriate buttons in the SF2 player.

Rendering a project to an audio file will cause the soundfonts chorus and reverb to be applied without any user intervention concerning the gui. Also if i drag a soundfont which has chorus and reverb settings, they will be applied immediately without needing to use the sf2 player buttons.however if i use the instruments internal browser and load the same preset the bug does not occur.
On render the actual .wav file also has the effects applied too.
before I even drag & drop of sf2, the preset preview in the file browser also applies the chorus and reverb

thanks Mikobuntu

[tresf]

So are you saying chorus and reverb buttons are incorrectly enabled by default on SF2-file drag and on render? Related #1233

[mikobuntu]

Not the actual buttons but the effects themselves are applied. You will need a soundfont with settings for chorus and reverb to notice the bug, as not all soundfonts have this.

test with this sf2 that i created : https://drive.google.com/file/d/0B780anJGotTaLS14anR5SUVIMHM/view?usp=sharing
thanks Mikobuntu ;)

#1238

So are you saying chorus and reverb buttons are incorrectly enabled by default on SF2-file drag and on render?

—

                  =

[tresf]

Ok.. the term "instrument effects" is a bit confusing in the title since all instruments can have effect chains stacked on top. :) Can we reproduce this on older builds? I'd like to know if it's newly introduced with 1.1 or not so I know how to categorize it.

[tresf]

I looked into this for a while and couldn't find anything obvious.

@diizy, can you help me understand the BoolModel constructor? I noticed that some instruments take a 4th parameter and SF2 does not for some of these values that are causing issues. The 4th value is labeled as defaultConstructed in AutomatableModel.h#L381 but I'm not entirely sure if it has any bearing on this bug (chorus on during render or file-drag when it should be off)

https://github.com/LMMS/lmms/blob/stable-1.1/plugins/sf2_player/sf2_player.cpp#L99

[mikobuntu]

#Ok.. the term "instrument effects" is a bit confusing in the title since all instruments can have effect chains stacked on top. :) Can we reproduce this on older builds? I'd like to know if it's newly introduced with 1.1 or not so I know how to categorize it.#
I will change the title to reflect this ;) and i have a few old builds of LMMS on my machine so i will get testing now and report back.....

thanks Mikobuntu ;)

                  =

[mikobuntu]

@tresf Ok i tested with lmms-0.4-14-rc1 and the bug existed there too, so this is not newly introduced.
( perhaps git bisect would be useful here to find the bad commit ? )

• i am testing with git bisect now, i will try to get to the cause of this, .... here's looking for " git blame " ;)

---

Git Bisect is a nightmare to use with LMMS branches, I think the upstream changes to ZynAddsubFX make it almost impossible to build LMMS on older branches that i need to test against.

---

.

[diizy]

On 10/25/2014 05:08 AM, Tres Finocchiaro wrote:

I looked into this for a while and couldn't find anything obvious.

@diizy https://github.com/diizy, can you help me understand the
|BoolModel| constructor? I noticed that some instruments take a 4th
parameter and SF2 does not for some of these values that are causing
issues.

The 4th value is labeled as |defaultConstructed| in
AutomatableModel.h#L381
https://github.com/LMMS/lmms/blob/stable-1.1/include/AutomatableModel.h#L381
but I'm not entirely sure if it has any bearing on this bug (chorus on
during render or file-drag when it should be off)

Well, the parameter is optional, since it has a default value in the
definition. In this case, it defaults to false if the parameter is not
given. As you can see from AutomatableModel.h line 416.

Try changing it to true and see if that does anything?

[tresf]

Some rather interesting findings.... I first flagged that BoolModel to use false as it's 4th and optional parameter (since that's the intended default value) and no luck, chorus still enabled by default (which I expected, since it defaults to false anyway).

But then I tried as suggested... forced true as the 4th and optional parameter, and the plugin crashes when clicking the close button... backtrace is here:

https://gist.github.com/tresf/d37c65fbe5115217a8d5

Any thoughts?

Edit
Furthermore, there seems to have been a release bump as well as some code code changes (new code not yet released under a new version number) since our version (Ubuntu 12.04 comes with 1.1.5)

Here's the recent commits:
http://sourceforge.net/p/fluidsynth/code-git/ci/4bbc44d22b702d573e0790b2397c10f890a26c41/log/?path=/fluidsynth

[tresf]

I'm starting to believe this is related to #649. It is very obvious to me that fluidsynth is not being initialized correctly. From what I can gather, it might have to do with the way it is initialized/cast (supported by the fact that the instrument plays fine in GUI, but experiences the bugs/console errors on render).

Edit: Also, if I were to speculatively trace this bug back, I would associate it with the initial implementation of the chorus/reverb 6 years ago here: 564ed10

[diizy]

On 10/28/2014 03:15 AM, Tres Finocchiaro wrote:

Some rather interesting findings.... I first flagged that BoolModel to
use |false| as it's 4th and optional parameter (since that's the
intended default value) and no luck, chorus still enabled by default.

Right, that didn't change anything - omiting the parameter is the same
thing as providing "false".

But then I tried as suggested... forced |true| as the 4th and optional
parameter, and the plugin crashes when clicking the close button...
backtrace is here:

Then it probably needs to be false...

I've really no idea what that parameter is for (wouldn't it be cool if
we had documented APIs...)

[tresf]

I'm fairly certain these are at least somewhat related to fluidsynth bugs. Our version is internally reported as 1.1.5 (2011) in Ubuntu 12.04. Version 1.1.6 (2012) has been released since as well as some additional patches in the past two years.

Ubuntu 14.04 claims to be bundled with 1.1.6-2 so perhaps it is time to update my build environments?

[tresf]

In addition, I went over some working plugins which don't muck up their settings and compared the code... I can't find anything that stands out. I notice there are places in vestige where emit dataChanged() is fired (such as file load) which doesn't seem to be as straight forward in sf2_player, but adding it made no difference with this bug. Even if I hard coded in the reverb and chorus settings, I couldn't get one good render using the export. Every. time. export. is. used. the. chorus. is. enabled. </baffled>

[mikobuntu]

@tresf I'm running ubuntu-12.04 too, I downloaded and installed a snapshot of the latest master branch of Fluidsynth http://sourceforge.net/p/fluidsynth/code-git/ci/master/tree/ (hint you will have to use cmake to build,) still the bug remains...... It looks like this is STILL OUR BUG..

[diizy]

On 10/28/2014 07:22 AM, Tres Finocchiaro wrote:

In addition, I went over some working plugins which don't muck up
their settings and I can't find anything that stands out. I notice
there are places in vestige where |emit dataChanged()| is fired (such
as file load)

All the dataChanged signal does is inform LMMS to set the dirty flag,
ie. that the project has been modified. Shouldn't affect plugin/preset
data at all.

[tresf]

I downloaded and installed a snapshot of the latest master branch of Fluidsynth [...] still the bug remains

Thanks for testing this, greatly appreciated.

dataChanged signal [...] inform sets the dirty flag, ie. that the project has been modified.

Ah, that makes much more sense. What I can't wrap my head around is how this reverb and chorus is ever getting enabled to begin with. From what I can see, the BoolModel is a GUI-less object model so that rendering can happen without the graphical objects be instantiated. (and then the graphical objects are attached to this model linking the data as well as having a storage place for our XML data). So at some point, it's like the BoolModel(s) are getting completely ignored. I was hoping to find a typo or a missing constructor somewhere, but the code looks much like the rest of the plugins. Since Apple sporadically crashes with the constructor, I would like to think I'm missing something obvious here (such as a mutex not being locked properly).

I went as far as to look at the fluidsynth source code as well to see if there is a boolean flag getting triggered when the reverb and chorus settings are applied (I haven't ruled this out) but when I comment out the respective setters, this bug still occurs, so I'm at a total loss at this point. This shouldn't be so hard to find!!! 😢 😭 😃

[diizy]

On 10/28/2014 02:28 PM, Tres Finocchiaro wrote:

I downloaded and installed a snapshot of the latest master branch
of Fluidsynth [...] still the bug remains

Thanks for testing this, greatly appreciated.

dataChanged signal [...] inform sets the dirty flag, ie. that the
project has been modified.

Ah, that makes much more sense. What I can't wrap my head around is
how this reverb and chorus is ever getting enabled to begin with.

Ok, I think I maybe see what's going on here... I think what happens is
that the functions for setting the on/off state of reverb/chorus aren't
getting called when the fluidsynth instance is initialized without GUI,
because those functions are called from slots that are triggered only
when the state of the reverb/chorus buttons change.

I'm going to try a quick fix...

[diizy]

@tresf try it now on stable-1.1 240c0b2

[mikobuntu]

@diizy so far so good , the initial test of dragging my test.sf2 preset does not have the effects applied, will test song render now ....

[mikobuntu]

@diizy great Confirmed fixed ;)

[tresf]

How did I miss that?

Confirmed fixed.

[rubiefawn]

I'm glad this is being fixed. Reverb and Chorus in the SF2 player have
never worked at all for me.

On Tue, Oct 28, 2014 at 7:51 AM, Tres Finocchiaro notifications@github.com
wrote:

How did I miss that?

Confirmed fixed.

[image: image]
https://cloud.githubusercontent.com/assets/6345473/4809224/8a2c2b6c-5ea9-11e4-907e-68b5bc4dc638.png

—
Reply to this email directly or view it on GitHub
#1238 (comment).

[diizy]

On 10/28/2014 07:02 PM, Ian Sannar wrote:

I'm glad this is being fixed. Reverb and Chorus in the SF2 player have
never worked at all for me.

You need to use soundfonts that support those features.

[rubiefawn]

I have been. Piano soundfonts, mostly.

On Tue, Oct 28, 2014 at 11:03 AM, Vesa V notifications@github.com wrote:

On 10/28/2014 07:02 PM, Ian Sannar wrote:

I'm glad this is being fixed. Reverb and Chorus in the SF2 player have
never worked at all for me.

You need to use soundfonts that support those features.

—
Reply to this email directly or view it on GitHub
#1238 (comment).

[rubiefawn]

The reverb worked, (only reverb was supported) but the knobs made no
difference.
Isn't that what's being fixed?

On Tue, Oct 28, 2014 at 11:05 AM, Ian Sannar ian.sannar@gmail.com wrote:

I have been. Piano soundfonts, mostly.

On Tue, Oct 28, 2014 at 11:03 AM, Vesa V notifications@github.com wrote:

On 10/28/2014 07:02 PM, Ian Sannar wrote:

I'm glad this is being fixed. Reverb and Chorus in the SF2 player have
never worked at all for me.

You need to use soundfonts that support those features.

—
Reply to this email directly or view it on GitHub
#1238 (comment).

[diizy]

On 10/28/2014 07:05 PM, Ian Sannar wrote:

I have been. Piano soundfonts, mostly.

Well if you have a soundfont that supports reverb/chorus, then they
already work just fine. This bug was just about reverb/chorus getting
enabled when they shouldn't be.

[rubiefawn]

Maybe I have soundfonts with cancer, lol.

On Tue, Oct 28, 2014 at 11:08 AM, Vesa V notifications@github.com wrote:

On 10/28/2014 07:05 PM, Ian Sannar wrote:

I have been. Piano soundfonts, mostly.

Well if you have a soundfont that supports reverb/chorus, then they
already work just fine. This bug was just about reverb/chorus getting
enabled when they shouldn't be.

—
Reply to this email directly or view it on GitHub
#1238 (comment).

[diizy]

Make sure you have reverb/chorus enabled.