Skip to content

Effect enable fix #1176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
daschuer merged 4 commits into from
Feb 11, 2017
Merged

Effect enable fix #1176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a383d71
Use SampleBuffer in EngineEffectChain
daschuer Feb 4, 2017
7afb6b7
De-click the effects when turing to fully dry.
daschuer Feb 5, 2017
038dfcf
De-click effect enable and disbale actions
daschuer Feb 5, 2017
32f3c9e
Do not maloc SampleBuffer, it malocs the required memory internaly.
daschuer Feb 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 16 additions & 17 deletions src/effects/native/biquadfullkilleqeffect.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,11 @@ EffectManifest BiquadFullKillEQEffect::getManifest() {
}

BiquadFullKillEQEffectGroupState::BiquadFullKillEQEffectGroupState()
: m_oldLowBoost(0),
: m_pLowBuf(MAX_BUFFER_LEN),
m_pBandBuf(MAX_BUFFER_LEN),
m_pHighBuf(MAX_BUFFER_LEN),
m_tempBuf(MAX_BUFFER_LEN),
m_oldLowBoost(0),
m_oldMidBoost(0),
m_oldHighBoost(0),
m_oldLowKill(0),
Expand All @@ -153,11 +157,6 @@ BiquadFullKillEQEffectGroupState::BiquadFullKillEQEffectGroupState()
m_groupDelay(0),
m_oldSampleRate(kStartupSamplerate) {

m_pLowBuf = std::make_unique<SampleBuffer>(MAX_BUFFER_LEN);
m_pBandBuf = std::make_unique<SampleBuffer>(MAX_BUFFER_LEN);
m_pHighBuf = std::make_unique<SampleBuffer>(MAX_BUFFER_LEN);
m_pTempBuf = std::make_unique<SampleBuffer>(MAX_BUFFER_LEN);

// Initialize the filters with default parameters

m_lowBoost = std::make_unique<EngineFilterBiquad1Peaking>(
Expand Down Expand Up @@ -280,21 +279,21 @@ void BiquadFullKillEQEffect::processChannel(

if (activeFilters % 2 == 0) {
inBuffer.append(pInput);
outBuffer.append(pState->m_pTempBuf->data());
outBuffer.append(pState->m_tempBuf.data());

inBuffer.append(pState->m_pTempBuf->data());
inBuffer.append(pState->m_tempBuf.data());
outBuffer.append(pOutput);

inBuffer.append(pOutput);
outBuffer.append(pState->m_pTempBuf->data());
outBuffer.append(pState->m_tempBuf.data());

inBuffer.append(pState->m_pTempBuf->data());
inBuffer.append(pState->m_tempBuf.data());
outBuffer.append(pOutput);

inBuffer.append(pOutput);
outBuffer.append(pState->m_pTempBuf->data());
outBuffer.append(pState->m_tempBuf.data());

inBuffer.append(pState->m_pTempBuf->data());
inBuffer.append(pState->m_tempBuf.data());
outBuffer.append(pOutput);
}
else
Expand All @@ -303,19 +302,19 @@ void BiquadFullKillEQEffect::processChannel(
outBuffer.append(pOutput);

inBuffer.append(pOutput);
outBuffer.append(pState->m_pTempBuf->data());
outBuffer.append(pState->m_tempBuf.data());

inBuffer.append(pState->m_pTempBuf->data());
inBuffer.append(pState->m_tempBuf.data());
outBuffer.append(pOutput);

inBuffer.append(pOutput);
outBuffer.append(pState->m_pTempBuf->data());
outBuffer.append(pState->m_tempBuf.data());

inBuffer.append(pState->m_pTempBuf->data());
inBuffer.append(pState->m_tempBuf.data());
outBuffer.append(pOutput);

inBuffer.append(pOutput);
outBuffer.append(pState->m_pTempBuf->data());
outBuffer.append(pState->m_tempBuf.data());
}

int bufIndex = 0;
Expand Down
8 changes: 4 additions & 4 deletions src/effects/native/biquadfullkilleqeffect.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ class BiquadFullKillEQEffectGroupState final {
std::unique_ptr<EngineFilterBiquad1Peaking> m_midKill;
std::unique_ptr<EngineFilterBiquad1HighShelving> m_highKill;
std::unique_ptr<LVMixEQEffectGroupState<EngineFilterBessel4Low>> m_lvMixIso;
std::unique_ptr<SampleBuffer> m_pLowBuf;
std::unique_ptr<SampleBuffer> m_pBandBuf;
std::unique_ptr<SampleBuffer> m_pHighBuf;
std::unique_ptr<SampleBuffer> m_pTempBuf;

SampleBuffer m_pLowBuf;
SampleBuffer m_pBandBuf;
SampleBuffer m_pHighBuf;
SampleBuffer m_tempBuf;

double m_oldLowBoost;
double m_oldMidBoost;
Expand Down
27 changes: 13 additions & 14 deletions src/effects/native/threebandbiquadeqeffect.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ EffectManifest ThreeBandBiquadEQEffect::getManifest() {
}

ThreeBandBiquadEQEffectGroupState::ThreeBandBiquadEQEffectGroupState()
: m_oldLowBoost(0),
: m_tempBuf(MAX_BUFFER_LEN),
m_oldLowBoost(0),
m_oldMidBoost(0),
m_oldHighBoost(0),
m_oldLowCut(0),
Expand All @@ -146,8 +147,6 @@ ThreeBandBiquadEQEffectGroupState::ThreeBandBiquadEQEffectGroupState()
m_highFreqCorner(0),
m_oldSampleRate(kStartupSamplerate) {

m_pTempBuf = std::make_unique<SampleBuffer>(MAX_BUFFER_LEN);

// Initialize the filters with default parameters

m_lowBoost = std::make_unique<EngineFilterBiquad1Peaking>(
Expand Down Expand Up @@ -267,21 +266,21 @@ void ThreeBandBiquadEQEffect::processChannel(

if (activeFilters % 2 == 0) {
inBuffer.append(pInput);
outBuffer.append(pState->m_pTempBuf->data());
outBuffer.append(pState->m_tempBuf.data());

inBuffer.append(pState->m_pTempBuf->data());
inBuffer.append(pState->m_tempBuf.data());
outBuffer.append(pOutput);

inBuffer.append(pOutput);
outBuffer.append(pState->m_pTempBuf->data());
outBuffer.append(pState->m_tempBuf.data());

inBuffer.append(pState->m_pTempBuf->data());
inBuffer.append(pState->m_tempBuf.data());
outBuffer.append(pOutput);

inBuffer.append(pOutput);
outBuffer.append(pState->m_pTempBuf->data());
outBuffer.append(pState->m_tempBuf.data());

inBuffer.append(pState->m_pTempBuf->data());
inBuffer.append(pState->m_tempBuf.data());
outBuffer.append(pOutput);
}
else
Expand All @@ -290,19 +289,19 @@ void ThreeBandBiquadEQEffect::processChannel(
outBuffer.append(pOutput);

inBuffer.append(pOutput);
outBuffer.append(pState->m_pTempBuf->data());
outBuffer.append(pState->m_tempBuf.data());

inBuffer.append(pState->m_pTempBuf->data());
inBuffer.append(pState->m_tempBuf.data());
outBuffer.append(pOutput);

inBuffer.append(pOutput);
outBuffer.append(pState->m_pTempBuf->data());
outBuffer.append(pState->m_tempBuf.data());

inBuffer.append(pState->m_pTempBuf->data());
inBuffer.append(pState->m_tempBuf.data());
outBuffer.append(pOutput);

inBuffer.append(pOutput);
outBuffer.append(pState->m_pTempBuf->data());
outBuffer.append(pState->m_tempBuf.data());
}

int bufIndex = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/effects/native/threebandbiquadeqeffect.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class ThreeBandBiquadEQEffectGroupState final {
std::unique_ptr<EngineFilterBiquad1Peaking> m_lowCut;
std::unique_ptr<EngineFilterBiquad1Peaking> m_midCut;
std::unique_ptr<EngineFilterBiquad1HighShelving> m_highCut;
std::unique_ptr<SampleBuffer> m_pTempBuf;
SampleBuffer m_tempBuf;
double m_oldLowBoost;
double m_oldMidBoost;
double m_oldHighBoost;
Expand Down
2 changes: 2 additions & 0 deletions src/engine/effects/engineeffect.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,14 @@ void EngineEffect::process(const ChannelHandle& handle,
if (!m_effectRampsFromDry) {
// the effect does not fade, so we care for it
if (effectiveEnableState == EffectProcessor::DISABLING) {
DEBUG_ASSERT(pInput != pOutput); // Fade to dry only works if pInput is not touched by pOutput
// Fade out (fade to dry signal)
SampleUtil::copy2WithRampingGain(pOutput,
pInput, 0.0, 1.0,
pOutput, 1.0, 0.0,
numSamples);
} else if (effectiveEnableState == EffectProcessor::ENABLING) {
DEBUG_ASSERT(pInput != pOutput); // Fade to dry only works if pInput is not touched by pOutput
// Fade in (fade to wet signal)
SampleUtil::copy2WithRampingGain(pOutput,
pInput, 1.0, 0.0,
Expand Down
129 changes: 54 additions & 75 deletions src/engine/effects/engineeffectchain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ EngineEffectChain::EngineEffectChain(const QString& id)
m_enableState(EffectProcessor::ENABLED),
m_insertionType(EffectChain::INSERT),
m_dMix(0),
m_pBuffer(SampleUtil::alloc(MAX_BUFFER_LEN)) {
m_buffer1(MAX_BUFFER_LEN),
m_buffer2(MAX_BUFFER_LEN) {
// Try to prevent memory allocation.
m_effects.reserve(256);
}

EngineEffectChain::~EngineEffectChain() {
SampleUtil::free(m_pBuffer);
}

bool EngineEffectChain::addEffect(EngineEffect* pEffect, int iIndex) {
Expand Down Expand Up @@ -165,98 +165,77 @@ void EngineEffectChain::process(const ChannelHandle& handle,

EffectProcessor::EnableState effectiveEnableState = channel_info.enable_state;

if (channel_info.enable_state == EffectProcessor::DISABLING) {
channel_info.enable_state = EffectProcessor::DISABLED;
Copy link
Copy Markdown
Contributor

@Be-ing Be-ing Feb 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be outside the scope of this PR, but temporal effects like echo and reverb should continue to output what is left in their buffers after their enable switch is turned off to have a smooth tail out of the effect. This is really helpful for transitioning between tracks that are much different tempos. To do this, I think the effect needs to be able to handle the disabling process itself and tell the engine when it is disabled.

This is currently possible by turning the send knob (which is linked to the metaknob by default) all the way down, but it would be easier if this could be accomplished just by pressing the enable switch of the effect.

Copy link
Copy Markdown
Contributor

@JosepMaJAZ JosepMaJAZ Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm.. What is done in music composing sometimes is that the effects run all the time, and the input volume to the effect is controlled.

Copy link
Copy Markdown
Contributor

@Be-ing Be-ing Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effects should only use the CPU if they need to.

Copy link
Copy Markdown
Member Author

@daschuer daschuer Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue was discussed almost completely here:
https://bugs.launchpad.net/mixxx/+bug/1481170

Copy link
Copy Markdown
Contributor

@Be-ing Be-ing Feb 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I outlined a general idea at the end of the comments on that bug, but there are details that remain unresolved. Anyway, this PR does not introduce the issue, so there's no need for this PR needs to solve it.

} else if (channel_info.enable_state == EffectProcessor::ENABLING) {
channel_info.enable_state = EffectProcessor::ENABLED;
}

if (m_enableState == EffectProcessor::DISABLING) {
effectiveEnableState = EffectProcessor::DISABLING;
m_enableState = EffectProcessor::DISABLED;
} else if (m_enableState == EffectProcessor::ENABLING) {
effectiveEnableState = EffectProcessor::ENABLING;
m_enableState = EffectProcessor::ENABLED;
}

// At this point either the chain and channel are enabled or we are ramping
// out. If we are ramping out then ramp to 0 instead of m_dMix.
CSAMPLE wet_gain = m_dMix;
CSAMPLE wet_gain_old = channel_info.old_gain;

// INSERT mode: output = input * (1-wet) + effect(input) * wet
if (m_insertionType == EffectChain::INSERT) {
if (wet_gain_old == 1.0 && wet_gain == 1.0) {
// Fully wet, no ramp, insert optimization. No temporary buffer needed.
for (int i = 0; i < m_effects.size(); ++i) {
EngineEffect* pEffect = m_effects[i];
if (pEffect == NULL || !pEffect->enabled()) {
continue;
}
pEffect->process(handle, pInOut, pInOut,
numSamples, sampleRate,
effectiveEnableState, groupFeatures);
}
} else if (wet_gain_old == 0.0 && wet_gain == 0.0) {
// Fully dry, no ramp, insert optimization. No action is needed
} else {
// Clear scratch buffer.
SampleUtil::clear(m_pBuffer, numSamples);
if (wet_gain_old == 0.0 && wet_gain == 0.0) {
// Fully dry, no ramp, insert optimization. No action is needed
return;
} else if (wet_gain_old != 0.0 && wet_gain == 0.0) {
// Tell the effect that this is the last call before disabling
effectiveEnableState = EffectProcessor::DISABLING;
}

// Chain each effect
bool anyProcessed = false;
for (int i = 0; i < m_effects.size(); ++i) {
EngineEffect* pEffect = m_effects[i];
if (pEffect == NULL || !pEffect->enabled()) {
continue;
}
const CSAMPLE* pIntermediateInput = (anyProcessed) ? m_pBuffer : pInOut;
CSAMPLE* pIntermediateOutput = m_pBuffer;
pEffect->process(handle, pIntermediateInput, pIntermediateOutput,
numSamples, sampleRate,
effectiveEnableState, groupFeatures);
anyProcessed = true;
}
// Ramping code inside the effects need to access the original samples
// after writing to the output buffer. This requires not to use the same buffer
// for in and output:
int enabledEffectCount = 0;
CSAMPLE* pIntermediateInput = pInOut;
CSAMPLE* pIntermediateOutput = m_buffer1.data();

if (anyProcessed) {
// m_pBuffer now contains the fully wet output.
// TODO(rryan): benchmark applyGain followed by addWithGain versus
// copy2WithGain.
SampleUtil::copy2WithRampingGain(
pInOut, pInOut, 1.0 - wet_gain_old, 1.0 - wet_gain,
m_pBuffer, wet_gain_old, wet_gain, numSamples);
}
for (EngineEffect* pEffect: m_effects) {
if (pEffect == NULL || !pEffect->enabled()) {
continue;
}
} else { // SEND mode: output = input + effect(input) * wet
// Clear scratch buffer.
SampleUtil::applyGain(m_pBuffer, 0.0, numSamples);

// Chain each effect
bool anyProcessed = false;
for (int i = 0; i < m_effects.size(); ++i) {
EngineEffect* pEffect = m_effects[i];
if (pEffect == NULL || !pEffect->enabled()) {
continue;
}
const CSAMPLE* pIntermediateInput = (i == 0) ? pInOut : m_pBuffer;
CSAMPLE* pIntermediateOutput = m_pBuffer;
pEffect->process(handle, pIntermediateInput,
pIntermediateOutput, numSamples, sampleRate,
effectiveEnableState, groupFeatures);
anyProcessed = true;
pEffect->process(
handle,
pIntermediateInput, pIntermediateOutput,
numSamples, sampleRate,
effectiveEnableState, groupFeatures);

++enabledEffectCount;
if (enabledEffectCount % 2) {
pIntermediateInput = m_buffer1.data();
pIntermediateOutput = m_buffer2.data();
} else {
pIntermediateInput = m_buffer2.data();
pIntermediateOutput = m_buffer1.data();
}
}

if (anyProcessed) {
// m_pBuffer now contains the fully wet output.
SampleUtil::addWithRampingGain(pInOut, m_pBuffer,
wet_gain_old, wet_gain, numSamples);
if (enabledEffectCount > 0) {
if (m_insertionType == EffectChain::INSERT) {
// INSERT mode: output = input * (1-wet) + effect(input) * wet
SampleUtil::copy2WithRampingGain(
pInOut,
pInOut, 1.0 - wet_gain_old, 1.0 - wet_gain,
pIntermediateInput, wet_gain_old, wet_gain,
numSamples);
} else {
// SEND mode: output = input + effect(input) * wet
SampleUtil::addWithRampingGain(
pInOut,
pIntermediateInput, wet_gain_old, wet_gain,
numSamples);
}
}

// Update ChannelStatus with the latest values.
channel_info.old_gain = wet_gain;

if (m_enableState == EffectProcessor::DISABLING) {
m_enableState = EffectProcessor::DISABLED;
} else if (m_enableState == EffectProcessor::ENABLING) {
m_enableState = EffectProcessor::ENABLED;
}

if (channel_info.enable_state == EffectProcessor::DISABLING) {
channel_info.enable_state = EffectProcessor::DISABLED;
} else if (channel_info.enable_state == EffectProcessor::ENABLING) {
channel_info.enable_state = EffectProcessor::ENABLED;
}
}
5 changes: 4 additions & 1 deletion src/engine/effects/engineeffectchain.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#include "util/class.h"
#include "util/types.h"
#include "util/samplebuffer.h"
#include "util/memory.h"
#include "engine/channelhandle.h"
#include "engine/effects/message.h"
#include "engine/effects/groupfeaturestate.h"
Expand Down Expand Up @@ -64,7 +66,8 @@ class EngineEffectChain : public EffectsRequestHandler {
EffectChain::InsertionType m_insertionType;
CSAMPLE m_dMix;
QList<EngineEffect*> m_effects;
CSAMPLE* m_pBuffer;
SampleBuffer m_buffer1;
SampleBuffer m_buffer2;
ChannelHandleMap<ChannelStatus> m_channelStatus;

DISALLOW_COPY_AND_ASSIGN(EngineEffectChain);
Expand Down