AudioProcessorGraph && ChangeBroadcaster

Ah! doh! I went to a lot of trouble to make sure it was only called the minimum number of times, but then had it called indirectly by mistake.

Can you sanity-check that changing line 1020 from

return ! isConnected (c);

to

return true;

sorts it out?

…actually, looking at it again, I think I was using the wrong data structure. I’ve changed it now to use a std::unordered_map, which I think should make it really smooth - would be interested in how your pathological case performs…

That’s a lot better… much closer to mine on a small kit… but on a large preset still about 40% slower

The Small kit has less connections with 197 Nodes w/ 291 Connections, the Large preset with full bleed has 700 Nodes with 1,691 Connections.

I’ll do some profiling…

Thanks,

Rail

Here’s a profile of the init section of my plugin where the Standalone App instantiates my Processor and it initializes the Mixer, etc… We can see that your new buildRenderingSequence() is slower than the original:

original:

new:

Cheers,

Rail

Which leads to if I only build the RenderSequenceFloat in buildRenderingSequence() I beat or equal my code’s speed*… perhaps if we could figure out the AudioBuffer FloatType we could set a flag for the graph??

*If you replace the MessageManagerLock in buildRenderingSequence() with a ScopedLock… otherwise you get a performance hit (around 18%)… I haven’t run into any issues with the change.

Rail

1 Like

So other than the previous changes I had to make I had to change AudioProcessorGraph:: getConnection() (which I use for debugging to log the graph) to:

Definition:

/** Returns a pointer to one of the connections in the graph. */
const Connection* getConnection (int index) const;

Implementation:

 const AudioProcessorGraph::Connection* AudioProcessorGraph::getConnection (int index) const
{
    std::vector<Connection> tmp (connections.begin(), connections.end());

    std::sort (tmp.begin(), tmp.end());

    return tmp.data() + index;
}

Thanks,

Rail

Hmm. I wonder if I just need to re-think the whole approach and give the nodes pointers to their connection objects rather than storing the connections in a container. That could magically speed up all the hotspots that you’re hitting… I might give that a try later.

That would be cool.

Can I ask what was the cause of the original issue of this thread?

Cheers,

Rail

I honestly have no idea… With the new version where I added the ChangeBroadcaster, does that work OK for you?

Yeah - it no longer causes the error I was getting by simply adding the ChangeBroadcaster

Weird :slight_smile:

Cheers,

Rail

I’m going to just throw these in here so they don’t get lost in the noise:

class JUCE_API  Node   : public ReferenceCountedObject
{
public:
    //==============================================================================
    :

    void setBypass (bool bypass)        { bypassed = bypass;    }
    bool isBypassed() const noexcept    { return bypassed;      }

    //==============================================================================
    /** A convenient typedef for referring to a pointer to a node object. */
    typedef ReferenceCountedObjectPtr<Node> Ptr;

private:
    //==============================================================================
    friend class AudioProcessorGraph;

    const ScopedPointer<AudioProcessor> processor;
    bool isPrepared = false;
    bool bypassed = false;

    :

    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (Node)
};

 struct ProcessOp   : public RenderingOp
    {
    :

     void callProcess (AudioBuffer<float>& buffer, MidiBuffer& midiMessages)
         {
             if (processor.isUsingDoublePrecision())
             {
                 tempBufferDouble.makeCopyOf (buffer, true);
        
                 if (node->isBypassed())
                     processor.processBlockBypassed (tempBufferDouble, midiMessages);
                 else
                     processor.processBlock (tempBufferDouble, midiMessages);
        
                 buffer.makeCopyOf (tempBufferDouble, true);
             }
              else
             {
                 if (node->isBypassed())
                     processor.processBlockBypassed (buffer, midiMessages);
                 else
                     processor.processBlock (buffer, midiMessages);
             }
         }

          void callProcess (AudioBuffer<double>& buffer, MidiBuffer& midiMessages)
         {
             if (processor.isUsingDoublePrecision())
             {
                 if (node->isBypassed())
                     processor.processBlockBypassed (buffer, midiMessages);
                 else
                     processor.processBlock (buffer, midiMessages);
             }
             else
             {
                 tempBufferFloat.makeCopyOf (buffer, true);
        
                 if (node->isBypassed())
                     processor.processBlockBypassed (tempBufferFloat, midiMessages);
                 else
                     processor.processBlock (tempBufferFloat, midiMessages);
        
                 buffer.makeCopyOf (tempBufferFloat, true);
             }
         }
    :
    }

and a public method to be able to refresh the graph externally when one of the processors changes it’s latency:

 void refreshGraph() { triggerAsyncUpdate(); }

and finally in createRenderingOpsForNode() change

    if (numOuts == 0)
        totalLatency = maxLatency;

to:

    if (maxLatency > totalLatency)
        totalLatency = maxLatency;

Cheers,

Rail

Just to chime in here a little bit: I also made my own version of the AudioProcessorGraph. I needed the ability to update to changing latencies as well. I did have to get rid of triggerAsyncUpdate, it just didn’t work for me like that, because I need reliable timing on graph changes, so I used a system with a timer and added the option to accumulate and postpone graph updates for a while (when patches change).

In addition I needed a way to prevent the user from creating loops in the graph. To detect a potential loop before it gets added, I used the boost graph library and one of it’s graph traversal search algorithms. Such a graph library could also be used to create rendering sequences and I believe performance could be much better/consistent that way. IMHO the buildRenderingSequence suffers from too much iteration and recursive calls.

Just so it’s on your radar… the other spots the profiler shows are bottlenecks are in createRenderingOpsForNode()

findBufferForInputAudioChannel() and findBufferForInputMidiChannel()

Cheers,

Rail

So I just tested the latest tip with the new code… and the graph works… but changing connections seems slower (definitely a lot slower in debug due to the recursion in isAnInputTo()… but changing or adding connections appear to be quite a bit slower than what I was using (in Release)… I’ll make some time in the next few days to run profile comparisons.

Thanks,

Rail

Still seeing this… with float only… and changing the MessageManagerLock to a ScopedLock… we can profile and see the biggest bottleneck is the recursion in createOrderedNodeList()

Rail

Now this is interesting… I’m comparing what is basically your experimental code base prior to the tip release code…

In my Plug-in which can host other plug-ins… you can drag the internal effects around to change their order… this changes some connections in the graph… now if we look at the profile for the new tip code:

and the previous (cleanup_audiograph) based code profile:

We can see that your previous code is a lot more efficient.

Rail

Between your changes and mine, we’ve tried a lot of different approaches to this and they’ll all have different performance characteristics with different use-cases! I’m really slammed this week but will try to come back to have a look at this when I get chance.

Thanks… More importantly, I’ve just run into a scenario which happens in both your cleanup_audiograph and the tip… but not in the older AudioProcessorGraph… where I’m getting one of the processors in the graph leaking. I have a reproducible case to cause the issue… but I’m still trying to isolate the exact cause of the leak (why is one of the processors not getting deleted when the graph is deleted)… when I list my connections they appear the same as in the original graph except for the MIDI connections…

Original:

There are 48 Connections in the graph

Connection[000]: Src: Midi Input                                        (002) Channel Index:  4096 -- Dest: Midi Output                                       (003) Channel Index:  4096
Connection[001]: Src: Midi Input                                        (002) Channel Index:  4096 -- Dest: Synth Processor                                   (078) Channel Index:  4096
Connection[002]: Src: Bus: Master                                       (004) Channel Index:     0 -- Dest: Master Track Volume Processor                     (093) Channel Index:     0
Connection[003]: Src: Bus: Master                                       (004) Channel Index:     1 -- Dest: Master Track Volume Processor                     (093) Channel Index:     1

New:

There are 48 Connections in the graph

Connection[000]: Src: UNDEFINED                                         (000) Channel Index:     0 -- Dest: UNDEFINED                                         (000) Channel Index:     0
Connection[001]: Src: Bus: Out 25-26                                    (049) Channel Index:  4096 -- Dest: Synth Processor                                   (078) Channel Index:  4096
Connection[002]: Src: Bus: Master                                       (004) Channel Index:     0 -- Dest: Master Track Volume Processor                     (093) Channel Index:     0
Connection[003]: Src: Bus: Master                                       (004) Channel Index:     1 -- Dest: Master Track Volume Processor                     (093) Channel Index:     1

None of these is related to the leaking processor though, where the log is the same on both versions (it’s Node 79’s processor which is not being deleted):

NodeID: 79  ::: Processor: Bleed Input Processor
NodeID: 80  ::: Processor: Instr Track Trim & Phase Processor

Connection[006]: Src: Bleed Bus: KICK_MONO_001                          (077) Channel Index:     0 -- Dest: Bleed Input Processor                             (079) Channel Index:     0
Connection[007]: Src: Bleed Bus: KICK_MONO_001                          (077) Channel Index:     1 -- Dest: Bleed Input Processor                             (079) Channel Index:     1
Connection[008]: Src: Synth Processor                                   (078) Channel Index:     8 -- Dest: Instr Track Trim & Phase Processor                (080) Channel Index:     0
Connection[009]: Src: Synth Processor                                   (078) Channel Index:     8 -- Dest: Instr Track Trim & Phase Processor                (080) Channel Index:     1
Connection[010]: Src: Bleed Input Processor                             (079) Channel Index:     0 -- Dest: Volume & Mute Processor                           (081) Channel Index:     0
Connection[011]: Src: Bleed Input Processor                             (079) Channel Index:     1 -- Dest: Volume & Mute Processor                           (081) Channel Index:     1
Connection[012]: Src: Instr Track Trim & Phase Processor                (080) Channel Index:     0 -- Dest: Volume & Mute Processor                           (081) Channel Index:     0
Connection[013]: Src: Instr Track Trim & Phase Processor                (080) Channel Index:     1 -- Dest: Volume & Mute Processor                           (081) Channel Index:     1

I’ll let you know if I can isolate the issue. The only thing I see which is off in the trigger is the source connection to node 78 (Synth Processor) changes in the log… where it shouldn’t change.

Cheers,

Rail

If I change the sample rate in my host and call prepareToPlay() on the graph, it doesn’t update the sample rate in all the graph’s processors…

void AudioProcessorGraph::prepareToPlay (double sampleRate, int estimatedSamplesPerBlock)
{
    setRateAndBufferSizeDetails (sampleRate, estimatedSamplesPerBlock); // <---  Added this line

    if (renderSequenceFloat != nullptr)
        renderSequenceFloat->prepareBuffers (estimatedSamplesPerBlock);

    if (renderSequenceDouble != nullptr)
        renderSequenceDouble->prepareBuffers (estimatedSamplesPerBlock);

    clearRenderingSequence();
    buildRenderingSequence();

    isPrepared = true;
}

Cheers,

Rail

The answer may be to allow a graph to be a child of another graph… allowing MIDI in and out, etc. That would let us reduce the number of connections needing to be resorted.

Cheers,

Rail