AudioBridge: (re) configure remote rtp

keremcadirci commented 4 months ago

This PR is intended to address the topic discussed in https://janus.discourse.group/t/audiobridge-rtp-join-sip-scenarios/1049/3
This PR enhance slightly SIP interoperability provided by the plain RTP join feature of AudioBridge, such as in the case of an 18x message with SDP. Specifically, during an outbound SIP call, there is currently no way to direct RTP issued from a SIP 18x Ringing or Session Progress (e.g., ringback tone provided by RTP or a custom message sent by the operator during ringing), into the AudioBridge room.

Solution Proposal:

Add an rtp object in the configure api

"request" : "configure",
[..] 
"rtp" : {
    "ip" : "<IP address you want media to be sent to>",
    "port" : <port you want media to be sent to>,
    "payload_type" : <payload type to use for RTP packets (optional; only needed in case Opus is used, automatic for G.711)>,
    "audiolevel_ext" : <ID of the audiolevel RTP extension, if used (optional)>,
    "fec" : <true|false, whether FEC should be enabled for the Opus stream (optional; only needed in case Opus is used)>
}

With this request we will not change local RTP port of the plain RTP participant generated with join request
We will close existing socket created by join request, then reconnect/rebind the same local RTP port with the the new remote RTP provided by configure request.
We will accept rtp object in configure request only if the participant has previously joined with plain rtp.
We will not modify any responses/events related to configure request

I believe this approach is backward compatible.

How this enhancement improve SIP interoperability? How will SIP/janus flow will be?

Before making an outbound call, make an RTP join request by providing a dummy sipSDP to janus and get janusSDP. Then Inıtiate a SIP Invite with janusSDP.
Since we have provided an SDP during Invite, SIP UAS may send a SIP 18x with an SDP. (if SIP Invite does not have SDP then 18x message will not contain any sdp)
When SIP UAC receives an 18x with SDP we will send a configure request to janus with 18x SDP. Thus Audio room participants will hear rinback tone or ring message into room
When SIP UAC receives an OK with SDP (1) we will send a configure request to janus with SIP OK SDP or (2) make a RE-Invite without SDP and rejoin to janus ....

Implementation & Tests

We have well tested with real word use cases:

Tests successfully made:

Backward compatibility: when rtp object is not present everything works as expected.
Happy path (configure rtp): We have seen in netstat that the existing socket is correctly changed after configure request. We have seen that both way RTP sessions are OK and both way audio communication is well established. the socket is closed after participant left or after session close.
Multiple confiure rtp requests on a single session works fine
Error Case: (1) allowRtpParticipants is false on room, (2) join event does not contain rtp

Not Tested:

Only pcma is tested: pcmu and opus not tested
payload_type changes on configure is not tested

lminiero commented 4 months ago

To be honest, I don't like this approach, which seems wasteful. It creates a socket already knowing it will be destroyed shortly thereafter. I'd rather see a possible alternative to RTP participants creation, where the plugin is asked to provide a port first, and the application will then provide their own later on (e.g., via a configure). This would map better to an inverted negotiation pattern (where the plugin "offers" and the application "answers"). We have both approaches already on the WebRTC side, for instance, so you can see how that's done for WebRTC to possibly reuse the same methods and just add the required attributes there.

keremcadirci commented 4 months ago

Do you suggest something like generate-offer with join request ?

In that case which of two join request would you prefer (or a 3rd suggestion):

add generate-rtp-offer attribute

{
"request" : "join",
"room" : <numeric ID of the room to join>,
    [..]
    "generate-rtp-offer: <true|false, whether audioroom reserve a local port to receive rtp. configure request should be later usedd to setup the PeerConnection\>
}

add generate-offer into rtp object

{
"request" : "join",
"room" : <numeric ID of the room to join>,
    [..]
    "rtp" : {
            "generate-offer: <true|false, whether audioroom reserve a local port to receive rtp. configure request should be later usedd to setup the PeerConnection. If true all other rtp properties are discarded\>,
            [..] <other properties are discarded\>
}
}

lminiero commented 4 months ago

I think the second one is better, since it adds the property as part of the already existing rtp container. For consistency it should use a generate_offer, though (notice the underscore), since that's how the property is called for WebRTC usage.

keremcadirci commented 3 months ago

The fallowing rtp.generate_offer is added

{
    "request" : "join",
    "room" : <numeric ID of the room to join>,
        [..]
        "rtp" : {
                "generate_offer: <true|false, whether audioroom reserve a local port to receive rtp. configure request should be later usedd to setup the PeerConnection. If true all other rtp properties are discarded\>,
                [..] <other properties are discarded\>
    }
}

@lminiero My previous remarks about limitations and tests remains the same

lminiero commented 3 months ago

That's a LOT of new code... I'm not sure I'm comfortable with that many changes in critical parts of the plugin. I'll need some time to review this.

meetecho / janus-gateway