-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add C++ plugins framework #1175
Comments
I like the approach. Something that worries me is the synchronous nature of the implementation. Plugins processing impacts directly on worker performance (not in this specific 'ping' example but potentially in any others). I would prefer an async framework where the communication between the plugin and the worker is made by libuv communication means. Ie: the same way Rust communicates with the worker. NOTE: I know how making this async would complicate things.., so I wonder if it would be acceptable to let the plugin user assume the performance cost of the plugin. NOTE2: Another worrying point is the fact that everything runs in the same process, meaning a crash in a plugin would crash the worker. |
Thx a lot for taking the time to look at this. NOTE1: There is a feature that it is hard to achieve with asynchronous processing that is having plugins that intercept the processing of a packet so that it doesn't continue normally. For example a plugin throttling the RTP packets received so that nobody can send more than X pkts/sec to avoid DoS attacks. NOTE2: |
Maybe the plugin itself could handle the events in an asynchronous way if the processing affects the main loop performance. |
I cannot comment much about this yet since I don't know anything about how a native plug-in can be "installed", but from architectural point of view I think that every plug-in should have:
|
Worker was supposed to be hidden behind TypeScript/JavaScript library, not sure I like the idea of adding C++ plugins. You should really consider Rust if you want performance, then you can still treat worker as opaque implementation, yet achieve higher performance. |
Thank you for your time, see inline.
Check the three alternatives mentioned in the description (Loading of plugins)
Consider the option of allowing plugins to decide what producers/routers/workers they are interested in, like in the prototype. Potentially a plugin could be used for other logic that is not related to producers or consumers IMO.
The way I have implemented this in other SFUs is with the plugins returning a flag (CONTINUE, BREAK...) to tell the next plugin or logic in the chain if the processing should continue or not and the default is to continue with normal processing.
This flow is covered in the prototype, right?
This could be another reason to follow the sync approach instead of the async one, right? |
I agree it is a paradigm change where the mediasoup API is not only the JS API but also the C++ API. I like that and ideally would allow me to run mediasoup in my own C++ process without any Javascript :) but I understand it is a big change and makes the API surface much bigger. I included Rust as an option in the Issue description and I think it is the best approach in many cases. My "problem" is that for many teams rust is not the most convenience language and is an entry barrier if to create a simple 100 lines plugin they need to rewrite 20.000 lines of javascript code. |
For context I was hoping we can eventually have more Rust in mediasoup and use https://github.com/neon-bindings/neon or something similar to create a Node.js module. Having C++ plugins support would mean more breaking changes to downstream users or more work for us to continue supporting it. Not saying it is going to happen any time soon, but I'm hopeful that it might happen eventually. |
Of course I only consider the "add some simple dynamic loading capabilities" approach. Others are not plug-ins at all and we need something that should work with Can your prototype be used as follows?
Not against this.
Yes, that's basically what I meant.
Yes.
Imagine you want to apply a lot of IA into every RTP packet. This is probably async stuff. I wouldn't assume sync comm. |
My example was decoding audio and it is similar to yours. In that case the plugin can create any threads that it needs to process the rtp packets and not block the mediasoup event loop. |
I agree it would mean more breaking changes. I think at least initially the plugins feature should be used only for very advanced use cases / developers willing to pay the extra effort and risk of breaking changes. It could potentially be enabled during builds (#ifdef PLUGINS_SUPPORT) and only people really needing it builds its own version of mediasou pwith that support enabled. |
Something that worries me about the approach in your example is that there are no boundaries about what a plugin can do as it receives the core instances. It could, for example, call We already have a communication framework between the core (Worker) and Node and Rust. Such framework provides a control and data message format (JSON in v3, flatbuffers very soon in v3) as well as the means to handle those in a secure way. There are currently 2 ways we exchange those control and data messages (defined in
IMHO we should use this very same framework (reusing the same message formats) by adding a third way to exchange these messages between the core and, now, C++:
If there was the need to send async messages from plugins to the core it could use [*]: New messages would be needed to be created as for example notifications to tell the plugin manager that a new transport, [data]consumer, [data]producer, etc has been created, etc. |
@jmillan the thing here is that worker plugins should just run on C++ without interacting with the Node/Rust layer. I agree that they should NOT be able to close transports or producers or consumers since that would ruin the whole state, but this is about limiting the API exposed to them. Note that, even if we force plugins to communicate with the worker via uv_async, same problem would happen if we allow the plug-in to close a producer directly in C++, so I don't see any benefit on forcing plugins to use flatbuffers (AKA a Channel) to communicate with worker. We use a Channel to communicate Node and Worker because those are separate processes, but why should make plugins run into a separate process? They could be native threads instead and hence they share memory with the Worker. Imagine a plug-in that receives RTP packets and tells the worker to NOT relay original packets. Instead, the plug-in collects some received packets, transcodes them and eventually tells the worker to send a new packet to X consumer. We need av_async since the plug-in may need to send that new packet at any time (and not as a result of a packet received from the worker), and we don't want the plug-in to have direct access to the entire consumer. So we can just provide the plug-in with some limited API such as sendRtpPacket(packet, consumerId). |
Of course, this communication is about worker and plugins only. I was exposing a way to communicate both: I agree though that there is no need to serialize data we are passing to the same process, but I want to reiterate the importance of limiting the access of a plugin to the overall instances. |
Is the proposal in the feature description enough to address this?: "Methods: Initially all the public interface of mediasoup objects (Transports, Consumers, Producers... ) could be used from plugins but some interfaces could be extracted (TransportApi, ConsumerApi, ProducerApi...) so that plugins don't see the whole interface" |
Definitely a reduced interface should be exposed, yes. |
But this is not in sync with what I said, which was:
So how does your comment match those requirements that I thought we had already agreed on? I insist: I don't want to pass Consumer instances to plug-ins. So we don't pass them, that's all. It doesn't mean that the only approach to satisfy it is by using a channel between plugins and worker. And BTW, if the communication between plug-in and worker is async (it cannot be sync at all) then communication between worker and plugin must also be async, otherwise the plug-in may run two block of code at the same time, something that we probably don't want. But this just means that we use uv_async in both directions and we make plugins work within a separate libuv loop, that's all. |
There is a misunderstanding:
Of course a plugin would have a subset of requests/notifications, as it cannot close a consumer for consistency.
I already agreed.
I didn't said that this was the only approach. I was justifying why I did propose it in the first place. To be clear. I'm not advocating for serializing data into any format or using a specific communication channel. |
I would suggest to pick 3 relevant use cases and write the plugins (it can be pseudocode) to see how it looks like with each approach. Some examples that could cover a large spectrum of use cases could be:
There are a couple of things suggested by @ibc (async communication or no access to consumers) that I'm not sure how compatible they are with those use cases and I think that could help to clarify it. I'm happy to write the pseudocode for those selected use cases using my proposed approach if that helps. |
While I see the benefit of having a plugin system in mediasoup I need to add some points around it: We do offer a control layer on top of the worker in TS, Rust (and there is also a Go implementation).
What would make the worker agnostic of the language the plugins are written and hence do not discard the possibility to write the plugins in Rust in a future? If a project using TS mediasoup wanted to write a Native plugin we would first need to change how we launch the worker and communicate with it in a similar way we do in Rust. Not by spawning a process as we do in TS but by creating a thread. This would be done from a native Node module written in C++. Once the thread is created the communication from the Node module to the worker is done via In essence, having the C++ worker launched from a native C++ Node module would have the following benefits:
Of course that requires a previous work of launching and communicating with the Worker via a native module in Node. Let's keep this in mind, and if for some reason, we implement the plugin in the worker-land now, let's be ready to remove it once we are able to do it the other way as it seems to be the way to go in my opinion. Am I alone in this thinking? |
Thx @jmillan for taking the time. Some questions for me to understand better the proposal: 1/ With that approach how is my C++ plugin class going to connect to the rest of mediasoup? What would be those methods "that the native module has previously registered"? Do they exist already? For example if my plugin needs to receive some datachannels messages how would that code look like? 2/ I guess the mediasoup-worker (not the libmediasoup-worker) is the equivalent of that node native module today, right? Why can't it be integrated there then? FWIW I would consider the difference between languages that can "control" mediasoup (like Go, Rust or Javascript) and languages that you can use to extend mediasoup (like C++ and Rust). For example in Redis you can only write modules in C even if you have redis libraries/connectors for any language. Note: We couldn't wait so we are already using the plugins approach described initially so happy to provide feedback from real usage if needed at some point. |
|
I've been giving this a second thought and it seems perfectly fine having a plugin system as it is suggested here[*], which is how, AFAIK, they are typically built. [*]: I mean having a plugin system in the core of the worker that calls the methods of the loaded plugins. I've probably been distracted by the fact that the example plugin given here adds code inside the worker, while a real plugin should be able to be loaded as a .so library. Such library can be implemented in C++, Rust or other language of choose that allows it. Apart from that, the plugins should have a way to be controlled, ie: enable for Producer X only, and such, and that was already commented here in this comment by @ibc #1175 (comment).
Is this your ultimate goal for the plugin system? |
Feature Request (more like a feedback request)
Protoype: ggarber@14a3ba0
Some use cases require custom processing of Media and Data packets and because of the performance implications it is not always possible to forward to those packets in real time from C++ to JS and then back from JS to C++ using DirectTransport capability.
Some examples of those use cases could be:
Nowadays the solution in these cases is to use an additional server and forward those messages from mediasoup to this server but that adds some extra complexity in terms of orchestration and also maintaining that additional server. The other solution today is to use Rust instead of JS library but that's not an option for all the teams.
The alternative is to provide support to extend mediasoup C++ layer with some custom code (let's call it plugins for now) so that developers can extend the standard capabilities of mediasoup with their use case specific logic.
The main components of that approach would be:
A) The API exposed by mediasoup to those plugins.
B) How those plugins are loaded.
API exposed to those plugins
It needs to be a mix of methods and events.
Methods: Initially all the public interface of mediasoup objects (Transports, Consumers, Producers... ) could be used from plugins but some interfaces could be extracted (TransportApi, ConsumerApi, ProducerApi...) so that plugins don't see the whole interface.
Events: mediasoup needs to expose a mechanism for plugins to listen for events. This can be in the form of events that plugins can be attached for (see prototype).
Loading of plugins
There are different alternatives:
worker.loadPlugins('xxxx.so', 'yyyy.so')
.The text was updated successfully, but these errors were encountered: