diff --git a/include/PluginPinConnector.h b/include/PluginPinConnector.h index ba03ea1b424..52043d057a8 100644 --- a/include/PluginPinConnector.h +++ b/include/PluginPinConnector.h @@ -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& { return m_pins[trackChannel]; } @@ -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: @@ -141,6 +148,7 @@ class LMMS_EXPORT PluginPinConnector PinMap m_pins; int m_channelCount = 0; + const bool m_isOutput = false; std::vector m_channelNames; //!< optional }; @@ -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; @@ -312,7 +320,6 @@ inline void PluginPinConnector::Router* outPtr = out.buffer(outChannel); for (std::uint8_t inChannelPairIdx = 0; inChannelPairIdx < inSizeConstrained; ++inChannelPairIdx) @@ -333,7 +340,6 @@ inline void PluginPinConnector::Router(inPtr[frame].right()); } - ++numRouted; break; } case 0b10: // L channel only @@ -342,7 +348,6 @@ inline void PluginPinConnector::Router(inPtr[frame].left()); } - ++numRouted; break; } case 0b11: // Both channels @@ -351,7 +356,6 @@ inline void PluginPinConnector::Router(inPtr[frame].left() + inPtr[frame].right()); } - numRouted += 2; break; } default: @@ -359,16 +363,6 @@ inline void PluginPinConnector::Router* inPtr = in.buffer(inChannel); @@ -437,10 +427,8 @@ inline void PluginPinConnector::Routerm_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]; @@ -457,7 +445,6 @@ inline void PluginPinConnector::Routerm_out.enabled(outChannel + 1, inChannel)) { - ++numRoutedR; for (f_cnt_t frame = 0; frame < inOut.frames; ++frame) { outPtr[frame].rightRef() += inPtr[frame]; @@ -469,7 +456,6 @@ inline void PluginPinConnector::Routerm_out.enabled(outChannel, inChannel)) { continue; } - ++numRoutedL; for (f_cnt_t frame = 0; frame < inOut.frames; ++frame) { outPtr[frame].leftRef() += inPtr[frame]; @@ -480,45 +466,12 @@ inline void PluginPinConnector::Routerm_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; - } - } }; @@ -572,31 +525,18 @@ inline void PluginPinConnector::Routerdata(); - /* * 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(enabledPins() >> 2); // for L out channel constexpr auto epR = static_cast(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 @@ -656,42 +596,6 @@ inline void PluginPinConnector::Router 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 @@ -729,7 +633,7 @@ inline void PluginPinConnector::Router diff --git a/src/core/PluginPinConnector.cpp b/src/core/PluginPinConnector.cpp index e6c711fcfc9..87bc4b250e3 100644 --- a/src/core/PluginPinConnector.cpp +++ b/src/core/PluginPinConnector.cpp @@ -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) { @@ -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); }); } @@ -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) { @@ -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); }); } @@ -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(); @@ -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); diff --git a/tests/src/core/PluginPinConnectorTest.cpp b/tests/src/core/PluginPinConnectorTest.cpp index aff02c5bb1a..7ec47c4d48b 100644 --- a/tests/src/core/PluginPinConnectorTest.cpp +++ b/tests/src/core/PluginPinConnectorTest.cpp @@ -246,12 +246,12 @@ private slots: // In Out // _ _ // |X| |X| - // |X| |X| + // | | |X| // - - auto pc1x1 = PluginPinConnector{1, 1, false, &model}; QCOMPARE(pc1x1.in().enabled(0, 0), true); - QCOMPARE(pc1x1.in().enabled(1, 0), true); + QCOMPARE(pc1x1.in().enabled(1, 0), false); QCOMPARE(pc1x1.out().enabled(0, 0), true); QCOMPARE(pc1x1.out().enabled(1, 0), true); @@ -261,12 +261,12 @@ private slots: // In Out // _ _______ // |X| |X| | | | - // |X| | |X| | | + // | | | |X| | | // - ------- auto pc1x4 = PluginPinConnector{1, 4, false, &model}; QCOMPARE(pc1x4.in().enabled(0, 0), true); - QCOMPARE(pc1x4.in().enabled(1, 0), true); + QCOMPARE(pc1x4.in().enabled(1, 0), false); QCOMPARE(pc1x4.out().enabled(0, 0), true); QCOMPARE(pc1x4.out().enabled(0, 1), false); @@ -362,13 +362,18 @@ private slots: auto coreBus = getCoreBus(); SampleFrame* trackChannels = coreBus.bus[0]; // channels 0/1 - // Downmix stereo to mono plugin input, upmix mono plugin output to stereo + // Use left channel as plugin input, upmix mono plugin output to stereo // In Out // _ _ // |X| |X| - // |X| |X| + // | | |X| // - - + // NOTE: If both channels were connected to the mono input, the signals would + // be summed together and the amplitude would be doubled which is + // undesirable, so the pin connector uses only the left channel as the + // plugin input, following what REAPER does by default. + // Data on frames 0, 1, and 33 trackChannels[0].setLeft(123.f); trackChannels[0].setRight(321.f); @@ -386,10 +391,10 @@ private slots: auto router = pc.getRouter(); router.routeToPlugin(coreBus, ins); - // Check that plugin inputs have data on frames 0, 1, and 33 (should be mixed down to mono) - QCOMPARE(ins.buffer(0)[0], (123.f + 321.f) / 2); - QCOMPARE(ins.buffer(0)[1], (456.f + 654.f) / 2); - QCOMPARE(ins.buffer(0)[33], (789.f + 987.f) / 2); + // Check that plugin inputs have data on frames 0, 1, and 33 (should be left channel's data) + QCOMPARE(ins.buffer(0)[0], 123.f); + QCOMPARE(ins.buffer(0)[1], 456.f); + QCOMPARE(ins.buffer(0)[33], 789.f); // Do work of processImpl - in this case it doubles the amplitude transformBuffer(ins, outs, [](auto s) { return s * 2; }); @@ -398,20 +403,20 @@ private slots: auto coreBufferExpected = std::vector(MaxFrames); auto coreBufferPtrExpected = coreBufferExpected.data(); auto coreBusExpected = CoreAudioBusMut{&coreBufferPtrExpected, 1, MaxFrames}; - coreBusExpected.bus[0][0] = SampleFrame{123.f + 321.f, 123.f + 321.f}; - coreBusExpected.bus[0][1] = SampleFrame{456.f + 654.f, 456.f + 654.f}; - coreBusExpected.bus[0][33] = SampleFrame{789.f + 987.f, 789.f + 987.f}; + coreBusExpected.bus[0][0] = SampleFrame{123.f * 2, 123.f * 2}; + coreBusExpected.bus[0][1] = SampleFrame{456.f * 2, 456.f * 2}; + coreBusExpected.bus[0][33] = SampleFrame{789.f * 2, 789.f * 2}; // Route from plugin back to Core router.routeFromPlugin(outs, coreBus); - // Check that result is mono mix with doubled amplitude - QCOMPARE(coreBus.bus[0][0].left(), 123.f + 321.f); - QCOMPARE(coreBus.bus[0][0].right(), 123.f + 321.f); - QCOMPARE(coreBus.bus[0][1].left(), 456.f + 654.f); - QCOMPARE(coreBus.bus[0][1].right(), 456.f + 654.f); - QCOMPARE(coreBus.bus[0][33].left(), 789.f + 987.f); - QCOMPARE(coreBus.bus[0][33].right(), 789.f + 987.f); + // Check that result is the original left track channel with doubled amplitude + QCOMPARE(coreBus.bus[0][0].left(), 123.f * 2); + QCOMPARE(coreBus.bus[0][0].right(), 123.f * 2); + QCOMPARE(coreBus.bus[0][1].left(), 456.f * 2); + QCOMPARE(coreBus.bus[0][1].right(), 456.f * 2); + QCOMPARE(coreBus.bus[0][33].left(), 789.f * 2); + QCOMPARE(coreBus.bus[0][33].right(), 789.f * 2); // Test the rest of the buffer compareBuffers(coreBus, coreBusExpected); @@ -505,6 +510,13 @@ private slots: // |X| | |X| | // | |X| | | | // --- --- + pc.in().pins(0)[0]->setValue(true); + pc.in().pins(0)[1]->setValue(false); + pc.in().pins(1)[0]->setValue(false); + pc.in().pins(1)[1]->setValue(true); + pc.out().pins(0)[0]->setValue(true); + pc.out().pins(0)[1]->setValue(false); + pc.out().pins(1)[0]->setValue(false); pc.out().pins(1)[1]->setValue(false); // Data on frames 0, 1, and 33 @@ -626,6 +638,81 @@ private slots: // Test the rest of the buffer compareBuffers(coreBus, coreBusExpected); } + + //! Verifies correct signal summing when routing a 1x2 non-interleaved (split) plugin + void Routing_Split1x2_Sum() + { + using namespace lmms; + + // Setup + auto model = Model{nullptr}; + auto pc = PluginPinConnector{1, 2, false, &model}; + auto coreBus = getCoreBus(); + SampleFrame* trackChannels = coreBus.bus[0]; // channels 0/1 + + // Sum both track channels together for plugin input, and sum the + // two plugin output channels together for the left track channel output + // In Out + // _ ___ + // |X| |X|X| + // |X| | | | + // - --- + pc.in().pins(0)[0]->setValue(true); + pc.in().pins(1)[0]->setValue(true); + pc.out().pins(0)[0]->setValue(true); + pc.out().pins(0)[1]->setValue(true); + pc.out().pins(1)[0]->setValue(false); + pc.out().pins(1)[1]->setValue(false); + + // Data on frames 0, 1, and 33 + trackChannels[0].setLeft(123.f); + trackChannels[0].setRight(321.f); + trackChannels[1].setLeft(456.f); + trackChannels[1].setRight(654.f); + trackChannels[33].setLeft(789.f); + trackChannels[33].setRight(987.f); + + // Plugin input and output buffers + auto bufferSplit1x2 = AudioPluginBufferDefaultImpl{}; + auto ins = bufferSplit1x2.inputBuffer(); + auto outs = bufferSplit1x2.outputBuffer(); + + // Route to plugin + auto router = pc.getRouter(); + router.routeToPlugin(coreBus, ins); + + // Check that plugin inputs have data on frames 0, 1, and 33 (should be both track channels summed together) + QCOMPARE(ins.buffer(0)[0], 123.f + 321.f); + QCOMPARE(ins.buffer(0)[1], 456.f + 654.f); + QCOMPARE(ins.buffer(0)[33], 789.f + 987.f); + + // Do work of processImpl - in this case it does nothing (passthrough) + const auto process = [](auto s) { return s; }; + std::transform(ins.buffer(0), ins.buffer(0) + ins.frames(), outs.buffer(0), process); + std::transform(ins.buffer(0), ins.buffer(0) + ins.frames(), outs.buffer(1), process); + + // Construct buffer with the expected core bus result + auto coreBufferExpected = std::vector(MaxFrames); + auto coreBufferPtrExpected = coreBufferExpected.data(); + auto coreBusExpected = CoreAudioBusMut{&coreBufferPtrExpected, 1, MaxFrames}; + coreBusExpected.bus[0][0] = SampleFrame{(123.f + 321.f) * 2, 321.f}; + coreBusExpected.bus[0][1] = SampleFrame{(456.f + 654.f) * 2, 654.f}; + coreBusExpected.bus[0][33] = SampleFrame{(789.f + 987.f) * 2, 987.f}; + + // Route from plugin back to Core + router.routeFromPlugin(outs, coreBus); + + // Check that result is the two original track channels added together then doubled + QCOMPARE(coreBus.bus[0][0].left(), (123.f + 321.f) * 2); + QCOMPARE(coreBus.bus[0][0].right(), 321.f); + QCOMPARE(coreBus.bus[0][1].left(), (456.f + 654.f) * 2); + QCOMPARE(coreBus.bus[0][1].right(), 654.f); + QCOMPARE(coreBus.bus[0][33].left(), (789.f + 987.f) * 2); + QCOMPARE(coreBus.bus[0][33].right(), 987.f); + + // Test the rest of the buffer + compareBuffers(coreBus, coreBusExpected); + } }; QTEST_GUILESS_MAIN(PluginPinConnectorTest)