Skip to content

Commit

Permalink
Fixed to ADSR Analog Mode
Browse files Browse the repository at this point in the history 
ADSR analog mode had two bugs

1. It misused _mm_and_ps so was incorrectly rounding decay time
2. It didn't allow for swells in sustain once the sustain stage
   was underway

This fixes both those problems. In some cases the fix to surge-synthesizer#1 will
change the sound of patches by making the decay time slower (but
more correct).

Closes surge-synthesizer#1439
Closes surge-synthesizer#825
  • Loading branch information
@baconpaul
baconpaul committed Jan 8, 2020
1 parent 5b678cc commit 1ba7676
Show file tree
Hide file tree
Showing 2 changed files with 207 additions and 68 deletions.
16 changes: 14 additions & 2 deletions src/common/dsp/AdsrEnvelope.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,13 @@ class AdsrEnvelope : public ModulationSource

bool gate = (envstate == s_attack) || (envstate == s_decay);
__m128 v_gate = gate ? _mm_set_ss(v_cc) : _mm_set_ss(0.f);
__m128 v_is_gate = _mm_cmpgt_ss( v_gate, _mm_set_ss( 0.0 ) );

discharge = _mm_and_ps(_mm_or_ps(_mm_cmpgt_ss(v_c1_delayed, one), discharge), v_gate);
// The original code here was
// _mm_and_ps(_mm_or_ps(_mm_cmpgt_ss(v_c1_delayed, one), discharge), v_gate);
// which ored in the v_gate value as opposed to the boolean
discharge = _mm_and_ps( _mm_or_ps(_mm_cmpgt_ss(v_c1_delayed, one), discharge), v_is_gate );

v_c1_delayed = v_c1;

__m128 S = _mm_load_ss(&lc[s].f);
Expand All @@ -174,7 +179,14 @@ class AdsrEnvelope : public ModulationSource
__m128 v_release = v_gate;

__m128 diff_v_a = _mm_max_ss(_mm_setzero_ps(), _mm_sub_ss(v_attack, v_c1));
__m128 diff_v_d = _mm_min_ss(_mm_setzero_ps(), _mm_sub_ss(v_decay, v_c1));

// This change from a straight min allows sustain swells
__m128 diff_vd_kernel = _mm_sub_ss(v_decay, v_c1);
__m128 diff_vd_kernel_min = _mm_min_ss(_mm_setzero_ps(), diff_vd_kernel );
__m128 dis_and_gate = _mm_and_ps(discharge, v_is_gate );
__m128 diff_v_d = _mm_or_ps(_mm_and_ps(dis_and_gate, diff_vd_kernel),
_mm_andnot_ps(dis_and_gate, diff_vd_kernel_min));

__m128 diff_v_r = _mm_min_ss(_mm_setzero_ps(), _mm_sub_ss(v_release, v_c1));

// calculate coefficients for envelope
Expand Down
259 changes: 193 additions & 66 deletions src/headless/UnitTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,9 @@ TEST_CASE( "ADSR Envelope Behaviour", "[mod]" )
auto runAdsr = [surge](float a, float d, float s, float r,
int a_s, int d_s, int r_s,
bool isAnalog,
float releaseAfter, float runUntil )
float releaseAfter, float runUntil,
float pushSusAt = -1,
float pushSusTo = 0 )
{
auto* adsrstorage = &(surge->storage.getPatch().scene[0].adsr[0]);
std::shared_ptr<AdsrEnvelope> adsr( new AdsrEnvelope() );
Expand Down Expand Up @@ -549,6 +551,7 @@ TEST_CASE( "ADSR Envelope Behaviour", "[mod]" )
adsr->attack();

bool released = false;
bool pushSus = false;
int i = 0;
std::vector<std::pair<float,float>> res;
res.push_back(std::make_pair(0.f, 0.f));
Expand All @@ -565,6 +568,13 @@ TEST_CASE( "ADSR Envelope Behaviour", "[mod]" )
adsr->release();
released = true;
}

if( pushSusAt > 0 && ! pushSus && t > pushSusAt )
{
pushSus = true;
svn(&(adsrstorage->s), pushSusTo);
copyScenedataSubset(&(surge->storage), 0, ids, ide);
}

adsr->process_block();
res.push_back( std::make_pair( (float)t, adsr->output ) );
Expand Down Expand Up @@ -669,6 +679,7 @@ TEST_CASE( "ADSR Envelope Behaviour", "[mod]" )
}
};

#if 0
SECTION( "Test the Digital Envelope" )
{
for( int as=0;as<3;++as )
Expand Down Expand Up @@ -711,7 +722,7 @@ TEST_CASE( "ADSR Envelope Behaviour", "[mod]" )

if( obs.first > a + d + d * 0.5 && obs.first < holdFor && s > 0.05 ) // that 0.1 lets the delay ring off
{
REQUIRE( obs.second < s * s * 1.02 );
REQUIRE( obs.second == Approx( s * s ).margin( 1e-3 ) );
heldPeriod.push_back(obs.second);
}
if( obs.first > a + d && obs.second < 1e-6 && zerot == 0 )
Expand Down Expand Up @@ -748,7 +759,7 @@ TEST_CASE( "ADSR Envelope Behaviour", "[mod]" )
REQUIRE( valAtRelEnd < s * 0.025 );
}
};

testAnalog( 0.1, 0.2, 0.5, 0.1 );
testAnalog( 0.1, 0.2, 0.0, 0.1 );
for( int rc=0;rc<50; ++rc )
Expand All @@ -759,91 +770,207 @@ TEST_CASE( "ADSR Envelope Behaviour", "[mod]" )
auto r = rand() * 1.0 / RAND_MAX;
testAnalog( a, d, s, r);
}

}

#endif

// This is just a rudiemntary little test of this in digital mode
SECTION( "Test Digital Sus Push" )
{
auto testSusPush = [&]( float s1, float s2 )
{
auto digPush = runAdsr( 0.05, 0.05, s1, 0.1, 0, 0, 0, false, 0.5, s2, 0.25, s2 );
int obs = 0;
for( auto s : digPush )
{
if( s.first > 0.2 && obs == 0 )
{
REQUIRE( s.second == Approx( s1 ).margin( 1e-5 ) );
obs++;
}
if( s.first > 0.3 && obs == 1 )
{
REQUIRE( s.second == Approx( s2 ).margin( 1e-5 ) );
obs++;
}
}
};

for( auto i=0; i<10; ++i )
{
auto s1 = 0.95f * rand() / RAND_MAX + 0.02;
auto s2 = 0.95f * rand() / RAND_MAX + 0.02;
testSusPush( s1, s2 );
}
}

/*
** This section recreates the somewhat painful SSE code in readable stuff
*/
SECTION( "Clone the Analog" )
{
auto analogClone = [](float a_sec, float d_sec, float s, float r_sec, float releaseAfter, float runUntil )
{
float a = log(a_sec)/log(2.0);
float d = log(d_sec)/log(2.0);
float r = log(r_sec)/log(2.0);
auto analogClone = [](float a_sec, float d_sec, float s, float r_sec, float releaseAfter, float runUntil, float pushSusAt = -1, float pushSusTo = 0 )
{
float a = limit_range((float)( log(a_sec)/log(2.0) ), -8.f, 5.f);
float d = limit_range((float)( log(d_sec)/log(2.0) ), -8.f, 5.f);
float r = limit_range((float)( log(r_sec)/log(2.0) ), -8.f, 5.f);

int i = 0;
bool released = false;
std::vector<std::pair<float,float>> res;
res.push_back(std::make_pair(0.f, 0.f));

auto v_c1 = 0.f;
auto v_c1_delayed = 0.f;
bool discharge = false;
const float v_cc = 1.5f;
int i = 0;
bool released = false;
std::vector<std::pair<float,float>> res;
res.push_back(std::make_pair(0.f, 0.f));

float v_c1 = 0.f;
float v_c1_delayed = 0.f;
bool discharge = false;
const float v_cc = 1.5f;

while( true )
while( true )
{
float t = 1.0 * (i+1) * BLOCK_SIZE * dsamplerate_inv;
i++;
if( t > runUntil || runUntil < 0 )
break;

if( t > releaseAfter && ! released )
{
auto t = 1.0 * (i+1) * BLOCK_SIZE * dsamplerate_inv;
i++;
if( t > runUntil || runUntil < 0 )
break;
released = true;
}

if( pushSusAt > 0 && t > pushSusAt )
s = pushSusTo;

auto gate = !released;
float v_gate = gate ? v_cc : 0.f;

// discharge = _mm_and_ps(_mm_or_ps(_mm_cmpgt_ss(v_c1_delayed, one), discharge), v_gate);
discharge = ( ( v_c1_delayed > 1 ) || discharge ) && gate;
v_c1_delayed = v_c1;

float S = s * s;

float v_attack = discharge ? 0 : v_gate;
// OK so this line:
// __m128 v_decay = _mm_or_ps(_mm_andnot_ps(discharge, v_cc_vec), _mm_and_ps(discharge, S));
// The semantic intent is discharge ? S : v_cc
// but in the ADSR discharge has a value of v_gate which is 1.5 (v_cc) not 1.0.
// That bitwise and with 1.5 acts as a binary filter (the mantissa) and a rounding operation
// (from the .5) so I need to duplicate it exactly here.
/*
__m128 sM = _mm_load_ss(&S);
__m128 dM = _mm_load_ss(&v_cc);
__m128 vdv = _mm_and_ps(dM, sM);
float v_decay = discharge ? vdv[0] : v_cc;
*/
// Alternately I can correct the SSE code for discharge
float v_decay = discharge ? S : v_cc;
float v_release = v_gate;

float diff_v_a = std::max( 0.f, v_attack - v_c1 );
float diff_v_d = ( discharge && gate ) ? v_decay - v_c1 : std::min( 0.f, v_decay - v_c1 );
// float diff_v_d = std::min( 0.f, v_decay - v_c1 );
float diff_v_r = std::min( 0.f, v_release - v_c1 );

const float shortest = 6.f;
const float longest = -2.f;
const float coeff_offset = 2.f - log( samplerate / BLOCK_SIZE ) / log( 2.f );

float coef_A = pow( 2.f, std::min( 0.f, coeff_offset - a ) );
float coef_D = pow( 2.f, std::min( 0.f, coeff_offset - d ) );
float coef_R = pow( 2.f, std::min( 0.f, coeff_offset - r ) );

v_c1 = v_c1 + diff_v_a * coef_A;
v_c1 = v_c1 + diff_v_d * coef_D;
v_c1 = v_c1 + diff_v_r * coef_R;


if( t > releaseAfter && ! released )
{
released = true;
}
// adsr->process_block();
float output = v_c1;
if( !gate && !discharge && v_c1 < 1e-6 )
output = 0;

res.push_back( std::make_pair( (float)t, output ) );
}
return res;
};

auto gate = !released;
auto v_gate = gate ? v_cc : 0.f;
SECTION( "Clone the Analog" )
{
auto compareSrgRepl = [&](float a, float d, float s, float r )
{
auto t = a + d + 0.5 + r * 3;
auto surgeA = runAdsr( a, d, s, r, 0, 0, 0, true, a + d + 0.5, t );
auto replA = analogClone( a, d, s, r, a + d + 0.5, t );

// discharge = _mm_and_ps(_mm_or_ps(_mm_cmpgt_ss(v_c1_delayed, one), discharge), v_gate);
discharge = ( ( v_c1_delayed > 1 ) || discharge ) && gate;
v_c1_delayed = v_c1;
REQUIRE( surgeA.size() == replA.size() );

for( auto i=0; i<surgeA.size(); ++i )
{
REQUIRE( replA[i].second == Approx( surgeA[i].second ).margin( 1e-6 ) );
}
};

auto S = s * s;
compareSrgRepl( 0.1, 0.2, 0.5, 0.1 );
compareSrgRepl( 0.1, 0.2, 0.0, 0.1 );

for( int rc=0;rc<100; ++rc )
{
float a = rand() * 1.0 / RAND_MAX + 0.01;
float d = rand() * 1.0 / RAND_MAX + 0.01;
float s = 0.7 * rand() * 1.0 / RAND_MAX + 0.1; // we have tested the s=0 case above
float r = rand() * 1.0 / RAND_MAX + 0.01;
INFO( "Testing with ADSR=" << a << " " << d << " " << s << " " << r );
compareSrgRepl( a, d, s, r );

auto v_attack = discharge ? 0 : v_gate;
auto v_decay = discharge ? S : v_cc;
auto v_release = v_gate;
}
}

auto diff_v_a = std::max( 0.f, v_attack - v_c1 );
auto diff_v_d = std::min( 0.f, v_decay - v_c1 );
auto diff_v_r = std::min( 0.f, v_release - v_c1 );
SECTION( "Test Analog Sus Push" )
{
auto testSusPush = [&]( float s1, float s2 )
{
auto aSurgePush = runAdsr( 0.05, 0.05, s1, 0.1, 0, 0, 0, true, 0.5, s2, 0.25, s2 );
auto aDupPush = analogClone( 0.05, 0.05, s1, 0.1, 0.5, 0.7, 0.25, s2 );

const float shortest = 6.f;
const float longest = -2.f;
const float coeff_offset = 2.f - log( samplerate / BLOCK_SIZE ) / log( 2.f );
int obs = 0;
INFO( "Analog Dup passes sus push" );
for( auto s : aDupPush )
{
if( s.first > 0.2 && obs == 0 )
{
REQUIRE( s.second == Approx( s1 * s1 ).margin( 1e-3 ) );
obs++;
}
if( s.first > 0.4 && obs == 1 )
{
REQUIRE( s.second == Approx( s2 * s2 ).margin( 1e-3 ) );
obs++;
}
}

float coef_A = pow( 2.f, std::min( 0.f, coeff_offset - a ) );
float coef_D = pow( 2.f, std::min( 0.f, coeff_offset - d ) );
float coef_R = pow( 2.f, std::min( 0.f, coeff_offset - r ) );

v_c1 = v_c1 + diff_v_a * coef_A;
v_c1 = v_c1 + diff_v_d * coef_D;
v_c1 = v_c1 + diff_v_r * coef_R;


// adsr->process_block();
auto output = v_c1;
if( !gate && !discharge && v_c1 < 1e-6 )
output = 0;

res.push_back( std::make_pair( (float)t, output ) );
obs = 0;
INFO( "Analog SSE passes sus push" );
for( auto s : aSurgePush )
{
if( s.first > 0.2 && obs == 0 )
{
REQUIRE( s.second == Approx( s1 * s1 ).margin( 1e-3 ) );
obs++;
}
if( s.first > 0.4 && obs == 1 )
{
REQUIRE( s.second == Approx( s2 * s2 ).margin( 1e-5 ) );
obs++;
}
}
return res;
};

auto surgeA = runAdsr( 0.1, 0.2, 0.5, 0.1, 0, 0, 0, true, .5, 1.0 );
auto replA = analogClone( 0.1, 0.2, 0.5, 0.1, 0.5, 1.0 );

REQUIRE( surgeA.size() == replA.size() );

for( auto i=0; i<surgeA.size(); ++i )
testSusPush( 0.3, 0.7 );
for( auto i=0; i<10; ++i )
{
REQUIRE( replA[i].second == Approx( surgeA[i].second ).margin( 1e-6 ) );
auto s1 = 0.95f * rand() / RAND_MAX + 0.02;
auto s2 = 0.95f * rand() / RAND_MAX + 0.02;
testSusPush( s1, s2 );
}
}

Expand Down

0 comments on commit 1ba7676

Please sign in to comment.