RTCPeerConnection.setLocalDescription() without arguments

[data-retriever]

The Perfect Negotiation document says that the set local description method shouldn't receive a argument for a perfect negotiation.

Is this feature already on the roadmap?

[k0nserv]

Not on the roadmap at the moment, but part of what you mentioned is implemented i.e. you can pass an argument to set_local_description with an empty sdp field and it will re-use the last offer but it will not detect the need to generate a new offer.

[k0nserv]

If anyone wishes to implement support for this the details are available here:

https://w3c.github.io/webrtc-pc/#dom-peerconnection-setlocaldescription

[zduny]

I've managed to achieve a connection using "perfect negotiation" scheme using my implementation: zduny@5ceff6a#diff-7df0115dc6a55f713e6ad5ede4c7808165f5f0f12e2447091ca989dbbf11dbeb

But TBH have no idea if it is fully compliant with the specs...

[dimitri-lenkov]

EDIT: The text below is just a partial solution. I think it was only working because my dummy_sdp value was being parsed. I'm going to continue working on this. It's just one of many changes that needs to be had to make rollbacks work, 😿

EDIT (2): Added another patch at the end of this comment that includes the changes necessary to omit the dummy_sdp value by introducing a new RTCSessionDescription::rollback() function.

EDIT (3): See new patch below

Click here to see old patch details

Also, the existing logic in check_next_signaling_state does not permit rollbacks from RTCSignalingState::HaveLocalOffer. The attached patch below fixes this so it's possible to manually set the local description to type Rollback, which is valid according to:

https://w3c.github.io/webrtc-pc/#set-the-session-description

From f5635c18e1122af4499fe32af3b651fc527d30af Mon Sep 17 00:00:00 2001
From: Dimitri <dimitri@github.com>
Date: Thu, 11 Jan 2024 09:36:11 -0800
Subject: pc/signaling-state: support rollbacks for have local offer

---
 webrtc/src/peer_connection/signaling_state.rs | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/webrtc/src/peer_connection/signaling_state.rs b/webrtc/src/peer_connection/signaling_state.rs
index 2edd613b..bca7716d 100644
--- a/webrtc/src/peer_connection/signaling_state.rs
+++ b/webrtc/src/peer_connection/signaling_state.rs
@@ -156,11 +156,20 @@ pub(crate) fn check_next_signaling_state(
                     }
                     _ => {}
                 }
-            } else if op == StateChangeOp::SetLocal
-                && sdp_type == RTCSdpType::Offer
-                && next == RTCSignalingState::HaveLocalOffer
-            {
-                return Ok(next);
+            } else if op == StateChangeOp::SetLocal {
+                match sdp_type {
+                    RTCSdpType::Offer => {
+                        if next == RTCSignalingState::HaveLocalOffer {
+                            return Ok(next);
+                        }
+                    },
+                    RTCSdpType::Rollback => {
+                        if next == RTCSignalingState::Stable {
+                            return Ok(next);
+                        }
+                    }
+                    _ => {}
+                }
             }
         }
         RTCSignalingState::HaveRemotePranswer => {
-- 
cgit v1.2.3

Now, the call to peer_connection.set_local_description with a SDP of type rollback will be accepted!

This also seems to solve my perfect negotiation problems, but as @zduny said: I'm not sure if this is 100% compliant with the specification (e.g. maybe there needs to be more work destroying listeners, etc. on rollback to the stable state?), but it gets me past my current block.

Also, currently, there is no way to create an SDP of type rollback, so I'm using this hack to get around that (I may make another patch to remove this dummy_sdp string from my code though):

let rollback_sdp = {
  let dummy_sdp = "v=0\r\no=- 934697676791585089 442572738 IN IP4 0.0.0.0\r\ns=-\r\nt=0 0\r\na=fingerprint:sha-256 BC:AC:A4:A4:76:A1:0F:E8:0A:97:BA:37:AD:02:44:72:A3:A4:03:6C:53:37:CA:26:54:AB:E7:BA:4A:D5:3C:9D\r\na=group:BUNDLE 0\r\nm=application 9 UDP/DTLS/SCTP webrtc-datachannel\r\nc=IN IP4 0.0.0.0\r\na=setup:actpass\r\na=mid:0\r\na=sendrecv\r\na=sctp-port:5000\r\na=ice-ufrag:fbwPVDMJduBiMCbV\r\na=ice-pwd:HuwKsQYkDRogpMHUmXHkLOCLQzoiSQtY\r\n";
  let mut s = RTCSessionDescription::offer(dummy_sdp.into())?;
  s.sdp_type = RTCSdpType::Rollback;
  s
};

peer_connection.set_local_description(rollback_sdp).await?;

---

Update: The above hackish rollback creation with dummy_sdp can be removed entirely and replaced with:

peer_connection.set_local_description(RTCSessionDescription::rollback()?).await?;

Using the following patch:

From 52748e96cea76debf3bd56dbf55acb6956b78fff Mon Sep 17 00:00:00 2001
From: Dimitri <dimitri@github.com>
Date: Thu, 11 Jan 2024 10:24:47 -0800
Subject: pc/sdp: add rollback create fn & support setting as local desc

---
 webrtc/src/peer_connection/mod.rs                  | 28 ++++++++++++----------
 .../src/peer_connection/sdp/session_description.rs | 12 ++++++++++
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/webrtc/src/peer_connection/mod.rs b/webrtc/src/peer_connection/mod.rs
index b4989cef..8caeef8b 100644
--- a/webrtc/src/peer_connection/mod.rs
+++ b/webrtc/src/peer_connection/mod.rs
@@ -1215,22 +1215,26 @@ impl RTCPeerConnection {
             current_local_description.is_some()
         };
 
-        // JSEP 5.4
-        if desc.sdp.is_empty() {
-            match desc.sdp_type {
-                RTCSdpType::Answer | RTCSdpType::Pranswer => {
-                    let last_answer = self.internal.last_answer.lock().await;
-                    desc.sdp = last_answer.clone();
-                }
-                RTCSdpType::Offer => {
-                    let last_offer = self.internal.last_offer.lock().await;
-                    desc.sdp = last_offer.clone();
+
+        if desc.sdp_type != RTCSdpType::Rollback {
+            // JSEP 5.4
+            if desc.sdp.is_empty() {
+                match desc.sdp_type {
+                    RTCSdpType::Answer | RTCSdpType::Pranswer => {
+                        let last_answer = self.internal.last_answer.lock().await;
+                        desc.sdp = last_answer.clone();
+                    }
+                    RTCSdpType::Offer => {
+                        let last_offer = self.internal.last_offer.lock().await;
+                        desc.sdp = last_offer.clone();
+                    }
+                    _ => return Err(Error::ErrPeerConnSDPTypeInvalidValueSetLocalDescription),
                 }
-                _ => return Err(Error::ErrPeerConnSDPTypeInvalidValueSetLocalDescription),
             }
+
+            desc.parsed = Some(desc.unmarshal()?);
         }
 
-        desc.parsed = Some(desc.unmarshal()?);
         self.set_description(&desc, StateChangeOp::SetLocal).await?;
 
         let we_answer = desc.sdp_type == RTCSdpType::Answer;
diff --git a/webrtc/src/peer_connection/sdp/session_description.rs b/webrtc/src/peer_connection/sdp/session_description.rs
index 7085f34d..443bfd44 100644
--- a/webrtc/src/peer_connection/sdp/session_description.rs
+++ b/webrtc/src/peer_connection/sdp/session_description.rs
@@ -66,6 +66,18 @@ impl RTCSessionDescription {
         Ok(desc)
     }
 
+    /// Create a rollback RTCSessionDescription that can be given to
+    /// an RTCPeerConnection.
+    pub fn rollback() -> Result<RTCSessionDescription> {
+        let desc = RTCSessionDescription {
+            sdp: "".into(),
+            sdp_type: RTCSdpType::Rollback,
+            parsed: None,
+        };
+
+        Ok(desc)
+    }
+
     /// Unmarshal is a helper to deserialize the sdp
     pub fn unmarshal(&self) -> Result<SessionDescription> {
         let mut reader = Cursor::new(self.sdp.as_bytes());
-- 
cgit v1.2.3

[chanq-io]

Is there any update on this? Perfect negotiation seems to be the standard pattern for making robust connections, so would be great if it could be supported out of the box. While the patches look useful for webrtc-rs devs they're not practical for average users who want to depend on stable releases, e.g. the nice rollback function shared above requires private visibility

[dimitri-lenkov]

Update to the patch from this answer for the latest changes (desc.sdp.clone_from function usage).

View patch

diff --git a/webrtc/src/peer_connection/mod.rs b/webrtc/src/peer_connection/mod.rs
index 9f99e18b..fd7e829a 100644
--- a/webrtc/src/peer_connection/mod.rs
+++ b/webrtc/src/peer_connection/mod.rs
@@ -1225,22 +1225,25 @@ impl RTCPeerConnection {
             current_local_description.is_some()
         };
 
-        // JSEP 5.4
-        if desc.sdp.is_empty() {
-            match desc.sdp_type {
-                RTCSdpType::Answer | RTCSdpType::Pranswer => {
-                    let last_answer = self.internal.last_answer.lock().await;
-                    desc.sdp.clone_from(&last_answer);
-                }
-                RTCSdpType::Offer => {
-                    let last_offer = self.internal.last_offer.lock().await;
-                    desc.sdp.clone_from(&last_offer);
+        if desc.sdp_type != RTCSdpType::Rollback {
+            // JSEP 5.4
+            if desc.sdp.is_empty() {
+                match desc.sdp_type {
+                    RTCSdpType::Answer | RTCSdpType::Pranswer => {
+                        let last_answer = self.internal.last_answer.lock().await;
+                        desc.sdp.clone_from(&last_answer);
+                    }
+                    RTCSdpType::Offer => {
+                        let last_offer = self.internal.last_offer.lock().await;
+                        desc.sdp.clone_from(&last_offer);
+                    }
+                    _ => return Err(Error::ErrPeerConnSDPTypeInvalidValueSetLocalDescription),
                 }
-                _ => return Err(Error::ErrPeerConnSDPTypeInvalidValueSetLocalDescription),
             }
+
+            desc.parsed = Some(desc.unmarshal()?);
         }
 
-        desc.parsed = Some(desc.unmarshal()?);
         self.set_description(&desc, StateChangeOp::SetLocal).await?;
 
         let we_answer = desc.sdp_type == RTCSdpType::Answer;
diff --git a/webrtc/src/peer_connection/sdp/session_description.rs b/webrtc/src/peer_connection/sdp/session_description.rs
index 7c8bc15e..38612aeb 100644
--- a/webrtc/src/peer_connection/sdp/session_description.rs
+++ b/webrtc/src/peer_connection/sdp/session_description.rs
@@ -74,6 +74,18 @@ impl RTCSessionDescription {
         Ok(desc)
     }
 
+    /// Create a rollback RTCSessionDescription that can be given to
+    /// an RTCPeerConnection.
+    pub fn rollback() -> Result<RTCSessionDescription> {
+        let desc = RTCSessionDescription {
+            sdp: "".into(),
+            sdp_type: RTCSdpType::Rollback,
+            parsed: None,
+        };
+
+        Ok(desc)
+    }
+
     /// Unmarshal is a helper to deserialize the sdp
     pub fn unmarshal(&self) -> Result<SessionDescription> {
         let mut reader = Cursor::new(self.sdp.as_bytes());
diff --git a/webrtc/src/peer_connection/signaling_state.rs b/webrtc/src/peer_connection/signaling_state.rs
index 82d39c01..cc6b9cb1 100644
--- a/webrtc/src/peer_connection/signaling_state.rs
+++ b/webrtc/src/peer_connection/signaling_state.rs
@@ -164,11 +164,20 @@ pub(crate) fn check_next_signaling_state(
                     }
                     _ => {}
                 }
-            } else if op == StateChangeOp::SetLocal
-                && sdp_type == RTCSdpType::Offer
-                && next == RTCSignalingState::HaveLocalOffer
-            {
-                return Ok(next);
+            } else if op == StateChangeOp::SetLocal {
+                match sdp_type {
+                    RTCSdpType::Offer => {
+                        if next == RTCSignalingState::HaveLocalOffer {
+                            return Ok(next);
+                        }
+                    },
+                    RTCSdpType::Rollback => {
+                        if next == RTCSignalingState::Stable {
+                            return Ok(next);
+                        }
+                    }
+                    _ => {}
+                }
             }
         }
         RTCSignalingState::HaveRemotePranswer => {