Add support for synchronous RTP forwarder requests to SIP/NoSIP plugins#3639
Open
Add support for synchronous RTP forwarder requests to SIP/NoSIP plugins#3639
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a continuation of the work started in #3583, specifically to extend some of the functionality we added back then.
Originally, RTP forwarder support in the SIP and NoSIP plugins required you to send those request on the handle responsible for the call whose media you wanted to fork: as such, context was implicitly derived from the handle itself. While this worked, it was also quite suboptimal, as for applications wanting to have more control of the feature it meant having to actively control the handles too, which is awkward and can be problematic. This is not an issue in AudioBridge and VideoRoom (the other two plugins with RTP forwarding request), as RTP forwarding is implemented via synchronous requests there, where the specific room or publisher to forward can be easily addressed via their IDs, which was something we couldn't do when we first published #3583.
Aware of this limitation, I laid some groundwork in #3607, which added a new kind of ID specific to the SIP and NoSIP plugins, meant to uniquely identify and address specific handles/calls via a unique ID instead, which is returned to the user when they first connect and register at the plugin (in the case of NoSIP, when they first generate an offer or answer). Considering that ID was meant to externally address calls, this means we now have the missing info we needed to support a synchronous management of RTP forwarders requests too, which is basically what this PR is for.
In a nutshell, this PR extends #3583 in a backwards compatible way (meaning that the APIs we defined there will still work, if that was fine by you):
message_plugin: when you use the Admin API, the requests are synchronous (response sent right away; when you use the Janus API, they're asynchronous as before (response sent in events);unique_idproperty: when it's missing, the session context is derived from the handle on which the request was sent (as before), otherwise we address the session identified by the ID; considering that for Admin API requests there's no session, theunique_idproperty is mandatory when requests are sent there;admin_keyproperty to lock down RTP forwarding requests: when anadmin_keyproperty is set in the configuration, then all RTP forwarding requests (no matter where they're coming from, Janus or Admin API) need to include anadmin_keyproperty with that value, or the request will be rejected;stop_rtp_forwardrequest has been extended too: while you can still use the old approach of just passing a singlestream_id, you can now send astreamsarray instead (an array of integers), to get rid of multiple forwarders at the same time; the response will vary depending on what you passed, meaning it will look like before if you use the legacy syntax, and include an array of all the streams that were actually removed otherwise (passing non-existing IDs will not be an error anymore, those will simply be ignored).An example of how these changes can be beneficial is this simple curl one-liner, that shows how you can create some RTP forwarders and get a response back using a single (and simple) HTTP POST to the Admin API backend:
I tested this with both plugins and it seems to be working as expected, both using the old APIs and the new approach. Of course, if you're using this functionality somewhere, or plan to use it soon, you're encouraged to give this a try, as I expect this will be probably be merged relatively soon. Feedback welcome!