AudioProcessorGraph && ChangeBroadcaster

So in my previous thread:

I derived AudioProcessorGraph from ChangeBroadcaster and that worked perfectly in 4.2 – but I just tried doing that while testing 4.3.2 and 5.1.1 and if you derive AudioProcessorGraph from ChangeBroadcaster and try and process a block you get an EXC_BAD_ACCESS run-time error when it tries to call processAudio (the error location is consistent… but didn’t help to track down the cause of the issue BTW)

Any suggestions on how to send a message to the main plug-in AudioProcessor when the graph is re-calculated without using a ChangeBroadcaster?

Cheers,

Rail

Bizarrely I just wrote exactly that same thing last night… Please hold off changing AudioProcessorGraph as I’m in the middle of completely refactoring it, and am tidying up a bunch of stuff like this. Will share it when stable.

Thanks!! I can’t wait – I take it you’ve reviewed my past posts on this topic (audioProcessorGraph) for some ideas :slight_smile:

Cheers,

Rail

Yep, I took into account the stuff you posted about scaling to large graph sizes. Will be good to have you sanity-check my changes, as it sounds like you’re the heaviest user of this class!

Thanks Jules. Totally at your disposal to test it when you’re ready!

Cheers,

Rail

Thanks - here’s a branch if you want to have a go:
https://github.com/WeAreROLI/JUCE/tree/jules/cleanup_audiograph

I’ll eventually go a lot further with refactoring it - I’d like to make the graph flattening algorithms available separately, and add parallelism, but this was a first pass at cleaning up the really nasty old crusty bits.

Hi Jules, thanks for putting some time into cleaning up and reworking the graph! It’s great to see. :slight_smile:

I’ve made a fair number of changes to the graph and it would be great to get back more inline with the stock version, so maybe you will consider these points as you continue to refactor.

-Node ownership of AudioProcessors optional. (can default to owned for compatibility)
-Separate out all of the message thread/async notification stuff and introduce a core graph interface / listener that doesn’t assume any of the threading/async behaviours. It would be up to the user to do things correctly for their needs. The existing implementation could be still provided as is and would just work with the core interface instead of being baked in.

If you happen to decide to move in this direction then I’d be happy to do testing of it as well. Thanks!

I was hoping you would have removed the MessageManagerLock (and any MessageManger related) , and replacing it by a separate CriticalSections, or something better. The MML is a problem when using the graph in plugins (+ any kind of asynchron messaging)

PS: if you need an inspiration for a Multithread-Version, I made my thoughts years ago, somewhere here in the forum.

+1! Thanks for looking at this Jules

The nodes are ref-counted, aren’t they!?

And yes, part of my motivation for starting this is to eventually break it up into generic algorithms that I can re-use for other purposes too. Like I said, this is just a start.

I’m referring to the ScopedPointer in the Node. I manage my processor lifetimes outside of the graph.
const ScopedPointer<AudioProcessor> processor;

Sounds great. :slight_smile:

Hi Jules… Sorry went to bed just before you posted… and I’ve had to make a branch and change a bunch of my code… the biggest issue I’ve run into is your change to using AudioProcessorGraph::Connection as a single parameter for addConnection() and addConnection()… the first thing I’ve had to do so far is add a constructor for the NodeAndChannel struct:

struct NodeAndChannel
{
    NodeAndChannel() : nodeID (0), channelIndex (0) {}
    NodeAndChannel (NodeID theNodeID, int chIndex) : nodeID (theNodeID), channelIndex (chIndex) {}

    NodeID nodeID;
    int channelIndex;

    bool isMIDI() const noexcept                                    { return channelIndex == midiChannelIndex; }

    bool operator== (const NodeAndChannel& other) const noexcept    { return nodeID == other.nodeID && channelIndex == other.channelIndex; }
    bool operator!= (const NodeAndChannel& other) const noexcept    { return ! operator== (other); }
};

That way I can change my helper functions from:

bool canMakeStereoConnection (AudioProcessorGraph* graph, AudioProcessorGraph::Node::Ptr from, AudioProcessorGraph::Node::Ptr to)
{
    return graph->canConnect (from->nodeId, 0, to->nodeId, 0) && graph->canConnect (from->nodeId, 1, to->nodeId, 1);
}

 bool addStereoConnection (AudioProcessorGraph* graph, AudioProcessorGraph::Node::Ptr from, AudioProcessorGraph::Node::Ptr to, int iChanIndex1 /* = 0 */, int iChanIndex2 /* = 1 */)
{
    return graph->addConnection (from->nodeId, 0, to->nodeId, iChanIndex1) && graph->addConnection (from->nodeId, 1, to->nodeId, iChanIndex2);
}

to:

  bool canMakeStereoConnection (AudioProcessorGraph* graph, AudioProcessorGraph::Node::Ptr from, AudioProcessorGraph::Node::Ptr to)
 {
     return graph->canConnect ({ { from->nodeId, 0 }, { to->nodeId, 0 } }) && graph->canConnect ( { { from->nodeId, 1 }, { to->nodeId, 1 } });
 }

  bool addStereoConnection (AudioProcessorGraph* graph, AudioProcessorGraph::Node::Ptr from, AudioProcessorGraph::Node::Ptr to, int iChanIndex1 /* = 0 */, int iChanIndex2 /* = 1 */)
 {
     return graph->addConnection ( { { from->nodeId, 0 }, { to->nodeId, iChanIndex1 } }) && graph->addConnection ( { { from->nodeId, 1 }, { to->nodeId, iChanIndex2 } });
 }

Also had to add:

/** Returns the number of connections in the graph. */
int getNumConnections() const                                             { return (int) connections.size();       }

and

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

I’ll report more after I get all the changes done… and I’ll add more to this post as I progress in making this work with my code…

Thanks,

Rail

@jules whilst your at it, is there any particular reason to use the terminology “Filter” in the plugin host? Isn’t it an “AudioProcessorGraphDisplay” or “NodeGraph”? I don’t think it’s very intuitive since a filter can be a synth or i/o! or am i missing something?

I’ll send you a PM with a report… I have some connections not being made in your graph.

EDIT: Never mind… I just found a copy/paste error I made while updating my code.

Thanks,

Rail

So the good news is that the basic graph is working (with my changes above). Performance wise it’s twice the speed of my changes (I know you haven’t optimized it yet)… so probably similar to the original code’s performance.

I threw a huge preset at it with internal and external effects in the Mixer and a bunch of Aux sends/tracks… and everything connected correctly. PDC is off using external plug-ins… but I didn’t add a BroadcastChangeListener to update the latency yet.

Cheers,

Rail

Hi Rail - can you clarify what you mean here - you say it’s twice the speed, but also that it’s slower? Do you mean it’s half the speed? If so, I’m a bit stuck as to what else I could do, as I did my best to make sure I optimised the same areas that you mentioned earlier. If you can let me know the hotspots I can do some more tweaking!

Sorry, I re-read that myself and hoped you’d understand…

It’s slower than mine… meaning it takes twice as long to recall the preset than my code does.

I’ll do some profiling.

Thanks,

Rail

Yeah, I figured it must be an English idiom misunderstanding - “twice the speed” would be a good thing!

I just need more sleep :wink:

I added a File::Logger to AudioProcessorGraph::sortConnections() to see how many times you call it vs. how many times I call it…

Okay - So just initializing my plug-in… which creates a few basic tracks… in my code I call sortConnections() 14 times… the exact same code using your graph calls sortConnections() 22,558 times.

Profiling also shows that AudioProcesssorGraph::buildRenderingSequence() for a same section of code takes 8 mSecs with the old version… and 21 mSecs with your new code… so even comparing the original graph to yours you’re still slower (I haven’t changed anything in the buildRenderingSequence() method except to sendMessage().

Let me know if you need anything from me to assist you…

Cheers,

Rail

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?