Skip to content

Commit

Permalink
Use "summing" behavior rather than "averaging"
Browse files Browse the repository at this point in the history
In order to support multiple inputs/outputs correctly, all track
channels routed to a single plugin input need to be summed together
rather than averaged, and likewise for all plugin outputs routed to a
single track channel.

This change in behavior is accompanied by a change to the default
connections for mono plugins so that it does not double the audio
amplitude. Only the left track (rather than both) are connected to the
plugin input in this case, matching what REAPER does.

I also updated the unit tests and added a new test specifically to test
the new "summing" behavior.
  • Loading branch information
messmerd committed Jan 12, 2025
1 parent 262e540 commit b34df28
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 138 deletions.
122 changes: 13 additions & 109 deletions include/PluginPinConnector.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ class LMMS_EXPORT PluginPinConnector
class Matrix
{
public:
explicit Matrix(bool isOutput)
: m_isOutput{isOutput}
{
}

auto pins() const -> const PinMap& { return m_pins; }
auto pins(ch_cnt_t trackChannel) const -> const std::vector<BoolModel*>& { return m_pins[trackChannel]; }

Expand All @@ -128,6 +133,8 @@ class LMMS_EXPORT PluginPinConnector
return m_pins[trackChannel][pluginChannel]->value();
}

auto isOutput() const -> bool { return m_isOutput; }

friend class PluginPinConnector;

private:
Expand All @@ -141,6 +148,7 @@ class LMMS_EXPORT PluginPinConnector

PinMap m_pins;
int m_channelCount = 0;
const bool m_isOutput = false;
std::vector<QString> m_channelNames; //!< optional
};

Expand Down Expand Up @@ -263,8 +271,8 @@ public slots:
private:
void updateAllRoutedChannels();

Matrix m_in; //!< LMMS --> Plugin
Matrix m_out; //!< Plugin --> LMMS
Matrix m_in{false}; //!< LMMS --> Plugin
Matrix m_out{true}; //!< Plugin --> LMMS

//! TODO: Move this somewhere else; Will be >= 2 once there is support for adding new track channels
static constexpr std::size_t s_totalTrackChannels = DEFAULT_CHANNELS;
Expand Down Expand Up @@ -312,7 +320,6 @@ inline void PluginPinConnector::Router<layout, SampleT, channelCountIn, channelC

for (std::uint32_t outChannel = 0; outChannel < out.channels(); ++outChannel)
{
mix_ch_t numRouted = 0; // counter for # of in channels routed to the current out channel
SampleType<layout, SampleT>* outPtr = out.buffer(outChannel);

for (std::uint8_t inChannelPairIdx = 0; inChannelPairIdx < inSizeConstrained; ++inChannelPairIdx)
Expand All @@ -333,7 +340,6 @@ inline void PluginPinConnector::Router<layout, SampleT, channelCountIn, channelC
{
outPtr[frame] += convertSample<SampleT>(inPtr[frame].right());
}
++numRouted;
break;
}
case 0b10: // L channel only
Expand All @@ -342,7 +348,6 @@ inline void PluginPinConnector::Router<layout, SampleT, channelCountIn, channelC
{
outPtr[frame] += convertSample<SampleT>(inPtr[frame].left());
}
++numRouted;
break;
}
case 0b11: // Both channels
Expand All @@ -351,24 +356,13 @@ inline void PluginPinConnector::Router<layout, SampleT, channelCountIn, channelC
{
outPtr[frame] += convertSample<SampleT>(inPtr[frame].left() + inPtr[frame].right());
}
numRouted += 2;
break;
}
default:
unreachable();
break;
}
}

// Either no input channels were routed to this output and output stays zeroed,
// or only one channel was routed and normalization is not needed
if (numRouted <= 1) { continue; }

// Normalize output
for (f_cnt_t frame = 0; frame < in.frames; ++frame)
{
outPtr[frame] /= numRouted;
}
}
}

Expand Down Expand Up @@ -424,10 +418,6 @@ inline void PluginPinConnector::Router<layout, SampleT, channelCountIn, channelC
}
}

// Counters for # of in channels routed to the current pair of out channels
ch_cnt_t numRoutedL = 0;
ch_cnt_t numRoutedR = 0;

for (pi_ch_t inChannel = 0; inChannel < in.channels(); ++inChannel)
{
const SampleType<layout, const SampleT>* inPtr = in.buffer(inChannel);
Expand All @@ -437,10 +427,8 @@ inline void PluginPinConnector::Router<layout, SampleT, channelCountIn, channelC
// This input channel could be routed to either left, right, both, or neither output channels
if (m_pc->m_out.enabled(outChannel, inChannel))
{
++numRoutedL;
if (m_pc->m_out.enabled(outChannel + 1, inChannel))
{
++numRoutedR;
for (f_cnt_t frame = 0; frame < inOut.frames; ++frame)
{
outPtr[frame].leftRef() += inPtr[frame];
Expand All @@ -457,7 +445,6 @@ inline void PluginPinConnector::Router<layout, SampleT, channelCountIn, channelC
}
else if (m_pc->m_out.enabled(outChannel + 1, inChannel))
{
++numRoutedR;
for (f_cnt_t frame = 0; frame < inOut.frames; ++frame)
{
outPtr[frame].rightRef() += inPtr[frame];
Expand All @@ -469,7 +456,6 @@ inline void PluginPinConnector::Router<layout, SampleT, channelCountIn, channelC
// This input channel may or may not be routed to the left output channel
if (!m_pc->m_out.enabled(outChannel, inChannel)) { continue; }

++numRoutedL;
for (f_cnt_t frame = 0; frame < inOut.frames; ++frame)
{
outPtr[frame].leftRef() += inPtr[frame];
Expand All @@ -480,45 +466,12 @@ inline void PluginPinConnector::Router<layout, SampleT, channelCountIn, channelC
// This input channel may or may not be routed to the right output channel
if (!m_pc->m_out.enabled(outChannel + 1, inChannel)) { continue; }

++numRoutedR;
for (f_cnt_t frame = 0; frame < inOut.frames; ++frame)
{
outPtr[frame].rightRef() += inPtr[frame];
}
}
}

// If num routed is 0 or 1, either no plugin channels were routed to the output
// and the output stays zeroed, or only one channel was routed and normalization is not needed

if (numRoutedL > 1)
{
if (numRoutedR > 1)
{
// Normalize output - both channels
for (f_cnt_t frame = 0; frame < inOut.frames; ++frame)
{
outPtr[frame].leftRef() /= numRoutedL;
outPtr[frame].rightRef() /= numRoutedR;
}
}
else
{
// Normalize output - left channel
for (f_cnt_t frame = 0; frame < inOut.frames; ++frame)
{
outPtr[frame].leftRef() /= numRoutedL;
}
}
}
else if (numRoutedR > 1)
{
// Normalize output - right channel
for (f_cnt_t frame = 0; frame < inOut.frames; ++frame)
{
outPtr[frame].rightRef() /= numRoutedR;
}
}
};


Expand Down Expand Up @@ -572,31 +525,18 @@ inline void PluginPinConnector::Router<layout, SampleFrame, channelCountIn, chan
// Zero the output buffer - TODO: std::memcpy?
std::fill(out.begin(), out.end(), SampleFrame{});

// Counters for # of in channels routed to the current pair of out channels
mix_ch_t numRoutedL = 0;
mix_ch_t numRoutedR = 0;

const auto samples = in.frames * 2;
sample_t* outPtr = out.data()->data();

/*
* This is essentially a function template with specializations for each
* of the 16 total routing combinations of an input `SampleFrame*` to an
* output `SampleFrame*`. The purpose is to eliminate all branching within
* the inner for-loop in hopes of better performance.
*/
auto route2x2 = [&, samples, outPtr](const sample_t* inPtr, auto enabledPins) {
auto route2x2 = [samples = in.frames * 2, outPtr = out.data()->data()](const sample_t* inPtr, auto enabledPins) {
constexpr auto epL = static_cast<std::uint8_t>(enabledPins() >> 2); // for L out channel
constexpr auto epR = static_cast<std::uint8_t>(enabledPins() & 0b0011); // for R out channel

if constexpr (enabledPins() == 0) { return; }

if constexpr (epL == 0b11) { numRoutedL += 2; }
else if constexpr (epL != 0) { ++numRoutedL; }

if constexpr (epR == 0b11) { numRoutedR += 2; }
else if constexpr (epR != 0) { ++numRoutedR; }

for (f_cnt_t sampleIdx = 0; sampleIdx < samples; sampleIdx += 2)
{
// Route to left output channel
Expand Down Expand Up @@ -656,42 +596,6 @@ inline void PluginPinConnector::Router<layout, SampleFrame, channelCountIn, chan
break;
}
}

// If the number of channels routed to a specific output is <= 1, no normalization is needed,
// otherwise each output sample needs to be divided by the number that were routed.

if (numRoutedL > 1)
{
if (numRoutedR > 1)
{
// Normalize both output channels
for (f_cnt_t sampleIdx = 0; sampleIdx < samples; sampleIdx += 2)
{
outPtr[sampleIdx] /= numRoutedL;
outPtr[sampleIdx + 1] /= numRoutedR;
}
}
else
{
// Normalize left output channel
for (f_cnt_t sampleIdx = 0; sampleIdx < samples; sampleIdx += 2)
{
outPtr[sampleIdx] /= numRoutedL;
}
}
}
else
{
// Either no input channels were routed to either output channel and output stays zeroed,
// or only one channel was routed and normalization is not needed
if (numRoutedR <= 1) { return; }

// Normalize right output channel
for (f_cnt_t sampleIdx = 1; sampleIdx < samples; sampleIdx += 2)
{
outPtr[sampleIdx] /= numRoutedR;
}
}
}

template<AudioDataLayout layout, int channelCountIn, int channelCountOut>
Expand Down Expand Up @@ -729,7 +633,7 @@ inline void PluginPinConnector::Router<layout, SampleFrame, channelCountIn, chan
// Route to left output channel
if constexpr (epL == 0b11)
{
outPtr[sampleIdx] = (inPtr[sampleIdx] + inPtr[sampleIdx + 1]) / 2;
outPtr[sampleIdx] = inPtr[sampleIdx] + inPtr[sampleIdx + 1];
}
else if constexpr (epL == 0b01)
{
Expand All @@ -743,7 +647,7 @@ inline void PluginPinConnector::Router<layout, SampleFrame, channelCountIn, chan
// Route to right output channel
if constexpr (epR == 0b11)
{
outPtr[sampleIdx + 1] = (inPtr[sampleIdx] + inPtr[sampleIdx + 1]) / 2;
outPtr[sampleIdx + 1] = inPtr[sampleIdx] + inPtr[sampleIdx + 1];
}
else if constexpr (epR == 0b01)
{
Expand Down
1 change: 0 additions & 1 deletion plugins/ZynAddSubFx/ZynAddSubFx.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ class ZynAddSubFxRemotePlugin
};


// TODO: Is it always 0 inputs, 2 outputs?
class ZynAddSubFxInstrument
: public AudioPlugin<Instrument, float,
PluginConfig{ .layout = AudioDataLayout::Split, .inputs = 0, .outputs = 2, .customBuffer = true }>
Expand Down
20 changes: 12 additions & 8 deletions src/core/PluginPinConnector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,6 @@ void PluginPinConnector::Matrix::setTrackChannelCount(PluginPinConnector* parent
auto parentModel = parent->parentModel();
assert(parentModel != nullptr);

const bool isOutMatrix = this == &parent->out();

m_pins.resize(count);
for (auto tcIdx = oldSize; tcIdx < count; ++tcIdx)
{
Expand All @@ -263,7 +261,7 @@ void PluginPinConnector::Matrix::setTrackChannelCount(PluginPinConnector* parent
{
const auto name = nameFormat.arg(tcIdx + 1).arg(channelName(pcIdx));
BoolModel* model = channels.emplace_back(new BoolModel{false, parentModel, name});
if (isOutMatrix)
if (isOutput())
{
parentModel->connect(model, &BoolModel::dataChanged, [=]() { parent->updateRoutedChannels(tcIdx); });
}
Expand All @@ -280,7 +278,6 @@ void PluginPinConnector::Matrix::setPluginChannelCount(PluginPinConnector* paren
assert(parentModel != nullptr);

const bool initialSetup = m_channelCount == 0;
const bool isOutMatrix = this == &parent->out();

if (channelCount() < count)
{
Expand All @@ -292,7 +289,7 @@ void PluginPinConnector::Matrix::setPluginChannelCount(PluginPinConnector* paren
{
const auto name = nameFormat.arg(tcIdx + 1).arg(channelName(pcIdx));
BoolModel* model = pluginChannels.emplace_back(new BoolModel{false, parentModel, name});
if (isOutMatrix)
if (isOutput())
{
parentModel->connect(model, &BoolModel::dataChanged, [=]() { parent->updateRoutedChannels(tcIdx); });
}
Expand All @@ -314,7 +311,7 @@ void PluginPinConnector::Matrix::setPluginChannelCount(PluginPinConnector* paren

m_channelCount = count;

if (initialSetup && (!parent->isInstrument() || isOutMatrix))
if (initialSetup && (!parent->isInstrument() || isOutput()))
{
// Set default connections, unless this is the input matrix for an instrument
setDefaultConnections();
Expand All @@ -328,9 +325,16 @@ void PluginPinConnector::Matrix::setDefaultConnections()
switch (channelCount())
{
case 0: break;
case 1:
case 1: // mono
m_pins[0][0]->setValue(true);
m_pins[1][0]->setValue(true);
if (isOutput())
{
// Only the first track channel is routed to mono-input
// plugins, otherwise the pin connector's input-summing behavior
// would cause mono plugins to be louder than stereo ones.
// This behavior matches what REAPER's plug-in pin connector does.
m_pins[1][0]->setValue(true);
}
break;
default: // >= 2
m_pins[0][0]->setValue(true);
Expand Down
Loading

0 comments on commit b34df28

Please sign in to comment.