Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Implement ICE restart (#27) #138

Merged
merged 31 commits into from
Sep 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
38ece42
Add ICE restart
evdokimovs Sep 4, 2020
40a7240
Implement ICE restart on Peer connection fail
evdokimovs Sep 4, 2020
8365d18
Refactor
evdokimovs Sep 4, 2020
6e29e9e
Add PeerMetric::PeerConnectionState adding
evdokimovs Sep 4, 2020
3b3ea31
Minor fmt fix
evdokimovs Sep 4, 2020
8150584
Add docs
evdokimovs Sep 7, 2020
2b21872
Add IceRestart deduping
evdokimovs Sep 7, 2020
fa30c04
Rename TracksApplied to PeerUpdated
evdokimovs Sep 7, 2020
ac54304
Fix mockall panic [run ci]
evdokimovs Sep 7, 2020
7da8732
Reread
evdokimovs Sep 8, 2020
6752a10
Add E2E test for ICE restart [run ci]
evdokimovs Sep 8, 2020
09eb686
Add more tests [run ci]
evdokimovs Sep 8, 2020
0708b5d
Debug E2E on CI [run ci]
evdokimovs Sep 9, 2020
283bbe5
Merge branch 'master' into restart-ice
evdokimovs Sep 9, 2020
575c13c
Final reread [run ci]
evdokimovs Sep 9, 2020
6acc9ff
Update CHANGELOG
evdokimovs Sep 9, 2020
f5aebd2
Add delay to E2E test, minor fixes [run ci]
evdokimovs Sep 9, 2020
13a5586
Merge branch 'master' into restart-ice
alexlapa Sep 10, 2020
9006c84
refactor, todos
alexlapa Sep 11, 2020
ff776b3
Merge branch 'master' into restart-ice
alexlapa Sep 11, 2020
216271a
Refactor, use ConnectionState in QualityMeterStatsHandler
evdokimovs Sep 11, 2020
3466453
Remove TrackChange renaming changes [run ci]
evdokimovs Sep 14, 2020
809ed3c
Fix lint [run ci]
evdokimovs Sep 14, 2020
c2f0f5a
Minor fixes
evdokimovs Sep 14, 2020
b5759a3
refactor
alexlapa Sep 16, 2020
aaf0a79
Merge branch 'master' into restart-ice
alexlapa Sep 17, 2020
b770062
fix merge
alexlapa Sep 17, 2020
869a330
Merge branch 'master' into restart-ice
alexlapa Sep 21, 2020
7442814
refactor [run ci]
alexlapa Sep 21, 2020
9b4e368
fix changelog
alexlapa Sep 21, 2020
fd9dc29
Corrections
tyranron Sep 22, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ All user visible changes to this project will be documented in this file. This p
- Emit `TracksApplied` event to create new and update existing tracks ([#105]);
- `PeerConnection` renegotiation functionality ([#105]);
- Calculate and send call quality score based on RTC stats ([#132]);
- Muting/unmuting `MediaTrack`s by receiver ([#127]).
- Muting/unmuting `MediaTrack`s by receiver ([#127]);
- Send `TrackUpdate::IceRestart` based on RTC stats analysis ([#138]).
- [Coturn] integration:
- [Coturn] sessions destroying ([#84]);
- [Coturn] stats processing ([#94]).
Expand Down Expand Up @@ -79,6 +80,7 @@ All user visible changes to this project will be documented in this file. This p
[#127]: /../../pull/127
[#132]: /../../pull/132
[#135]: /../../pull/135
[#138]: /../../pull/138



Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,4 @@ lazy_static = "1.4"
mockall = "0.8"
serial_test = "0.5"
tempfile = "3.1"
tokio = { version = "0.2", features = ["macros", "rt-threaded"] }
3 changes: 2 additions & 1 deletion jason/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ All user visible changes to this project will be documented in this file. This p
- `ApplyTracks` for muting/unmuting ([#81]);
- `AddPeerConnectionStats` with `RtcStats` ([#90]);
- Handling of RPC events:
- `TracksApplied` ([#105]);
- `TracksApplied` with `TrackUpdate::Added`, `TrackUpdate::Updated` and `TrackUpdate::IceRestart` ([#105], [#138]);
- `ConnectionQualityUpdated` ([#132]).
- Error handling:
- Library API:
Expand Down Expand Up @@ -104,6 +104,7 @@ All user visible changes to this project will be documented in this file. This p
[#127]: /../../pull/127
[#132]: /../../pull/132
[#137]: /../../pull/137
[#138]: /../../pull/138



Expand Down
1 change: 1 addition & 0 deletions jason/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ wee_alloc = { version = "0.4", optional = true }
"RtcIceConnectionState",
"RtcIceServer",
"RtcIceTransportPolicy",
"RtcOfferOptions",
"RtcPeerConnection", "RtcPeerConnectionIceEvent",
"RtcRtpReceiver", "RtcRtpSender",
"RtcRtpTransceiver", "RtcRtpTransceiverDirection",
Expand Down
3 changes: 3 additions & 0 deletions jason/src/api/room.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1002,6 +1002,9 @@ impl EventHandler for InnerRoom {
TrackUpdate::Updated(track_patch) => {
patches.push(track_patch);
}
TrackUpdate::IceRestart => {
peer.restart_ice();
}
}
}
peer.patch_tracks(patches)
Expand Down
38 changes: 31 additions & 7 deletions jason/src/peer/conn.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
use std::{cell::RefCell, convert::TryFrom as _, rc::Rc};
use std::{
cell::{Cell, RefCell},
convert::TryFrom as _,
rc::Rc,
};

use derive_more::{Display, From};
use medea_client_api_proto::{
Expand All @@ -8,7 +12,7 @@ use tracerr::Traced;
use wasm_bindgen_futures::JsFuture;
use web_sys::{
Event, RtcBundlePolicy, RtcConfiguration, RtcIceCandidateInit,
RtcIceConnectionState, RtcIceTransportPolicy,
RtcIceConnectionState, RtcIceTransportPolicy, RtcOfferOptions,
RtcPeerConnection as SysRtcPeerConnection, RtcPeerConnectionIceEvent,
RtcRtpTransceiver, RtcRtpTransceiverDirection, RtcRtpTransceiverInit,
RtcSdpType, RtcSessionDescription, RtcSessionDescriptionInit,
Expand Down Expand Up @@ -212,6 +216,10 @@ pub struct RtcPeerConnection {
/// [1]: https://w3.org/TR/webrtc/#rtcpeerconnection-interface
peer: Rc<SysRtcPeerConnection>,

/// Flag which indicates that ICE restart will be performed on next
/// [`RtcPeerConnection::create_and_set_offer`] call.
ice_restart: Cell<bool>,

/// [`onicecandidate`][2] callback of [RTCPeerConnection][1] to handle
/// [`icecandidate`][3] event. It fires when [RTCPeerConnection][1]
/// discovers a new [RTCIceCandidate][4].
Expand Down Expand Up @@ -286,6 +294,7 @@ impl RtcPeerConnection {

Ok(Self {
peer: Rc::new(peer),
ice_restart: Cell::new(false),
on_ice_candidate: RefCell::new(None),
on_ice_connection_state_changed: RefCell::new(None),
on_connection_state_changed: RefCell::new(None),
Expand Down Expand Up @@ -535,6 +544,15 @@ impl RtcPeerConnection {
Ok(())
}

/// Marks [`RtcPeerConnection`] to trigger ICE restart.
///
/// After this function returns, the offer returned by the next call to
/// [`RtcPeerConnection::create_and_set_offer`] is automatically configured
/// to trigger ICE restart.
pub fn restart_ice(&self) {
self.ice_restart.set(true);
}

/// Obtains [SDP answer][`SdpType::Answer`] from the underlying
/// [RTCPeerConnection][`SysRtcPeerConnection`] and sets it as local
/// description.
Expand Down Expand Up @@ -593,11 +611,17 @@ impl RtcPeerConnection {
pub async fn create_and_set_offer(&self) -> Result<String> {
let peer: Rc<SysRtcPeerConnection> = Rc::clone(&self.peer);

let create_offer = JsFuture::from(peer.create_offer())
.await
.map_err(Into::into)
.map_err(RTCPeerConnectionError::CreateOfferFailed)
.map_err(tracerr::wrap!())?;
let mut offer_options = RtcOfferOptions::new();
if self.ice_restart.take() {
offer_options.ice_restart(true);
}
let create_offer = JsFuture::from(
peer.create_offer_with_rtc_offer_options(&offer_options),
)
.await
.map_err(Into::into)
.map_err(RTCPeerConnectionError::CreateOfferFailed)
.map_err(tracerr::wrap!())?;
let offer = RtcSessionDescription::from(create_offer).sdp();

let mut desc = RtcSessionDescriptionInit::new(RtcSdpType::Offer);
Expand Down
9 changes: 9 additions & 0 deletions jason/src/peer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,15 @@ impl PeerConnection {
Ok(())
}

/// Marks [`PeerConnection`] to trigger ICE restart.
///
/// After this function returns, the offer returned by the next call to
/// [`PeerConnection::get_offer`] is automatically configured to trigger ICE
/// restart.
pub fn restart_ice(&self) {
self.peer.restart_ice();
}

/// Returns `true` if all [`Sender`]s audio tracks are enabled.
pub fn is_send_audio_enabled(&self) -> bool {
self.media_connections.is_send_audio_enabled()
Expand Down
2 changes: 1 addition & 1 deletion jason/src/rpc/reconnect_handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ impl ReconnectHandle {
Duration::from_millis(u64::from(max_delay)).into(),
);
backoff_delayer.delay().await;
if upgrade_or_detached!(rpc, JsValue)?
while upgrade_or_detached!(rpc, JsValue)?
.connect(token.clone())
.await
.is_err()
Expand Down
49 changes: 49 additions & 0 deletions jason/tests/peer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1055,3 +1055,52 @@ async fn new_remote_track() {
);
}
}

/// Tests that after [`PeerConnection::restart_ice`] call, `ice-pwd` and
/// `ice-ufrag` IDs will be updated in the SDP offer.
#[wasm_bindgen_test]
async fn ice_restart_works() {
fn get_ice_pwds(offer: &str) -> Vec<&str> {
offer
.lines()
.filter_map(|line| {
if line.contains("ice-pwd") {
Some(line.split(':').skip(1).next().unwrap())
} else {
None
}
})
.collect()
}

fn get_ice_ufrags(offer: &str) -> Vec<&str> {
offer
.lines()
.filter_map(|line| {
if line.contains("ice-ufrag") {
Some(line.split(':').skip(1).next().unwrap())
} else {
None
}
})
.collect()
}

let peers = InterconnectedPeers::new().await;
let sdp_offer_before = peers.first_peer.get_offer(vec![]).await.unwrap();
let ice_pwds_before = get_ice_pwds(&sdp_offer_before);
let ice_ufrags_before = get_ice_ufrags(&sdp_offer_before);
peers.first_peer.restart_ice();
let sdp_offer_after = peers.first_peer.get_offer(vec![]).await.unwrap();
let ice_pwds_after = get_ice_pwds(&sdp_offer_after);
let ice_ufrags_after = get_ice_ufrags(&sdp_offer_after);

ice_pwds_before
.into_iter()
.zip(ice_pwds_after.into_iter())
.for_each(|(before, after)| assert_ne!(before, after));
ice_ufrags_before
.into_iter()
.zip(ice_ufrags_after.into_iter())
.for_each(|(before, after)| assert_ne!(before, after));
}
4 changes: 3 additions & 1 deletion proto/client-api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ All user visible changes to this project will be documented in this file. This p
- `is_required` field to `AudioSettings` and `VideoSettings` ([#106]);
- `TracksApplied` event with `TrackUpdate::Updated` and `TrackUpdate::Added` variants ([#81], [#105]);
- `ConnectionQualityUpdated` event ([#132]);
- `TrackPatchEvent` and `TrackPatchCommand` types ([#127]).
- `TrackPatchEvent` and `TrackPatchCommand` types ([#127]);
- `IceRestart` variant to `TrackUpdate` ([#138]).

[#28]: /../../pull/28
[#58]: /../../pull/58
Expand All @@ -71,6 +72,7 @@ All user visible changes to this project will be documented in this file. This p
[#115]: /../../pull/115
[#132]: /../../pull/132
[#127]: /../../pull/127
[#138]: /../../pull/138



Expand Down
85 changes: 82 additions & 3 deletions proto/client-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,12 +210,44 @@ pub enum PeerMetrics {
#[cfg_attr(feature = "jason", derive(Serialize))]
#[derive(Clone, Debug, PartialEq)]
pub enum IceConnectionState {
/// ICE agent is gathering addresses or is waiting to be given remote
/// candidates.
New,

/// ICE agent has been given one or more remote candidates and is checking
/// pairs of local and remote candidates against one another to try to find
/// a compatible match, but hasn't yet found a pair which will allow the
/// `PeerConnection` to be made. It's possible that gathering of candidates
/// is also still underway.
Checking,

/// Usable pairing of local and remote candidates has been found for all
/// components of the connection, and the connection has been established.
/// It's possible that gathering is still underway, and it's also possible
/// that the ICE agent is still checking candidates against one another
/// looking for a better connection to use.
Connected,

/// ICE agent has finished gathering candidates, has checked all pairs
/// against one another, and has found a connection for all components.
Completed,

/// ICE candidate has checked all candidates pairs against one another and
/// has failed to find compatible matches for all components of the
/// connection. It is, however, possible that the ICE agent did find
/// compatible connections for some components.
Failed,

/// Checks to ensure that components are still connected failed for at
/// least one component of the `PeerConnection`. This is a less stringent
/// test than [`IceConnectionState::Failed`] and may trigger intermittently
/// and resolve just as spontaneously on less reliable networks, or during
/// temporary disconnections. When the problem resolves, the connection may
/// return to the [`IceConnectionState::Connected`] state.
Disconnected,

/// ICE agent for this `PeerConnection` has shut down and is no longer
/// handling requests.
Closed,
}

Expand All @@ -224,12 +256,56 @@ pub enum IceConnectionState {
#[cfg_attr(feature = "jason", derive(Serialize))]
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum PeerConnectionState {
Closed,
Failed,
Disconnected,
/// At least one of the connection's ICE transports are in the
/// [`IceConnectionState::New`] state, and none of them are in one
/// of the following states: [`IceConnectionState::Connecting`],
/// [`IceConnectionState::Checking`], [`IceConnectionState::Failed`], or
/// [`IceConnectionState::Disconnected`], or all of the connection's
/// transports are in the [`IceConnectionState::Closed`] state.
New,

/// One or more of the ICE transports are currently in the process of
/// establishing a connection; that is, their [`IceConnectionState`] is
/// either [`IceConnectionState::Checking`] or
/// [`IceConnectionState::Connected`], and no transports are in the
/// [`IceConnectionState::Failed`] state.
Connecting,

/// Every ICE transport used by the connection is either in use (state
/// [`IceConnectionState::Connected`] or [`IceConnectionState::Completed`])
/// or is closed ([`IceConnectionState::Closed`]); in addition,
/// at least one transport is either [`IceConnectionState::Connected`] or
/// [`IceConnectionState::Completed`].
Connected,

/// At least one of the ICE transports for the connection is in the
/// [`IceConnectionState::Disconnected`] state and none of the other
/// transports are in the state [`IceConnectionState::Failed`],
/// [`IceConnectionState::Connecting`], or
/// [`IceConnectionState::Checking`].
Disconnected,

/// One or more of the ICE transports on the connection is in the
/// [`IceConnectionState::Failed`] state.
Failed,

/// The `PeerConnection` is closed.
Closed,
}

impl From<IceConnectionState> for PeerConnectionState {
fn from(ice_con_state: IceConnectionState) -> Self {
use IceConnectionState as IceState;

match ice_con_state {
IceState::New => Self::New,
IceState::Checking => Self::Connecting,
IceState::Connected | IceState::Completed => Self::Connected,
IceState::Failed => Self::Failed,
IceState::Disconnected => Self::Disconnected,
IceState::Closed => Self::Closed,
}
}
}

/// Reason of disconnecting Web Client from Media Server.
Expand Down Expand Up @@ -354,6 +430,9 @@ pub enum TrackUpdate {
/// [`Track`] should be updated by this [`TrackPatchEvent`] in the `Peer`.
/// Can only refer tracks already known to the `Peer`.
Updated(TrackPatchEvent),

/// [`Peer`] should start ICE restart process on the next renegotiation.
IceRestart,
}

/// Represents [RTCIceCandidateInit][1] object.
Expand Down
2 changes: 1 addition & 1 deletion src/api/control/refs/local_uri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ mod specs {
let (endpoint_id, member_uri) = endpoint.take_endpoint_id();
assert_eq!(endpoint_id, String::from("endpoint_id").into());
let (member_id, room_uri) = member_uri.take_member_id();
assert_eq!(member_id, MemberId("room_element_id".to_string()));
assert_eq!(member_id, MemberId::from("room_element_id"));
let room_id = room_uri.take_room_id();
assert_eq!(room_id, RoomId::from("room_id"));
} else {
Expand Down
Loading