Skip to content

Commit

Permalink
Fix MPE Pitch Bend with non-MPE devices (surge-synthesizer#1492)
Browse files Browse the repository at this point in the history
MPE pitch bend with non-MPE devices would double bend since the
(really unused) MPE global pitch bend got double counted. So make
it just be single counted, and put the fix in place with a regtest.
While at it, add a regtest for non-monotonic tunings and tuning in
the presence of pitch bend.

Closes surge-synthesizer#1489
Closes surge-synthesizer#1487
  • Loading branch information
baconpaul authored Jan 22, 2020
1 parent b6134fa commit 143cfb6
Show file tree
Hide file tree
Showing 10 changed files with 394 additions and 52 deletions.
13 changes: 10 additions & 3 deletions src/common/SurgeSynthesizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ SurgeSynthesizer::SurgeSynthesizer(PluginLayer* parent, std::string suppliedData
mpeEnabled = false;
mpeVoices = 0;
mpePitchBendRange = Surge::Storage::getUserDefaultValue(&storage, "mpePitchBendRange", 48);
mpeGlobalPitchBendRange = 2;
mpeGlobalPitchBendRange = 0;

// load_patch(0);
}
Expand Down Expand Up @@ -739,6 +739,13 @@ void SurgeSynthesizer::pitchBend(char channel, int value)
}
}

/*
** So here's the thing. We want global pitch bend modulation to work for other things in MPE mode.
** This code has beenhere forever. But that means we need to ignore the channel[0] mpe pitchbend
** elsewhere, especially since the range was hardwired to 2 (but is now 0). As far as I know the
** main MPE devices don't have a global pitch bend anyway so this just screws up regular keyboards
** sending channel 0 pitch bend in MPE mode.
*/
if (!mpeEnabled || channel == 0)
{
storage.pitch_bend = value / 8192.f;
Expand Down Expand Up @@ -848,7 +855,7 @@ void SurgeSynthesizer::onRPN(int channel, int lsbRPN, int msbRPN, int lsbValue,
mpeEnabled = msbValue > 0;
mpeVoices = msbValue & 0xF;
mpePitchBendRange = Surge::Storage::getUserDefaultValue(&storage, "mpePitchBendRange", 48);
mpeGlobalPitchBendRange = 2;
mpeGlobalPitchBendRange = 0;
return;
}
else if (lsbRPN == 4 && msbRPN == 0 && channel != 0 && channel != 0xF )
Expand All @@ -857,7 +864,7 @@ void SurgeSynthesizer::onRPN(int channel, int lsbRPN, int msbRPN, int lsbValue,
mpeEnabled = true;
mpeVoices = msbValue & 0xF;
mpePitchBendRange = Surge::Storage::getUserDefaultValue(&storage, "mpePitchBendRange", 48);
mpeGlobalPitchBendRange = 2;
mpeGlobalPitchBendRange = 0;
return;
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/common/dsp/SurgeVoice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ inline float get1f(__m128 m, int i)

float SurgeVoiceState::getPitch()
{
return key + mainChannelState->pitchBendInSemitones + voiceChannelState->pitchBendInSemitones +
/*
** For this commented out section, see the comment on MPE global pitch bend in SurgeSynthesizer::pitchBend
*/
return key + /* mainChannelState->pitchBendInSemitones + */ voiceChannelState->pitchBendInSemitones +
detune;
}

Expand Down
12 changes: 10 additions & 2 deletions src/headless/Player.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,18 @@ void playAsConfigured(std::shared_ptr<SurgeSynthesizer> surge,
while (currEvt < events.size() && events[currEvt].atSample <= cs + BLOCK_SIZE - 1)
{
Event e = events[currEvt];
if (e.type == Event::NOTE_ON)
switch( e.type )
{
case Event::NOTE_ON:
surge->playNote(e.channel, e.data1, e.data2, 0);
if (e.type == Event::NOTE_OFF)
break;
case Event::NOTE_OFF:
surge->releaseNote(e.channel, e.data1, e.data2);
break;
case Event::LAMBDA_EVENT:
e.surgeLambda(surge);
break;
}

currEvt++;
}
Expand Down
5 changes: 4 additions & 1 deletion src/headless/Player.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ struct Event
{
NOTE_ON,
NOTE_OFF,

LAMBDA_EVENT,

NO_EVENT // Useful if you want to keep the player running and have nothing happen
} Type; // FIXME: Controllers etc...

Expand All @@ -36,6 +37,8 @@ struct Event
char data1;
char data2;

std::function<void(std::shared_ptr<SurgeSynthesizer>)> surgeLambda;

long atSample;
};

Expand Down
91 changes: 72 additions & 19 deletions src/headless/UnitTestUtilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,19 @@ namespace Surge
{
namespace Test
{
double frequencyForNote( std::shared_ptr<SurgeSynthesizer> surge, int note,
int seconds, bool audioChannel,
int midiChannel )
{
auto events = Surge::Headless::makeHoldNoteFor( note, 44100 * seconds, 64, midiChannel );
float *buffer;
int nS, nC;
Surge::Headless::playAsConfigured( surge, events, &buffer, &nS, &nC );

REQUIRE( nC == 2 );
REQUIRE( nS >= 44100 * seconds );
REQUIRE( nS <= 44100 * seconds + 4 * BLOCK_SIZE );

// Trim off the leading and trailing
int nSTrim = (int)(nS / 2 * 0.8);
int start = (int)( nS / 2 * 0.05 );
float *leftTrimmed = new float[nSTrim];
double frequencyFromData( float *buffer, int nS, int nC, int audioChannel,
int start, int trimTo )
{
float *leftTrimmed = new float[trimTo];

for( int i=0; i<nSTrim; ++i )
for( int i=0; i<trimTo; ++i )
leftTrimmed[i] = buffer[ (i + start) * 2 + audioChannel ];

// OK so now look for sample times between positive/negative crosses
int v = -1;
uint64_t dSample = 0, crosses = 0;
for( int i=0; i<nSTrim -1; ++i )
for( int i=0; i<trimTo -1; ++i )
if( leftTrimmed[i] < 0 && leftTrimmed[i+1] > 0 )
{
if( v > 0 )
Expand All @@ -46,9 +34,50 @@ double frequencyForNote( std::shared_ptr<SurgeSynthesizer> surge, int note,

float time = aSample / 44100.0;
float freq = 1.0 / time;


delete[] leftTrimmed;
return freq;
}

double frequencyForNote( std::shared_ptr<SurgeSynthesizer> surge, int note,
int seconds, int audioChannel,
int midiChannel )
{
auto events = Surge::Headless::makeHoldNoteFor( note, 44100 * seconds, 64, midiChannel );
float *buffer;
int nS, nC;
Surge::Headless::playAsConfigured( surge, events, &buffer, &nS, &nC );
for( auto i=0; i<500; ++i ) surge->process(); // Ring out any transients on this synth

REQUIRE( nC == 2 );
REQUIRE( nS >= 44100 * seconds );
REQUIRE( nS <= 44100 * seconds + 4 * BLOCK_SIZE );

// Trim off the leading and trailing
int nSTrim = (int)(nS / 2 * 0.8);
int start = (int)( nS / 2 * 0.05 );

auto freq = frequencyFromData( buffer, nS, nC, audioChannel, start, nSTrim );
delete[] buffer;

return freq;
}

double frequencyForEvents( std::shared_ptr<SurgeSynthesizer> surge,
Surge::Headless::playerEvents_t &events,
int audioChannel,
int startSample, int endSample )
{
float *buffer;
int nS, nC;

Surge::Headless::playAsConfigured( surge, events, &buffer, &nS, &nC );
for( auto i=0; i<500; ++i ) surge->process(); // Ring out any transients on this synth

REQUIRE( startSample < nS );
REQUIRE( endSample < nS );

auto freq = frequencyFromData( buffer, nS, nC, audioChannel, startSample, endSample - startSample );
delete[] buffer;

return freq;
Expand Down Expand Up @@ -81,6 +110,30 @@ void setupStorageRanges(Parameter *start, Parameter *endIncluding,
storage_id_end = max_id + 1;
}

/*
** Create a surge pointer on init sine
*/
std::shared_ptr<SurgeSynthesizer> surgeOnSine()
{
auto surge = Surge::Headless::createSurge(44100);

std::string otp = "Init Sine";
bool foundInitSine = false;
for (int i = 0; i < surge->storage.patch_list.size(); ++i)
{
Patch p = surge->storage.patch_list[i];
if (p.name == otp)
{
surge->loadPatch(i);
foundInitSine = true;
break;
}
}
if( ! foundInitSine )
return nullptr;
else
return surge;
}


}
Expand Down
12 changes: 10 additions & 2 deletions src/headless/UnitTestUtilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,21 @@ namespace Test {
** At one day we could do this with autocorrelation instead but no need now.
*/
double frequencyForNote( std::shared_ptr<SurgeSynthesizer> surge, int note,
int seconds = 2, bool audioChannel = 0,
int seconds = 2, int audioChannel = 0,
int midiChannel = 0 );

double frequencyFromData( float *buffer, int nS, int nC, int audioChannel,
int start, int trimTo );

double frequencyForEvents( std::shared_ptr<SurgeSynthesizer> surge,
Surge::Headless::playerEvents_t &events,
int audioChannel,
int startSample, int endSample );

void copyScenedataSubset(SurgeStorage *storage, int scene, int start, int end);
void setupStorageRanges(Parameter *start, Parameter *endIncluding,
int &storage_id_start, int &storage_id_end);


std::shared_ptr<SurgeSynthesizer> surgeOnSine();
}
}
Loading

0 comments on commit 143cfb6

Please sign in to comment.