For this to work, the Graph definition would have to know how many channels a control has. I think this isn't explicitly given anywhere. Either one writes it into the SynthDef and makes a new version (read it in GraphDef_ReadVer2 then) or perhaps there is some other way to find the right number of channels.

Just to know: what should be the right behaviour? \freq not defined? Should it sound?

I'm just discussing this with Luca. The problem is simply that when setting controls there is no bounds checking, and never has been. A similar problem:

x = {|mul = #[0.1, 0.2], freq = 440| SinOsc.ar(freq, 0, mul) }.play
x.set(\mul, [0.2, 0.1, 800])

My assumption is that this is for reasons of efficiency, and maybe also to do with the old (probably now unused, and actually maybe unpredictable) ability to set controls by index rather than name. I think a multichannel control is just a series of adjacent controls.

So perhaps there's a question of whether this should be fixed. Personally I always thought it should be, as I suspect any performance cost will be minimal in most cases.

If it is fixed, should it be fixed server side (robust across different clients) or lang side (allows it perhaps to be optional)?

I'd favour the former. I'm not sure off the top of my head if this would require a new version of the SynthDef format. It might be possible to derive the size of multichannel controls, but I'm not 100% sure that there wouldn't be corner cases where there could be mistakes.

The problem is simply that when setting controls there is no bounds checking, and never has been

There is bounds checking on the number of controls in a synth though.

BTW, if we're looking at this we should also fix:

y = {|mul = #[0.1, 0.2], freq = 660| SinOsc.ar(freq, 0, mul) }.play
y.get(\mul, _.postln);

Intuitively I'd think get should get both channels of mul.

If it is fixed, should it be fixed server side (robust across different clients) or lang side (allows it perhaps to be optional)?

I'd favour the former.

me, too. I'd say when you pass a name, then it should be bounded. You can still pass an index and have it non-bounded.

The method would be:

void Graph_SetControl(Graph* inGraph, int32 inHash, int32 *inName, uint32 inIndex, float inValue)
{
    ParamSpecTable* table = GRAPH_PARAM_TABLE(inGraph);
    ParamSpec *spec = table->Get(inHash, inName);
    if (spec) Graph_SetControl(inGraph, spec->mIndex + inIndex, inValue);
}

Maybe one could look for the index of the control as registered from the name: spec->mIndex?
Hm, thinking about it, the Graph doesn't know what the next control name is, so probably one would have to calculate the sizes when building the graph, in SC_GraphDEf.cpp:

After this has been done:

void ParamSpec_Read(ParamSpec* inParamSpec, char*& buffer);
void ParamSpec_Read(ParamSpec* inParamSpec, char*& buffer)
{
    ReadName(buffer, inParamSpec->mName);
    inParamSpec->mIndex = readInt32_be(buffer);
    inParamSpec->mHash = Hash(inParamSpec->mName);
}

And add a list in the method:
GraphDef* GraphDef_Read(World *inWorld, char*& buffer, GraphDef* inList, int32 inVersion)
But if we don't want to add a different synth def version, we have to create an array of paramspecs, sort it according to the index and then associate the difference between two consecutive indices as numChannels to each paramspec.

Actually the main question is: do the paramSpecs come from the SynthDef in some order or could their index be unordered?

Looking at a few examples it seems that:

• Unnamed controls don't exist when you use .add. They are "legal" in .send(because no SynthDesc is build), but they seem not to result in a correct synth def:

SynthDef(\x, {  Control.names([\itchi]).kr(0 ! 4); Control.names([0, 1]).kr([1, 2]).poll; }).send;
a = Synth(\x);
// both don't set the synth control:
a.set(0, 700);
a.set(1, 700);

In all named cases, the inParamSpec->mIndex is already ordered, so that we can easily determine the number of channels.

Unfortunately, the inParamSpec->mIndex is not ordered in some cases. We'd have to sort the list somehow first, I have no idea of how to best do this in C++.

Here is what I got so far:

graphDef->mNumParamSpecs = readInt32_be(buffer);
    printf("\n\n**************\n");
    if (graphDef->mNumParamSpecs) {
        int hashTableSize = NEXTPOWEROFTWO(graphDef->mNumParamSpecs);
        graphDef->mParamSpecTable = new ParamSpecTable(&gMalloc, hashTableSize, false);
        graphDef->mParamSpecs = (ParamSpec*)malloc(graphDef->mNumParamSpecs * sizeof(ParamSpec));
        ParamSpec *prevParamSpec;
        for (uint32 i=0; i<graphDef->mNumParamSpecs; ++i) {
            ParamSpec *paramSpec = graphDef->mParamSpecs + i;
            ParamSpec_Read(paramSpec, buffer);
            graphDef->mParamSpecTable->Add(paramSpec);

            if(i != 0) {
                printf("%s index: %i, prev: %i\n", paramSpec->mName, paramSpec->mIndex, prevParamSpec->mIndex);
                printf("setting %i num channels to %i\n", i - 1, paramSpec->mIndex - prevParamSpec->mIndex);
                prevParamSpec->mNumChannels = paramSpec->mIndex - prevParamSpec->mIndex;
            }
            if(i == (graphDef->mNumParamSpecs - 1)) {
                printf("%s (last)\n", paramSpec->mName);
                printf("setting %i num channels to %i\n", i, graphDef->mNumControls - paramSpec->mIndex);
                paramSpec->mNumChannels = graphDef->mNumControls - paramSpec->mIndex;
            }
            prevParamSpec = paramSpec;
        }

    } else {
        // empty table to eliminate test in Graph_SetControl
        graphDef->mParamSpecTable = new ParamSpecTable(&gMalloc, 4, false);
        graphDef->mParamSpecs = 0;
    }

OK, solved it, I hope that using std::sort() is appropriate with the standards we have for compilers.

The fix is here: 774ee0f
In a new branch: bounded-control

I'm sure it can be improved, also I wasn't sure if the reference to std::sort() is generally supported.

The solution in the branch 'bounded-control' sorted the member variable mParamSpecs in order to find the adjacent specs for the number of channels. This worked fine, but had bad side effects, so I reverted the merge. If anyone with more knowledge about c++ would like to continue, I think the fix is easy. You probably just have to sort a copy of that member instead of the member itself.

For reference, here is a reproducer for the problem that came from directly sorting mParamSpecs:

(
var target = Bus(\audio, 2, 4, s),
bus,
freq = 6,
synthTarget = s.defaultGroup;

c = s.controlBusAllocator.alloc(5);

bus = Bus.control(s, 4);

b = bus;

x = Synth.basicNew(\x, s, 1000);

SynthDef(\x, {
    arg bus, kbus, t_trig;  // t_trig lets client control timing/reset of Peak
    var sig;
    sig = Peak.ar(In.ar(bus, target.numChannels), T2A.ar(t_trig));
    Out.kr(kbus, sig);
}).send(target.server,
    x.newMsg(synthTarget, [\bus, target.index, \kbus, bus.index, \freq, freq],
        \addToTail).debug("the outgoing arg list is correct"));
)

the outgoing arg list is correct: 9, x, 1000, 1, 1, bus, 2, kbus, 10, freq, 6

s.queryAllNodes(true);

-->

SynthDef:x
localhost
NODE TREE Group 0
 1 group
    1000 x
      t_trig: 0 bus: 0 kbus: 0

Maybe this will be the correct solution: 6b20107.