[scsynth] PluginCmd and sending responses

[weefuzzy]

Hoping an scsynth maven can help me out here, as it’s likely I’m overlooking something.

tl;dr: is there an idiomatic / documented / recommended way that one would send data back to a client from a PluginCmd?

Context

PluginCmd is a facility that allows executing some code on the server outwith the context of a running synth. It doesn’t seem to be used very much. However, in our project there are a couple of contexts in which it could be helpful (triggering long running batch processes that happen in a worker thread, and maintaining data structures that need to persist beyond cmd-. from the client).

However, it’s not at all obvious to me how to use this and be able to send data back to a client, which limits its usefulness.

The standard idiom with these things seems to be to launch an AsyncPlugCmd process, which does a mutlistep thing that properly handles running the right things on the right threads etc. At the end of this, it will send /done back to the client, but there is no option to add any arguments to this.

Meanwhile, the ReplyAddr structure and SendReply function (not the UGen) were moved out of the public interface some time ago (https://github.com/supercollider/supercollider/pull/903), so there isn’t recourse to a lower-level facility, AFAICT.

It’s quite possible that the answer to this is so obvious and staring me in the face that I’ve missed it entirely.

[Spacechild1]

Hi, I’m using plugin commands in VSTPlugin.

@weefuzzy

The standard idiom with these things seems to be to launch an AsyncPlugCmd process, which does a mutlistep thing that properly handles running the right things on the right threads etc. At the end of this, it will send /done back to the client, but there is no option to add any arguments to this.

That’s correct. You’re right that DoAsynchronousCommand is a bit limited in that it can only send a /done <command_name> message.

@weefuzzy

However, it’s not at all obvious to me how to use this and be able to send data back to a client, which limits its usefulness.

If you’re dealing with local Servers, you can simply write the data to a temp file. This is done in certain spots in the Class library (e.g. Buffer.loadCollection) and I also do this in VSTPlugin, e.g. for sending plugin search results from the Server back to the Client. The steps are:

1. Client: create a tmp file name and passes it as an argument to the plugin command message
2. Server: try to execute the command and if it succeeds, create the tmp file and write the data into the file
3. Client: wait for the async command to finish (e.g. with s.sync)
4. Client: check if the tmp file exists (= success) or not (= fail). Alternatively, depending on the data format, the file could also contain an error code/message (this is what I do in VSTPlugin).
5. Client: read the data and delete the tmp file

For remote Servers I have implemented an alternative method where the binary data is stored in a Buffer (with each byte as a float) and then streamed to the Client with Buffer.getToFloatArray. See cmdProbe in VSTPlugin.cpp resp. prProbeRemote in VSTPlugin.sc. In earlier versions I had written my own streaming code, but then I realized it would be easier and more robust to piggyback on Buffer.getToFloatArray.

Generally, there wouldn’t be a sane way to send arbitrary data to the client in a single OSC message, because UDP messages are limited in size and not reliable. You have to either send the data over the file system (see Buffer.loadCollection) or break it up into several messages (see Buffer.sendCollection).

[Spacechild1]

That being said, it would be nice if we had API methods to send arbitrary OSC messages to invididual clients (using the ReplyAddr argument) or all clients (like SendReply, which is unfortunately limited to float arrays).

I have been thinking about making a PR for extending the plugin API, but there is only so much time…

[weefuzzy]

Thanks for that really helpful response!

For bigger stuff (like dumping the complete internal state of some things) we do currently use the file system, and buffers is a canny workaround for remote servers. For smaller stuff we’re currently able to use ft->SendNodeReply, because the whole framework is contorted into using Units/UGens. However, this is creating so much additional complexity (cognitive and computational), that I thought I’d revisit the PlugInCmd as an alternative.

I had vaguely thought that one possibility in the absence of access to the more general SendReply function would be to use a Node purely for handling responses, like

1. Client: send /cmd <whatever> to server, includes a NodeID as an argument
2. Server does job, gets node with ft->fGetNode and, if it exists, send a response with ft->SendNodeReply

But maybe your buffer idea would be a more robust way of doing the same sort of thing. In either case, it all feels quite protracted.

@Spacechild1

I have been thinking about making a PR for extending the plugin API, but there is only so much time…

Alas, there is only so much time. That said, seeing as I’m in the insanely lucky position of this being my day job for now, maybe I should come up with something concrete rather than just kvetching

It doesn’t seem like making this functionality available should be too hard, or do bad things (but I’m prepared to be corrected). Possibilities seem like they might include:

1. A public facing overload of the SendReply function from SC_ReplyImpl that takes a void* for the ReplyAddress
2. An overload of World_SendPacket in SC_WorldOptions.h that, likewise, takes a void* for the ReplyAddress instead of the ReplyFunc pointer we can’t access (but it’s maybe like that for a good reason!)

[Spacechild1]

@weefuzzy

• A public facing overload of the SendReply function from SC_ReplyImpl that takes a void* for the ReplyAddress
• An overload of World_SendPacket in SC_WorldOptions.h that, likewise, takes a void* for the ReplyAddress instead of the ReplyFunc pointer we can’t access (but it’s maybe like that for a good reason!)

The thing is that the implementation resides in scsynth and plugins don’t directly link back to it, so all function calls have to go via the interface table. On the other hand, if the ReplyFunc was the first member of ReplyAddress, then we could cast the ReplyAddress to a ReplyFunc and safely call it. But there’s another problem: the reply function must be called on the NRT thread (it directly sends data to the socket), but we would rather like to call our methods on the RT thread (like SendNodeReply). My thought was to add the following entries (names can change :-):

1. a generalized node reply:
   void (*fSendNodeReplyEx)(struct Node* inNode, int replyID, const char* cmdName, const char *typeTags, const char *args, int size);
2. a method to send arbitrary OSC messages to clients:
   void (*fSendReply)(void *replyAddr, const char *msg, int size);
NULL as replyAddr could mean “send to all clients”.

We would also need a method to copy a reply address, because it is currently only valid during the PlugInCmdFunc callback (fDoAsynchronousCommand internally makes a deep copy of the replyAddr argument). Maybe this doesn’t have to be an interface method, because the actual reply address is trivially copyable, so we would only need some kind of storage which is guaranteed to be large enough to hold a reply address, e.g.:

struct ReplyAddrStorage { char buf[kMaxReplyAddrSize]; };

Then we can just use memcpy to copy the void *replyAddr into the storage. This is very similar to sockaddr_storage (<sys/socket.h>), which is large enough to hold any socket address.

What do you think about this? Actually, I might actually have some time to implement this. It shouldn’t be too complicated.

But while I’m at it, I really want to make the plugin API itself extendable while staying backwards compatible, so we don’t have to recompile all existing UGens everytime the plugin API changes…

My idea is this:

1. add a padding array to the interface table (e.g. void *reserved[256]), which is filled with NULL pointers, but can be later filled with new API function pointers.
2. add a fGetVersion method, so that UGens can query the server version.

Plugins can then either check the server version, or check if the required API functions are not NULL

[Spacechild1]

@weefuzzy

For smaller stuff we’re currently able to use ft->SendNodeReply , because the whole framework is contorted into using Units/UGens.

BTW, I also use SendNodeReply to send data from specific plugin instances to the client controllers. Some data consists of strings (e.g. plugin parameter displays), which I have to encode as float arrays . Something like fSendNodeReplyEx would have been so nice to have…

[weefuzzy]

Ach, yes, I hadn’t really thought linking through properly.

My thoughts are a bit scattered, compared to yours. I guess the short version is that it looks like there’s (two?) possible RFCs here, one to address the SendReply thing, the other the extensibility issue more generally.

On SendReply:

• Those functions seem like they’d certainly cater for for my needs
• We’d definitely need to think about naming to make sure they’re not confusing, especially given (unenforceable?) threading restrictions
• Yes, we’d need to be able to copy the ReplyAddr. This solution feels quite C-ish. If we’d need to touch the struct in any case, is there value in exploring (say) a pimpl idiom instead? Perhaps the overhead would be too much.

My thoughts are (even) less clear about extensibility; I feel like there’d be more resistance to the idea in general. At the same time, I’d probably go even further and wonder if a (lightweight) mechanism for allowing plugins to talk to each other could be added to the interface table, maybe as simple as allowing plugins to call PluginCmd themselves (definitely just riffing at this point).

@Spacechild1

strings (e.g. plugin parameter displays), which I have to encode as float arrays

Us too

[Spacechild1]

We’d definitely need to think about naming to make sure they’re not confusing

SendReplyEx was just a joke reference to the Win32 API, we can surely find a more descriptive name . On the other hand, a function name can only tell you so much. What we need to do is to finally document the API functions. Currently, the plugin API is mostly undocumented and I had to figure out how certain functions work by studying (and sometimes even fixing) the source code.

Yes, we’d need to be able to copy the ReplyAddr . This solution feels quite C-ish.

After all, it’s a C API . But yes, we could at least offer a convenience inline function like:

CopyReplyAddress(void *src, ReplyAddressStorage *dst);

Note that you won’t have to copy the reply address when you use SendReply directly in the plugin command callback, only when you need to call it later.

My thoughts are (even) less clear about extensibility; I feel like there’d be more resistance to the idea in general.

There might be some initial pushback, but once this gets merged, there will be less resistance against adding new API methods in the future, because you wouldn’t need to bump the API version and recompile all existing plugins. So I think this RFC/PR should come first.

At the same time, I’d probably go even further and wonder if a (lightweight) mechanism for allowing plugins to talk to each other

I think the most general way would be to add a method to send arbitrary Server OSC messages. In fact, you can already do this by abusing DoAsynchronousCommand (calling it with NULL values except for the completion message), but it has to go through the NRT channel once. A proper function could look like this:

void (*fSendServerMessage)(int msgSize, const char *msgData);

[scztt]

@Spacechild1

My idea is this:

1. add a padding array to the interface table (e.g. void *reserved[256] ), which is filled with NULL pointers, but can be later filled with new API function pointers.
2. add a fGetVersion method, so that UGens can query the server version.

Plugins can then either check the server version, or check if the required API functions are not NULL

As long as you only add elements to the struct, plugins built against previous header versions will transparently continue to work. Adding entries does not change the memory layout of the table, so newer versions of the table should work transparently with plugins built against an older version of the table - this practice is commonly used for C plugin interfaces.

In fact, I’m pretty sure we could change pluginVersion == sc_api_version to pluginVersion <= sc_api_version right now and we would be able to run (at an ABI level, at least) plugins builds going back ~10 years (there was an ABI-breaking change somewhere back in ancient history?) with no additional changes.

There is no need for plugins to query the actual server version, and in fact this is a plugin anti-pattern. A given interface call should have guaranteed behavior regardless of the host version - it’s the responsibility of the HOST to bridge version differences and guarantee that, not the plugin. In practice, this can occasionally mean e.g. if the behavior of PluginFunctionFoo changes between version 4 and version 5 because of a bug fix (e.g. fixing an erroneous return value in some case), you might choose to fill the interface table with a patched version exhibiting the old behavior to ensure old plugins continue to work, if you detect a plugin built against v4. But, this is achieved by the host querying the version the plugin is written against (for us, via api_version), and not the other way around.
(Of course, plugins still have access to the api version, and can theoretically use it - in the real world, this is how plugins work around e.g. host bugs or subtle behavioral differences - but it’s a bad kind of technical debt, and something to be actively discouraged)

@Spacechild1

struct ReplyAddrStorage { char buf[kMaxReplyAddrSize]; };

This makes a lot of assumptions about the address - for example, it assumes that a memcpy is valid, which wouldn’t be the case if it contained a pointer or reference to some other kind of volatile/non-relocatable object. There may be some other subtleties too: suppose we are doing TCP communication and not UDP - if a plugin holds on to a reference to an address, this may mean that the host would want to keep this port open vs closing it - something it can only do if it’s properly reference tracking the address.

What about an implementation like this:

struct InterfaceTable {
   // ...
   using ClientId = int;
   ClientId (*fClientIdForAddress)(void* replyAddress);
   void (*fSendReply)(ClientId, const char *msg, int size);
   void (*fSendReplyAll)(const char *msg, int size);
}

These could be invoked in a DoAsynchronousCommand functor, with cmdData that looked like struct { ClientId id; int value; char name[256] } or whatever.

I’m wondering if the best-case API would actually wrap the OSC bundle assembly process, rather than just taking a const char* with a pre-assembled message? E.g. provide an abstraction for AllocateBundle, AddIntToBundle, etc etc. Forcing plugins to assemble their own OSC messages means that, in theory, every plugin that needs to do this would need to link to their own copy of an OSC library - this opens the door a LOT of problems if plugin devs don’t e.g. statically link and hide symbols properly.

[Spacechild1]

@scztt

Adding entries does not change the memory layout of the table, so newer versions of the table should work transparently with plugins built against an older version of the table

Yes, but what happens if a plugin built with a newer header runs on a Server that is built with an older header? The plugin would attempt to read bogus function pointers. That’s why it’s important to add padding members which are explicitly set to some well defined default (e.g. NULL for function pointers).

@scztt

this practice is commonly used for C plugin interfaces.

Simply extending a struct does indeed work, but only if the caller makes sure that the host version is recent enough to actually provide this member.

@scztt

there was an ABI-breaking change somewhere back in ancient history?

There has been more than one ABI break in the last few years. One break was changing some int16 to int32 and then fairly recently some member has been inserted in the middle of some struct (it might have been Unit, but I forgot).

@scztt

There is no need for plugins to query the actual server version, and in fact this is a plugin anti-pattern. A given interface call should have guaranteed behavior regardless of the host version - it’s the responsibility of the HOST to bridge version differences and guarantee that, not the plugin

I don’t see how this would work. How can a host bridge (future) functionality it doesn’t even know about? If a plugin wants to use certain API functions, it has to make sure that they are available.

In the case of plugins which link back to host this works implicitly, as loading would simply fail because of the missing symbol (this is the case with Pd externals).

But in the case of plugins which don’t link back, they have to query the host explicitly for functionality. This can be either by checking the host version, use a dedicated API function (like the audioMasterCanDo opcode of VST2 plugins), query an interface (in COM or COM-like plugins), etc.

In our case, the plugin has to either
a) check if the required interface functions are not NULL or
b) query the host version (e.g. because it knows that version 3.X introduced the required API functions).

Again, I’m strictly talking about extending the plugin API. For any ABI break (like changing structs or function signatures) we still must bump the plugin API version.

Changing the behavior of existing functions is an interesting edge case. Here you’re right that it should be the HOST’s responsibility to provide a compatibility layer. But the best thing would be to never introduce breaking changes in the behavior of existing functions

@scztt

This makes a lot of assumptions about the address

Well, of course the assumption would be that ReplyAddress is trivially copyable (which I think it currently is). You’re right, however, that using IDs would be safer.

You are right that the client might have shut down before we send the reply, but in practice this only means we’re trying to send to a closed socket. We can’t really handle this error in a meaningful way anyway, since the actual sending happens in the NRT thread. I think the plugin doesn’t even have to care about it.

@scztt

I’m wondering if the best-case API would actually wrap the OSC bundle assembly process, rather than just taking a const char* with a pre-assembled message?

That’s a good question and I’m not sure about it… Exposing a complete OSC library via the plugin API seems a bit overkill. Personally, I think it’s fine for plugin author’s to use their own copy of oscpack or whatever. After all, these kind of features wouldn’t be used by many developers.

@scztt

this opens the door a LOT of problems if plugin devs don’t e.g. statically link and hide symbols properly.

Well, all statically linked plugins have to hide their symbols! The issue with symbol name collision is not only true for OSC libraries but really any dependency. It is the plugin author’s responsibility to do this.

[Spacechild1]

On the other hand, we could do it the other away round and let the plugin tell the host which minimal version it expects, so the host would refuse to load it if it doesn’t meet the requirement, but in practice there’s not much difference. It’s a matter of taste, I guess.

The question is what would be the most elegant way to do this? Maybe a new extended plugin register function which takes the desired minimal Server version?

[Spacechild1]

BTW, it’s import to distinguish two things:

a) the plugin API version which signifies an ABI break. This version must match exactly and plugins have to be recompiled otherwise. Bumping the plugin API version should only be done if absolutely necessary

b) the minimal server version which offers the desired plugin API functions. This version uses a >= comparison, so that older plugins can run on newer servers.

Most plugin systems try to avoid the necessity of a) in the first place by never changing the ABI. Pd and VST2, for example, have managed to do so for over 20 years :-).

EDIT:

c) use a single plugin API version and internally keep track of any ABI breaks between versions, but this is rather messy, I think…

[scztt]

@Spacechild1

Yes, but what happens if a plugin built with a newer header runs on a Server that is built with an older header? The plugin would attempt to read bogus function pointers. That’s why it’s important to add padding members which are explicitly set to some well defined default (e.g. NULL for function pointers).

The case you’re talking about here is allowing a plugin to have conditional behavior depending on the presence of a feature or function - e.g. “if I have access to a SendReply function, call it, else skip it”? In this case - yes, this would not be trivially supported without giving the plugin some ability to query capabilities. I’ve seen this implemented as a suite request-and-response pattern, where you might have something like:

struct OscFuncs_v1 {
    void (*fSendReply)(const char*);
}
struct OscFuncs_v2 {
    void (*fSendReplyTo)(ClientId, const char*);
}
struct InterfaceTable {
     void* GetOscFuncs(size_t version) { /* conditionally return a table, or null_ptr */ }
}

or just directly with the primary interface table: void* GetInterfaceTable(size_t version).

This is almost identical to the null-initialized implementation you suggested, except that it may put a little more power regarding handling versions in the hands of the host (because there’s one extra function call we can intercept)? For that reason, I would intuitively prefer it, but I think it’s maybe not an important difference.

@Spacechild1

There has been more than one ABI break in the last few years. One break was changing some int16 to int32 and then fairly recently some member has been inserted in the middle of some struct (it might have been Unit , but I forgot).

Ugh - I remember this a little bit now. That’s too bad.

@Spacechild1

I don’t see how this would work. How can a host bridge (future) functionality it doesn’t even know about? If a plugin wants to use certain API functions, it has to make sure that they are available.

Sorry - yes, there is another piece to this: you would generally build your plugin against the minimum plugin version you require. Building a v30 implies to the host that you’re actually USING features specific to v30, which of course means a v29 host couldn’t support the plugin. Usually you’d maintain older versions so that plugins can only use the minimum required, if they care about cross-compatibility.

Because of our build system and headers, we are more or less implicitly doing this now: sc_api_version is the plugin requesting/reporting a specific API version (defined as a compile time constant now). The host is branching on this to build the interface table, so we already have an API that is functionally equivalent to something like:
InterfaceTable_v40* it = getInterfaceTable(40);
… where the call only works when the requested version is the exact version the host was built with.

So there are probably three pieces of functionality we would need to improve things:

1. Provide a straightforward way for a plugin to request an arbitrary API version (not just the current version). This might take the form of simply packaging versioned InterfaceTable structs in separate headers when we rev the API - it’s a little clunky, but plugin API changes are rare so it would be acceptable for the time being.
2. Give the host the ability to load and provide tables for more than just the current API version. Note that this would be an OPTIONAL host behavior - e.g. Supernova could still choose to ignore plugins that don’t match it’s own api version if it chooses (though there’s no good reason not to fix supernova as well).
3. Give the plugin the ability to request multiple different API versions, and branch depending on which is available. For compatibility, we might consider an “upgrade” based approach, where a plugin returns it’s minimum supported version from api_version (the current behavior), and we provide an additional fGetInterfaceTableVersion(size_t version) that would allow it to request higher versions if they are available. The one caveat here is - we don’t want to create a situation where a plugin is sharing e.g. pointers to objects between two different InterfaceTable versions - but dealing with this is probably an implementation detail on the host side, or even just a documentation issue.