TripleOscillator: Oscillators are getting out of sync #2047
Comments
Maybe you haven't removed the default limiter from FL? I haven't got a clue how synthesis really work, but I can easily imagine that there are different algorithms for making a sine wave... TripleOscillator for LMMS is not the same instrument as 3OSC for FL! If you had used a VSTi and no effects in both maybe we could discuss something. |
|
@Sti2nd I think it's really a bug (if it is not an intended feature). To get an idea of what might be causing this behaviour and to simplify the conditions I have set all three oscillators to play a sine wave instead of a saw. The same effect can still be observed which means that we can likely eliminate the aliasing in the saw oscillators as a possible cause. My guess is that the three oscillators go out of phase over time and that the observed lowering of amplitude is the effect of some destructive interference due to the phasing. Here is the sum of all three sines (fundamental + octave + 2. octave) at the beginning of the note (also the first samples of the test render): Please note that the waveform is symmetric at this point in time and that it looks like expected. And now here is the waveform towards the end of my rendered wav file: Under normal conditions it should look like in the first screenshot. I noticed in the code that the |
Umcaruje
changed the title
|
Yeah, this is a bug. The oscillators should stay in phase if you do not change the phase offset manually. |
|
This is likely to be a floating point precision thing. Getting the frequency for a note involves a lot of divisions, multiplications, powf() calls and whatnot. The tiniest errors add up and result in the oscillators getting out of phase. I made a test project with two oscillators. The first one has no detuning and plays the notes D4, D4, D4. The second one has its coarse detuning automated as 0, -12, -24 and playing the notes D4, D5, D6. This is their difference.
|
|
Rendering that test project, I get a perfectly silent .wav. Can't see any action on the master output "scope" when playing it either. Update: looks like I get silence exporting this from the command line, from the GUI I get those bursts of noise. I have a feeling that this problem might come from some automation SNAFU or somehing like that. Possibly worth an issue of its own. |
|
@grejppi The file works for me but I think the problem is not about using two instances of the Triple Osc. If I change the Triple Oscs in your example to only play a sine wave and move the phase of one of the synths by 180° I get perfect silence when playing in LMMS as is expected. In other words: the phases of the two Triple Oscs do not go out of sync (at least in that small example). I also realized that export seems to be broken because when I render the perfect silence described above I get something that is definitively not silence: If the problem is really caused by floating point precision then the code definitively needs refactoring. Normally you calculate the frequency from the MIDI note value and then initialize a phasor using the frequency and then only increment the phasor using an addition (and a boundary check) with each sample tick. Doing so should work for quite some time without any precision issues. I also noted that I wrote nonsense in the last section of my comment from 12.05.2015. If the oscillators are tuned differently we obviously need separate phase accumulators for each oscillator. However, if they are tuned in octaves apart they should always sync up once the oscillator with the lowest tuning has finished a cycle. |
|
I get phase drift when trying the original recipe from @musikBear, an 18 bar D3. And it looks to me like the drift increases with increasing sampling frequency; can't notice anything when exporting 44100x1, but with more oversampling the problem gets stronger. Smells like precision, rounding or something of that kind. |
|
OK, now I'm sure. It's a precision problem, see https://gist.github.com/softrabbit/d8232cda4bbc3b61cdbc Download, compile and run. Change the one To quote the final lines from both runs on my 32-bit system: Looks like this is one of the places where a 32-bit float isn't quite adequate. |
|
Very nice bughunt there 👍 |
|
Which parts of core are affected by this bug? Oscillator class for sure but what else? How much work do you think is to fix it? Would be interesting to hear if fixing this makes sound more punchy for other instruments also. |
|
@karmux good question! But is this fix added to Master? The ticket is still open Can anyone confirm for Master. |
|
@musikBear I'm on master and this is what very long D3 note made in TripleOscillator (using settings on screenshot above) gives:
|
|
This should be fixed with PR #3145. Here's the result of a long D3 as described in @musikBear's original report. Top is before the changes with float precision. Bottom is the fixed version where the phase variables have been replaced with doubles: I have also repeated my test with the three added sine waves and they now also reproduce correctly after longer time spans. Kudos to @grejppi and @softrabbit for their investigations! :) |
|
@karmux That is almost as weird as mine.. |
|
@musikBear no, the screen shot is from a PR that @michaelgregorious
proposed yesterday and it has not yet have been merged.
…On 7 Dec 2016 13:29, "musikBear" ***@***.***> wrote:
@karmux <https://github.com/karmux> That is almost as weird as mine..
@michaelgregorius <https://github.com/michaelgregorius> Yours looks
perfect! If *that* are current Master then @grejppi
<https://github.com/grejppi> and @softrabbit
<https://github.com/softrabbit> has crushed this bug, and the the ticket
should be closed.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2047 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AF_bPUoGw1ohlagL2RfIUIpwW7V-w7nJks5rFqaxgaJpZM4EX2C7>
.
|
|
Please note that using I you look at the screenshot at the original size you will see that the bottom waveform also starts to fade. With Last but not least, playing a single tone for that long does not seem to be that musical anyway. Unless you are an artist in the spirit of John Cage I guess. ;) |
|
@michaelgregorius Again the bottom screenie using double, looks perfect. |
|
For some patches the difference in sound is already quite noticeable for not so long notes. For example if you take the default preset of 3OSC and set all three oscillators to saw then the current I have also performed a test with the song "Tectonic" by Farbro from the demo folder as it uses several instances of 3OSC. I have rendered it once with the patch applied and once without. I have then inverted the phase on one of the renders and mixed them down. If there was no difference between the tracks this would yield perfect silence. However, this is the result of the action: As you can hear the bass seems to be quite affected by the change. To get an idea of how much random elements could influence the result I have also rendered the double version twice and then performed the actions. The result was that only few elements are affected by the randomness. For example the bass did not sound in this case. So the patch would definitively have an effect on some songs. For example if a song author used the "phaseyness" created by the oscillators as part of his sound design then this aspect of the sound will be gone with the patch. The following instruments use the class
|
OTOH, that phaseyness would've behaved differently for different export settings. So I'd say it's no big loss. |
|
@michaelgregorius super job! 🎆 |
It's the cost of fixing bugs. I wouldn't consider it problematic. I'm not sure how to explain this in a way that would properly warn people. The fix will be mentioned, but the end user effect of the fix, that will just have to be observed first hand. :\ |
LB302 and Monstro should be unaffected, they handle the phase by themselves and just look up values from Oscillator. NES is probably okay if it only uses the noise waveform. That leaves Organic and TripleOscillator, which most certainly are affected. |
|
I have an idea. I will just share it. |
|
People who need the old sound can finish their projects in an old version or bounce to audio. I don't think it's worth it to make the code vase messier for something like this. |
|
@lukas-w Good point, but there are some more issues that should be fixed(mostly |
I think it can be fixed by using the method I mentioned above. I will make a quick test for that. |
|
I've tested with |
No. I took it from memory. Memory wrong. |
|
@PhysSong Since it looks like you've basically solved this, maybe bump it to 1.2.0 ? |
|
Note that fixing this issue may cause performance degrade since it replaces single-precision( |
|
@PhysSong What's the status on this? I don't see a linked PR for the fix you mentioned, but if the fix is in master or stable 1.2 I'd like to close this issue. |
|
I haven't added the patch because it may lead to performance degrades due to the change from |
|
Performance probably doesn't suffer much. See https://stackoverflow.com/questions/4584637/double-or-float-which-is-faster for a discussion. My proof of concept code in a comment above, #2047 (comment), with slight modifications to run longer and print less gave results like this (seconds of user time): NB. It could be that doubles are slower than floats on e.g. ARM systems, but even so I think this phase accumulator should be a small enough part of the whole CPU usage that it wouldn't hurt much. |
|
I think the phase update code is only a very small part of what LMMS is doing when it's playing a song and I am pretty sure that the code with the double implementation will never show up in profiler runs. To quote Donald Knuth: "The real problem is that programmers have spent far too much time worrying about efficiency in the wrong places and at the wrong times; premature optimization is the root of all evil (or at least most of it) in programming." Concerning potentially weaker hardware like ARM: If a user wants to run LMMS on a weaker hardware then that user should pay the price for that and not all of LMMS' users. |
|
If we really don't want to switch to double, we could at least work around the LFO syncing problem by secretly faking it. Like resetting the value to initial phase every time we get to a new measure. A more proper, but quite longer, fix would be to refactor this feature to calculate values based on position rather than adding it up over time... |
|
Hi @PhysSong, can you please create a pull request of your changes so that we can put them through a profiler? To be frank, I find it a bit sad that this issue is five years old now and that it's still open although we know what the problem is, what the solution is and are very close to fixing it. I am pretty sure that if the oscillator had been implemented from the start using |
I'm one of those users on a weaker machine. I vote double precision. |
|
I stumbled in this long thread and was intrigued by it. I did some experiment as well with monstro as that one is said to be stable in previous posts using phases stored in floats. Put osc2 and osc3 to a saw waveform set coarse value to -12 respectively -24 semitone. Play F3. It takes a while (longer than 3osc) but you start to hear changes in the sound as well. By tearing down the volume of one the oscillators and panning the other one to the left and one to the right, you can see in audacity that the phases of left and right also start to shift versus each other. The phenomenon is accumulation of errors due to limited precision. Going from float to double will make it appear later, but eventually it will be there as well. Especially if you consider the whole range of notes and different sampling rates. Depending on the note, the errors accumulate with another speed and become more or less audible. It is like real analog oscillators, they also are never perfectly aligned (apart from the fact that they may even drift), except if you sync them. |
…tputs odd for some target waveforms. The internal waveforms of the class Oscillator produces the wrong amplitude when the input is a negative phase. When doing PM or FM, negative phases may occur. When a negative phase is e.g. passed to the the saw sample, it produces values less than -1.0, hence going out of range. Converted all fraction calls to absFraction calls. Removed the +2 in the function Oscillator::recalcPhase. The comment here was that it was needed to avoid negative phases in case of PM. But by converting fraction to absFraction in the waveforms, negative phases are not an issue anymore. On top of that the m_phase variable gains about 2 extra bits in precision. As side effect of that, it improves the behaviour of the issue LMMS#2047 - TripleOscillator: Oscillators are getting out of sync. Though I did not investigate it thoroughly over different notes and samplerates. Add documentation to the fraction and absFraction functions in lmms_math.h as it was not immediately clear by the name what the functions do. Correct the implementation of the functions in case the flag __INTEL_COMPILER is set. (floorf rounds always down).
A hell of a lot later. The fraction part in single precision floats is 23 bits, in double precision you get 52 bits. If those 29 additional bits translate straight to oscillator drift time it should be the same drift in 17 years as you get in a second with single precision. |
…s odd for some target waveforms. (#5651) The internal waveforms of the class Oscillator produces the wrong amplitude when the input is a negative phase. When doing PM or FM, negative phases may occur. When a negative phase is e.g. passed to the the saw sample, it produces values less than -1.0, hence going out of range. Converted all fraction calls to absFraction calls. Removed the +2 in the function Oscillator::recalcPhase. The comment here was that it was needed to avoid negative phases in case of PM. But by converting fraction to absFraction in the waveforms, negative phases are not an issue anymore. On top of that the m_phase variable gains about 2 extra bits in precision. As side effect of that, it improves the behaviour of the issue #2047 - TripleOscillator: Oscillators are getting out of sync. Though I did not investigate it thoroughly over different notes and samplerates. Add documentation to the fraction and absFraction functions in lmms_math.h as it was not immediately clear by the name what the functions do. Correct the implementation of the functions in case the flag __INTEL_COMPILER is set. (floorf rounds always down).
|
I started working on this again. Should I replace all frequency calculations involving |
…tputs odd for some target waveforms. (LMMS#5651) The internal waveforms of the class Oscillator produces the wrong amplitude when the input is a negative phase. When doing PM or FM, negative phases may occur. When a negative phase is e.g. passed to the the saw sample, it produces values less than -1.0, hence going out of range. Converted all fraction calls to absFraction calls. Removed the +2 in the function Oscillator::recalcPhase. The comment here was that it was needed to avoid negative phases in case of PM. But by converting fraction to absFraction in the waveforms, negative phases are not an issue anymore. On top of that the m_phase variable gains about 2 extra bits in precision. As side effect of that, it improves the behaviour of the issue LMMS#2047 - TripleOscillator: Oscillators are getting out of sync. Though I did not investigate it thoroughly over different notes and samplerates. Add documentation to the fraction and absFraction functions in lmms_math.h as it was not immediately clear by the name what the functions do. Correct the implementation of the functions in case the flag __INTEL_COMPILER is set. (floorf rounds always down).
|
Hi @PhysSong, I think this issue can be closed. I have repeated the original test described at the start of this issue and got a solid signal. |
|
stable-1.2 vs. The issue has improved greatly but it's still showing a hint of the issue mentioned. It's clearly improved so I'm pushing this to 1.3+. Demo project after @musikBear 's instructions: sawdetune.mmp.txt |
Just to make sure that i understand the image: |
Yes. Top track is on stable-1.2 and the bottom one on master (d71116b). |
I have spend some time trying to figure out, why lmms sounds less 'punchy/ agressive' than other daw's
during that i have seen something strange
To reproduce
insert a std 3oc
set all 3 osc to saw
insert a 18 bar long note in D3
play
The sound variates in a sweep-like fashion:
Why?
There are no
why is this note not completely uniform in spektrum?
If i do exactly the same with 3oc in fls-demo, i get a completely uniform spectrum
The text was updated successfully, but these errors were encountered: