-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #2047 ("TripleOscillator: Oscillators are getting out of sync") #3145
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
Fix #2047 ("TripleOscillator: Oscillators are getting out of sync") #3145
Conversation
Change all phase related variables in Oscillator from float to double. The parameters for the getSample methods have not been changed to double as it should suffice to only update the phase variables with double precision.
|
Hey you're back 👋 |
|
Hi @Umcaruje! Yes, I was absent for quite some time. I am now mainly working on my own small project in my spare time but this one intrigued me. :) |
|
Is there something else to do for this, or can we merge it? |
|
As discussed in #2047 we might consider to add some more information to the compatibility warning dialog but such a change should be independent of this one. |
|
@michaelgregorius I found a bug that I can only reproduce with changes from this pull request. Default SEGuitar.xpf TripleOscillator instrument in blank project kills sound usually after 1-3 seconds of playing. Need to reopen project to get sound back. I didn't test all bundled instruments and this is only one I found so far. I tried to debug with Valgrind but didn't see any related error, just millions of QT exceptions. |
|
@karmux I am not able to find a problem with the preset SEGuitar.xpf. Here's what I did:
Can you please provide a more detailed description on how to reproduce the problem? Thanks! |
|
@michaelgregorius after finishing testing in master I recompiled this PR and couldn't reproduce it anymore myself also. Very strange but glad that this issue is gone now :) |
|
Unfortunately it's back. I compiled several times this PR and master and never was able to reproduce it in master but always here (sometimes need to reload lmms few times to reproduce). |
|
@karmux I can't reproduce this. SEGuitar use a bunch of effect pluggins. Can you try and delete them one after the other from the sound and see if any one of them is causing this? |
Sounds like #1048 to me. It's caused by some LADSPA plugins IIRC |
|
It seems to be TripleOscillator in general and random LADSPA effects happening only in this PR. |
|
@karmux Can you please provide a project file that can be used to reproduce the problem? Thanks! |
|
@michaelgregorius it happens in every project, even in the empty default project. Culprits seems to be 3 LADSPA effects added to SEGuitar. To reproduce: Use default TripleOscillator in default project, add CEq, CPhaserII and/or C*Plate2x2 effects and play some notes. Adding all three gives better chance to hit the bug. Sooner or later it plays just silence. These three LADSPA effects somehow are able to block sound completely from other instruments as well. Disabling or removing these three plugins gives sound back (no project reload needed). All this happens only if I apply changes from this PR. |
|
Hi @karmux, unfortunately I am still not able to reproduce the problem. Here's what I did:
I have tested for several minutes but no silence appears. Can someone else please check if the problem is reproducible on other systems as well? @karmux, can you please check which of the three plugins has to be removed to get the sound back? I assume that only one of the plugins causes the problem. Thanks! |
|
@michaelgregorius Any of these three plugins could crash the sound. Before I edited files manually but checked several times over that I edited them correctly. Now I learned how to merge PRs into my fork and merged it into my staging branch. Can't reproduce this issue anymore 👍 Sorry for delaying this fix to be merged. |
…ync") (#3145)" This reverts commit e3e14bb in an attempt to fix issue #3292. The problem that's addressed is very elusive: * It cannot be reproduced reliably. * It seems to be more prone to appear in release builds. * It only seems to appear when two or more instruments are mixed into one channel. * If you solo a problematic Triple Osc it goes away. The observations above seem to indicate that there's rather a problem with the mixing (due to multithreading?) and that the changes made to Triple Osc seem to trigger this deeper problem more quickly.
…MMS#3145) Change all phase related variables in Oscillator from float to double. The parameters for the getSample methods have not been changed to double as it should suffice to only update the phase variables with double precision.
…ync") (LMMS#3145)" This reverts commit 291942f in an attempt to fix issue LMMS#3292. The problem that's addressed is very elusive: * It cannot be reproduced reliably. * It seems to be more prone to appear in release builds. * It only seems to appear when two or more instruments are mixed into one channel. * If you solo a problematic Triple Osc it goes away. The observations above seem to indicate that there's rather a problem with the mixing (due to multithreading?) and that the changes made to Triple Osc seem to trigger this deeper problem more quickly.
Fix for #2047
Change all phase related variables in
Oscillatorfromfloattodouble.The parameters for the
getSamplemethods have not been changed todoubleas it should suffice to only update the phase variables with
doubleprecision.