AudioProcessGraph: Remove "Connection"


#1

Hi Jules,

Was wondering how come the graph doesn’t have a removeConnection method with a pointer to a “Connection” as the parameter? Feels odd being able to remove connections every other way except via a pointer to a Connection object itself… and this would save a bit of typing.

bool AudioProcessorGraph::removeConnection (const Connection* connection)
{
    if (connection != nullptr
        && connections.contains (connection))
    {
        return removeConnection (connection->sourceNodeId, connection->sourceChannelIndex,
                                 connection->destNodeId, connection->destChannelIndex);
    }

    return false;
}

[FR] Iterator on AudioProcessorGraph::Connections
#2

Since Connection is a struct and therefore treated as a value type, it can also be safely used with copy semantics.
There really is not much gain in introducing pointers here.

Besides, you keeping a pointer to a Connection outside the AudioProcessorGraph means that in the case of the
AudioProcessorGraph deleting the Connection instance (which it assumes is owned by itself) can lead to nasty
access violations should you try to dereference that pointer later.
So, please do yourself a favor, do not keep those pointers dangling around in your code.


#3

Yes, I think that’s why I didn’t add a method like that - I didn’t expect people to hang onto Connection pointers, that’s why most of the methods use indexes rather than pointers.


#4

…and another request, because i want do an abstract of AudioProcessorGraph (to make it exchangeable with my multi-thread version (https://github.com/jchkn/ckjucetools/tree/master/AudioProcessorGraphMultiThreaded )
it would be cool if you make the interface more simpler, without references to inner data-types.
Its not much:
just move AudioProcessorGraph::Connection outside AudioProcessorGraph = AudioProcessorGraphConnection

and remove getNode() and getNodeForId
instead implement functions like getAudioProcessorForNodeID() to achieve the same functionality…


#5

Why not just base your multithread implementation on AudioProcessor as a base class?
The interface you mentioned is specific to AudioProcessorGraph, and it definitely makes sense to have types
that are only owned by it and make only sense with it as inner types.
You can always extract the interface portions you like to keep and add them as an interface to inherit from by your implementation, if you want to abstract away that interface.
Since C++ support multiple inheritance, you can do these kinds of things (just remember to specify the inheritance as virtual for interface classes).


#6

Because its ugly!

I like to have an AudioProcessorGraphSingleThreaded and AudioProcessorGraphMultiThreaded which inherit both from AudioProcessorGraphAbstract

My multiprocessor-version also use nodes, but they handled/typed differently , so if would inherit from AudioProcessorGraph, i have to write a wrapper class to provide the same interface ==> more ugly

jules: if your inspired by my code to write your own multi-thread capable version of it, you are welcome!


#7

sorry i misunderstood your post, but i have no time to correct yet :slight_smile:


#8

BTW: I already use AudioProcessor as base class


#9

[quote=“chkn”]

Because its ugly![/quote]

Actually, the ugly thing is that Jules made it that way :wink:
To your audio processing infrastructure, all that matters is that whateveryourimplementationiscalled is an AudioProcessor.
To another part of your application, probably a Component, all that matters is that it is a graph.
To the class implementation itself additionally it matters that it is either single or multi threaded.

There are very different aspects of what that class should do and how it should behave, and the actual ugly thing is that
in the reference implementation all of these have been rolled into one single API instead of separating them into several interfaces, implementing them in a concrete class and programming against those interfaces instead of a concrete class.


#10

I’m not storing Connection pointers - and you shouldn’t need to jump straight into assuming so. (I’m well aware of the implications of storing such pointers, but at least the Captain Obviousness may help newer users/programmers realize something important.)

What I am trying to do is procedurally remove connections, without having to store indices. I’ve achieved it, was just looking at minimizing typing.

This:

const juce::AudioProcessorGraph::Connection* connection
            = graph->getConnectionBetween (someId, 0, someOtherId, 0);

removeConnection (connection->sourceNodeId, connection->sourceChannelIndex,
                              connection->destNodeId, connection->destChannelIndex);

Versus this:

const juce::AudioProcessorGraph::Connection* connection
            = graph->getConnectionBetween (someId, 0, someOtherId, 0);
removeConnection (connection);

#11

Sorry for the assumption, my mistake.