TripleOscillator: Phase Randomization

[josh-audio]

This PR would add phase randomization to TripleOscillator. When enabled for an oscillator, the phase of the left/right voices are separately randomized. The amount is controlled by a knob - 100% means complete randomization, while 10% means it can move up to 10% of one period. The default is 0% which will have no effect.

This is a feature I've wanted for quite some time. My reasoning behind adding this is as follows:

• FL's 3xosc has a similar feature. It is a global phase randomization option instead of per-voice, but I think it makes sense to do it per-voice given TripleOscillator's modulation options.
• Phase randomization is essential to create a supersaw sound. Creating such a sound in 3xosc is a simple matter of layering, while in TripleOscillator it is basically impossible. You can fake it, but it isn't the same.

There are two things to note with this PR:

1. The art needs reworking, preferably by whoever made the original. I don't have the project files for it and there's only so much you can do with a clone tool.
2. I'm not entirely sure what side effects the change in Oscillator.h will have, if any. Using a float reference meant I couldn't pass in the result of an addition to the object without things becoming very broken very quickly.

Tagging @curlymorphic.

[curlymorphic]

I am liking the sound of this. I have had a look at the code, and it appears that your change to the OSCILLATOR should not have any negative impacts, as both m_ext_phaseOffset and m_phaseOffset are initialized to the same variable, prior to this pull request m_ext_phaseOffset appears to be unused. However a test with all the internal synths that use this oscillator would be beneficial.

[josh-audio]

Alright @curlymorphic, thanks for taking the time to review. I'll merge the PR once I've tested everything and @LocoMatt has gotten back to me with the updated artwork.

[PhysSong]

prior to this pull request m_ext_phaseOffset appears to be unused.

No, this is needed by TripleOscillator. Currently one can change oscillator phase while playing a note. With this PR the behavior will be lost. https://github.com/LMMS/lmms/blob/3e8120d532821e351e7a6e8298644007273d7baa/plugins/triple_oscillator/TripleOscillator.cpp#L195-L207

Also, there's no point to keep m_ext_phaseOffset if you change it from reference to normal variable. Please consider dropping m_ext_phaseOffset and find a way to replace Oscillator::recalcPhase().

[josh-audio]

Sounds good, thanks for looking into that! I remember thinking I'd go back to look at the live updating phase but I must have forgotten. I'll implement all those changes along with the UI as soon as I get the artwork.

[PhysSong]

FYI, I'm planning to add pitch randomization and unison later. So I afraid PR in the artwork may be unclear(P can be either pitch or phase).

[josh-audio]

I was planning to do the same thing, so let me go ahead and share my thoughts about that.

I'm concerned about the UI getting cluttered. I'm not sure there's room for three more knobs per row. My thought was to add a unison feature in the chord / arpeggios tab.

Of course, you would save two knobs per row if you made unison a global feature instead of per-voice, and I think that could make a lot of sense.

Allowing for different sizes of plugins would also be a great way to solve this, but that's a whole different can of worms. For now though, the small size encourages simplicity which I don't think is a bad thing.

Anyways, those are my thoughts, I'll take that off my list of things to eventually do :)

[josh-audio]

Oh also, I'll let LocoMatt know to make the text different. He hasn't done the text yet, and I think I'll have him put "PHR".

[PhysSong]

TODOs:

• [ ] Decide the multiplier to random variables(0~1 vs. -0.5~0.5), see https://github.com/LMMS/lmms/pull/4369#discussion_r191114609
• [ ] (Optional?) Restore the behavior of m_ext_phaseOffset, see https://github.com/LMMS/lmms/pull/4369#issuecomment-392424837

[rdrpenguin04]

My two cents: the random variable could be a range from 0 to 0.5, but the actual randomness could shift the phase anywhere from -x to x. I believe we have a separate phase offset, so if someone actually wanted to base the randomness at 0, they could use an offset to get back to 0; I just feel like the more sane default is to randomize around 0 phase.

[rubiefawn]

I'm concerned about the UI getting cluttered. I'm not sure there's room for three more knobs per row. My thought was to add a unison feature in the chord / arpeggios tab.

Regarding the user interface for this plugin and the extra knob(s) required for this feature, I'm planning on converting the assets to vector and separating the knob text from the background image, so don't worry too much about how much space the knobs take. If one knob for each oscillator is mechanically useful, let's go with that, and I can make the plugin window bigger to accommodate.

[rubiefawn]

Of course, you would save two knobs per row if you made unison a global feature instead of per-voice, and I think that could make a lot of sense.

FL Studio's 3 OSC instrument currently has just a single "PHASE RAND" knob at the bottom, so there is precedent for this. On the other hand, Xfer's Serum has per-oscillator phase randomization.

What are the benefits of having per-oscillator phase randomization instead of global? Given that Triple Oscillator is much simpler than Serum, I am currently of the opinion it should use a single global phase randomization knob, but I could be convinced otherwise.