Skip to content
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

Allow AudioBridge to originate SDP offers #2366

Merged
merged 1 commit into from
Oct 5, 2020
Merged

Conversation

lminiero
Copy link
Member

The AudioBridge has always worked with a specific pattern, with respect to WebRTC negotiations: the user sends the SDP offer, the plugin answers. I've been asked a few times if the pattern could be changed, i.e., to allow the plugin to send the offer instead and let the user answer: for several reasons I've always shut those requests down. It has come to my attention, though, that the way the plugin works right now there's no way to implement WebRTC cascading of mixers: if you want to bridge two or more AudioBridge instances together (e.g., to distribute the mixing load), there's no way to do that via WebRTC, as both AudioBridge instances would be waiting for an offer from the other, and so it just wouldn't work.

This PR tries to address that, by adding a new optional "generate offer" mode in the plugin. The whole API (join, configure, etc.) remains the same, but you can ask the plugin to prepare an offer, that you then will send an answer to. The approach is very simple, as if you want an offer all you need to do is add a generate_offer: true property to the join or configure request you previously used, e.g.:

{
    "request": "join",
    "room: 1234,
    [..]
    "generate_offer": true
}

without providing any JSEP SDP yourself. The plugin will then generate a JSEP SDP offer for you in a subsequent event, and it's then up to you to prepare the related JSEP SDP answer and send it via a configure request.

Notice how I clearly said the feature is optional: if you're ok with how the AudioBridge works right now, by all means keep on using it that way, as nothing changed there and it's still the preferred way of establishing a connection. You should only use this new mode if you really need it and know what you're doing. Also notice that the pattern you choose to establish the connection in the first place MUST be used for all subsequent updates as well, if any: if you need to renegotiate and initially asked the plugin to send an offer, then you will need the plugin to send you the offer for the renegotiation as well. It's an error to send the plugin an offer update if the session was created by an offer originated by the plugin: this is done on purpose to avoid ugly situations like glare, where both user and plugin send an offer at the same time, and so enforcing the same pattern for the whole session adds some structure.

I only tested this very briefly, and considering a lot has changed in the code, I consider this PR still experimental. As such, you're encouraged to test this extensively, especially if you rely on the AudioBridge for your applications. Please test and provide feedback, because I very likely will not have the time to test much myself.

@danjenkins
Copy link
Contributor

I can confirm this seems to work pretty damn well. Will do some further testing next week

@lminiero
Copy link
Member Author

lminiero commented Oct 5, 2020

Merging 👍

@lminiero lminiero merged commit be588d8 into master Oct 5, 2020
@lminiero lminiero deleted the audiobridge-offer branch October 5, 2020 10:01
PauKerr pushed a commit to caffeinetv/janus-gateway that referenced this pull request Nov 11, 2020
@fukemy
Copy link

fukemy commented Dec 7, 2022

hi, im found this pull when searching audio bridge restart-ice problem. can you please share me some example. My problem is:

  1. I created offer from local peer after reconnected
  2. Then I setLocalDescription(offer)

Then the iceConnection become to connected state again, But I can not hear anything from this device(When talk from this device, I can hear it from other devices)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants