TripleOscillator: Phase Randomization #4369

SecondFlight · 2018-05-22T03:13:15Z

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:

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.
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.

Wallacoloo · 2018-05-23T05:11:46Z

plugins/triple_oscillator/TripleOscillator.cpp

+		// setup phase randomization knob
+		Knob * phrk = new TripleOscKnob( this );
+		phrk->move( 165, knob_y );
+		phrk->setHintText( tr( "Osc %1 phase rand:" ).


How about "Osc %1 phase randomization:"? Don't want to throw off new users with too much jargon.

Works for me. I'll update that later today.

Wallacoloo · 2018-05-23T05:15:38Z

plugins/triple_oscillator/TripleOscillator.cpp

+			float phaseRandR;
+
+			phaseRandL = fastRandf( 1 ) * m_osc[i]->m_phaseRand;
+			phaseRandR = fastRandf( 1 ) * m_osc[i]->m_phaseRand;


Any reason for separating the declaration and initialization of these? Why not just

float phaseRandL = fastRandf( 1 ) * m_osc[i]->m_phaseRand; float phaseRandR = fastRandf( 1 ) * m_osc[i]->m_phaseRand;

?

Actually no, thanks for pointing that out. It made sense when it was set conditionally, but I removed that.

curlymorphic · 2018-05-25T21:34:59Z

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.

SecondFlight · 2018-05-26T02:10:14Z

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 · 2018-05-26T03:53:22Z

plugins/triple_oscillator/TripleOscillator.cpp

@@ -306,6 +324,9 @@ void TripleOscillator::playNote( NotePlayHandle * _n,
 		for( int i = NUM_OF_OSCILLATORS - 1; i >= 0; --i )
 		{

+			float phaseRandL = fastRandf( 1 ) * m_osc[i]->m_phaseRand;
+			float phaseRandR = fastRandf( 1 ) * m_osc[i]->m_phaseRand;


FYI, a periods corresponds to 2π in phase.

Never mind, Oscillator uses unusual convention(a period = phase of 1). So this code is fine as-is.

Just a random thought: would it be better to use -0.5 ~ 0.5 instead of 0 ~ 1?

PhysSong · 2018-05-26T03:54:46Z

plugins/triple_oscillator/TripleOscillator.cpp

@@ -314,14 +335,14 @@ void TripleOscillator::playNote( NotePlayHandle * _n,
 						&m_osc[i]->m_modulationAlgoModel,
 						_n->frequency(),
 						m_osc[i]->m_detuningLeft,
-						m_osc[i]->m_phaseOffsetLeft,
+						m_osc[i]->m_phaseOffsetLeft	+ phaseRandL,


Use a space before + instead of a tab.

PhysSong · 2018-05-26T03:55:01Z

plugins/triple_oscillator/TripleOscillator.cpp

@@ -331,22 +352,21 @@ void TripleOscillator::playNote( NotePlayHandle * _n,
 						&m_osc[i]->m_modulationAlgoModel,
 						_n->frequency(),
 						m_osc[i]->m_detuningLeft,
-						m_osc[i]->m_phaseOffsetLeft,
+						m_osc[i]->m_phaseOffsetLeft	+ phaseRandL,


PhysSong · 2018-05-28T05:42:31Z

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.

lmms/plugins/triple_oscillator/TripleOscillator.cpp

Lines 195 to 207 in 3e8120d

    
           void OscillatorObject::updatePhaseOffsetLeft() 
        
           { 
        
           	m_phaseOffsetLeft = ( m_phaseOffsetModel.value() + 
        
           				m_stereoPhaseDetuningModel.value() ) / 360.0f; 
        
           } 
        
           void OscillatorObject::updatePhaseOffsetRight() 
        
           { 
        
           	m_phaseOffsetRight = m_phaseOffsetModel.value() / 360.0f; 
        
           }

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().

SecondFlight · 2018-05-28T15:57:58Z

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 · 2018-05-29T01:32:36Z

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).

SecondFlight · 2018-05-29T10:36:06Z

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 :)

SecondFlight · 2018-05-29T10:37:27Z

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 · 2023-06-24T05:33:38Z

TODOs:

Decide the multiplier to random variables(0~1 vs. -0.5~0.5), see TripleOscillator: Phase Randomization #4369 (comment)
(Optional?) Restore the behavior of m_ext_phaseOffset, see TripleOscillator: Phase Randomization #4369 (comment)

SecondFlight added 13 commits May 15, 2018 20:51

Testing stuff

efe3457

(Broken) Naive attempt at phase randomization

706feba

More broken changes

6d7d8a0

Fix errors

4245c67

Add knob to UI - doesn't work yet

e478ca7

Use rand from lmms_math.h

6ed7817

Undo unnecessary change to oscillator.h

fc7be3d

Fix voice tracking

e001869

Fix segfault due to QMap not being thread-safe

c2dcdf5

Fix phase rand knobs

5a74a49

Clean up oscillator.h

c584a00

Turns out that a disturbing amount of that was unnecessary

6a05d0e

UI Tweaks

2708e6c

Wallacoloo reviewed May 23, 2018

View reviewed changes

Implement changes suggested by Wallacoloo

cb571b7

Wallacoloo approved these changes May 24, 2018

View reviewed changes

curlymorphic approved these changes May 25, 2018

View reviewed changes

PhysSong reviewed May 26, 2018

View reviewed changes

SecondFlight mentioned this pull request Apr 12, 2019

Indentation (tabs or spaces) #4810

Closed

ryuukumar mentioned this pull request Jul 21, 2020

Reorganising LMMS, renaming editors and classes and everything in between (cont. #120) #5592

Closed

56 tasks

Merge branch 'master' into phase-rand-experiments

738eea2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TripleOscillator: Phase Randomization #4369

TripleOscillator: Phase Randomization #4369

SecondFlight commented May 22, 2018 •

edited

Loading

Wallacoloo May 23, 2018

SecondFlight May 23, 2018

Wallacoloo May 23, 2018

SecondFlight May 23, 2018

curlymorphic commented May 25, 2018

SecondFlight commented May 26, 2018

PhysSong May 26, 2018

PhysSong May 26, 2018

PhysSong May 28, 2018

PhysSong May 26, 2018

PhysSong May 26, 2018

PhysSong commented May 28, 2018

SecondFlight commented May 28, 2018

PhysSong commented May 29, 2018

SecondFlight commented May 29, 2018 •

edited

Loading

SecondFlight commented May 29, 2018

PhysSong commented Jun 24, 2023

TripleOscillator: Phase Randomization #4369

Are you sure you want to change the base?

TripleOscillator: Phase Randomization #4369

Conversation

SecondFlight commented May 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

curlymorphic commented May 25, 2018

SecondFlight commented May 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PhysSong commented May 28, 2018

SecondFlight commented May 28, 2018

PhysSong commented May 29, 2018

SecondFlight commented May 29, 2018 • edited Loading

SecondFlight commented May 29, 2018

PhysSong commented Jun 24, 2023

SecondFlight commented May 22, 2018 •

edited

Loading

SecondFlight commented May 29, 2018 •

edited

Loading