Linked modulators never process

[matkatmusic]

I created a simple soundfont to test out the modulation system. it has 2 modulators, and the 2nd modulator is linked to the first. in this loop, ok always ends up false when the loop exits. This is easily verified by adding bool result = ok; and a breakpoint:

void ModulatorGroup::process()
{
    // Process the input of the modulators
    foreach (ParameterModulator * modulator, _modulators)
        modulator->processInput();

    // Compute the output of the modulators, as long as everything has not been completed
    // or until a maximum (reached in the case where we have a loop)
    bool ok;
    int count = 0;
    do {
        ok = true;
        foreach (ParameterModulator * modulator, _modulators)
            ok &= modulator->processOutput();
    } while (!ok && count++ < _modulators.count());
    bool result = ok; //if any modulators are linked, this is always false.
}

Somewhere _inputNumber is being set to 2 in the ParameterModulator class, which prevents this line:

bool ParameterModulator::processOutput()
{
    if (_inputCount < _inputNumber)
        return false; // We need to wait

from ever returning true.

it seems that this loop in ModulatorGroup::loadModulators() is causing modulator->setOutput(otherMod); to be called twice for 'modulator'

    foreach (ParameterModulator * modulator, _modulators)
    {
        quint16 output = modulator->getOuputType();
        if (output < 32768)
        {
            // The target is a parameter
            if (_parameters->contains(static_cast<AttributeType>(output)))
                modulator->setOutput(_parameters->value(static_cast<AttributeType>(output)));
        }
        else
        {
            // The target is another modulator
            int indexToFind = output - 32768;
            foreach (ParameterModulator * otherMod, _modulators)
            {
                if (otherMod != modulator && otherMod->getIndex() == indexToFind)
                {
                    modulator->setOutput(otherMod); 
                    break;
                }
            }
        }
    }

if you inspect modulator you'll see that its inputNumber gets set to 2 for the attached soundFont.

ModulationTest.sf2.zip

[matkatmusic]

ok, further digging reveals that the duplicate modulator->setOutput call is because of this function:

void VoiceParam::readDivisionModulators(EltID idDivision)
{
    bool isPrst = idDivision.isPrst();

    // Load global modulators
    EltID id(isPrst ? elementPrst : elementInst, idDivision.indexSf2, idDivision.indexElt);
    QList<ModulatorData> globalModulators;
    _sm->getAllModulators(id, globalModulators);
    if (isPrst)
        _modulatorGroupPrst.loadModulators(globalModulators);
    else
        _modulatorGroupInst.loadModulators(globalModulators);

    // Load division modulators
    QList<ModulatorData> divisionModulators;
    _sm->getAllModulators(idDivision, divisionModulators);
    if (isPrst)
        _modulatorGroupPrst.loadModulators(divisionModulators);
    else
        _modulatorGroupInst.loadModulators(divisionModulators);
}

I removed the instrument-level modulators and made them global preset modulators so only 2 modulators are used. it appears that sm->getAllModulators() is returning the same set of modulators, which is causing the linked modulator to have its _numberInputs set to '2'

ModulationTest.sf2.zip

[matkatmusic]

ok, when testing with the above soundfont that contains only 2 global modulators (and no instrument-level modulators) here is the actual issue in void VoiceParam::readDivisionModulators(EltID idDivision):

    if (isPrst)
        _modulatorGroupPrst.loadModulators(globalModulators);

ModulatorGroup will have the two modulators added via this line:

        if (!overwritten)
            _modulators << new ParameterModulator(modData, _isPrst, _initialKey, _keyForComputation, _velForComputation);

Then those 2 get linked in the lines that follow inside void ModulatorGroup::loadModulators(QList<ModulatorData> &modulators)

THEN when this line is called:

    // Load division modulators
    QList<ModulatorData> divisionModulators;
    _sm->getAllModulators(idDivision, divisionModulators);
    if (isPrst)
        _modulatorGroupPrst.loadModulators(divisionModulators);

it doesn't matter that there are zero divisionModulators. the same loop on line 85 that links the ModulatorGroup::_modulators will be called again, and the linked modulator will be linked again, incrementing its _numberInputs to 2.

[matkatmusic]

Here is a patch.

The only problem with it is that the Modulator that is using another Modulator as its source needs to normalize the value passed to it from the other modulator. otherwise you'll end up with a huge _input1 value.

Subject: [PATCH] Implemented partial fix that successfully computes linked
 modulators

---
 sources/sound_engine/modulatorgroup.cpp   | 2 +-
 sources/sound_engine/parametermodulator.h | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/sources/sound_engine/modulatorgroup.cpp b/sources/sound_engine/modulatorgroup.cpp
index 2c351b9c..4f03c191 100644
--- a/sources/sound_engine/modulatorgroup.cpp
+++ b/sources/sound_engine/modulatorgroup.cpp
@@ -98,7 +98,7 @@ void ModulatorGroup::loadModulators(QList<ModulatorData> &modulators)
             int indexToFind = output - 32768;
             foreach (ParameterModulator * otherMod, _modulators)
             {
-                if (otherMod != modulator && otherMod->getIndex() == indexToFind)
+                if (modulator->getOutput() == nullptr && otherMod != modulator && otherMod->getIndex() == indexToFind)
                 {
                     modulator->setOutput(otherMod);
                     break;
diff --git a/sources/sound_engine/parametermodulator.h b/sources/sound_engine/parametermodulator.h
index d0091bc6..7d9faa37 100644
--- a/sources/sound_engine/parametermodulator.h
+++ b/sources/sound_engine/parametermodulator.h
@@ -44,6 +44,7 @@ public:
     // Set the output
     void setOutput(ModulatedParameter * parameter);
     void setOutput(ParameterModulator * modulator);
+    ParameterModulator* getOutput() { return _outputModulator; }

     // Compute the input based on midi values
     void processInput();
@@ -67,7 +68,7 @@ private:
     bool _computed;
     double _input1, _input2;
     ModulatedParameter * _outputParameter;
-    ParameterModulator * _outputModulator;
+    ParameterModulator * _outputModulator = nullptr;
     bool _isPrst;

     // Distinction between the initial key that triggered the sound (still useful for the aftertouch)
-- 
2.27.0

[mbenkmann]

I'd like to add that the normalization needs to take into account that multiple modulators can be input to the same target modulator. They need to be summed up and the normalization needs to take the whole range of the sum into account. E.g. Mod 1 has an output range of -100 to 100 and Mod 2 has an output range of 0 to 50, both feed into Mod 3, then Mod 3 needs to normalize from a possible input range of -100 to 150

[mbenkmann]

Looking at the code it seems like it is currently not prepared to handle normalization of linked modulators. I see here

https://github.com/davy7125/polyphone/blob/75a5e88d361f6e141dbad17673e1fc25291cae10/sources/sound_engine/parametermodulator.cpp#L99

that the value from one modulator is simply sent on to another. What is needed is to expand this to send the min and max for normalization, too, e.g.

_outputModulator->setInput(result, min, max);

and setInput() will need to sum up min and max in addition to result. Finally when the result is evaluated it will need to be normalized as (X-sum(min))/(sum(max)-sum(min)) to bring it into the 0 to 1.0 range. It's probably easiest to handle that directly in setInput() https://github.com/davy7125/polyphone/blob/75a5e88d361f6e141dbad17673e1fc25291cae10/sources/sound_engine/parametermodulator.cpp#L115

like so

  _input1 += value;
  _minSum += min;
  _maxSum += max;
  _inputCount++;
   if (_inputCount == _inputNumber) {
       double range = _maxSum - _minSum;
       if (range == 0) // avoid division by 0
          range = 1;
       _input1 = (_input1 - _minSum) / range;
   }

Regarding min and max for a modulator I'd like to mention that currently polyphone always prints "Add From:0 To:0" when the target is a linked modulator when instead it should be computed as an unadjusted number (basically like "Tuning(cents)" without the "cents").

[davy7125]

@matkatmusic, @mbenkmann thank you very much for your analyze. It's been very helpful