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

Endless negotiation loop #191

Closed
k0nserv opened this issue May 17, 2022 · 2 comments
Closed

Endless negotiation loop #191

k0nserv opened this issue May 17, 2022 · 2 comments

Comments

@k0nserv
Copy link
Member

k0nserv commented May 17, 2022

I've encountered a case where on_negotiation_needed fires and if you negotiate as a consequence of that and endless offer answer loop will occur.

Reproduction steps

  1. Browser sends an offer
  2. Server sends an answer
  3. Optionally step 1 and 2 happen a few times, for example by first negotiating a data channel and then a video track(Video track is sendonly on the browser side and recvonly on the server side).
  4. We remove the video track with removeTrack in the browser
  5. The browser sends an offer with the diff:
@@ -36,7 +36,7 @@

 a=extmap:9 urn:ietf:params:rtp-hdrext:sdes:mid
 a=extmap:10 urn:ietf:params:rtp-hdrext:sdes:rtp-stream-id
 a=extmap:11 urn:ietf:params:rtp-hdrext:sdes:repaired-rtp-stream-id
-a=sendonly
+a=inactive
 a=msid:743400ce-3696-4cb3-b1a2-3fef7647be68 952e921c-dbbc-46ce-a8b4-f4bfa96972c9
 a=rtcp-mux
 a=rtcp-rsize
  1. server creates an answer with the diff
@@ -40,5 +40,5 @@

 a=extmap:4 http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01
 a=extmap:9 urn:ietf:params:rtp-hdrext:sdes:mid
 a=extmap:11 urn:ietf:params:rtp-hdrext:sdes:repaired-rtp-stream-id
-a=recvonly
+a=inactive
  1. The connection status reach "stable" on the server side.
  2. on_negotiation_needed fires
  3. We create an offer and perform a negotiation
  4. Go to step 8.

As we loop in step 8 and 9 the only thing that will differ for each offer and answer is the o= line, this is according to the specification.

Details

My assumption here is that after step 7 we don't expect on_negotiation_needed to fire, as evident by the fact that there is no meaningful change to the generated offer.

The reason we keep firing on_negotiation_needed is that the check in check_negotiation_needed that corresponds to step 5.4 of the check if negotiation needed process indicates we should negotiate.

The transceiver in question is stopped because we stop it when the remote description is applied with an inactive direction:

t.stop().await?;

Workaround

It strikes me that a local fix here could be to generate an offer, then diff it against the current local description, and stop the negotiation if only the o= line differs.

@k0nserv
Copy link
Member Author

k0nserv commented May 18, 2022

Also related to this, the specification(RFC8829 Section 5.2.1) says:

An "m=" section is generated for each RtpTransceiver that has been added to the PeerConnection, excluding any stopped RtpTransceivers;(...)

But webrtc-rs includes the m= lines for this transceiver which causes this problem. That said, I don't think the transceiver should be stopped at all when its direction becomes inactive.

@k0nserv
Copy link
Member Author

k0nserv commented May 23, 2022

This issue duplicates #191 so I'll close it

@k0nserv k0nserv closed this as completed May 23, 2022
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

No branches or pull requests

1 participant