Skip to content

Commit

Permalink
update electron and native to accept list of IceServers
Browse files Browse the repository at this point in the history
enable PruneTurnPorts field trial to prevent excessive resource usage 
when providing more candidates
  • Loading branch information
adel-signal authored Feb 16, 2024
1 parent 4f7d413 commit 58f0ab4
Show file tree
Hide file tree
Showing 14 changed files with 92 additions and 62 deletions.
16 changes: 8 additions & 8 deletions bin/fetch-artifact.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@
UNVERIFIED_DOWNLOAD_NAME = "unverified.tmp"

PREBUILD_CHECKSUMS = {
'android': '49d569a1e69630f6fcb5374079a0f04c986f407daf24da9039e7444fc3b601fe',
'ios': '0c3274547678276b0cf38ee715d73072c2b39e8114ebddf860976d3de63eaa45',
'linux-arm64': '098a6e95f93e0272035249969df004b7ae5cceb0433c90d40b3e3da9d1d6f4cb',
'linux-x64': '51c8579c2ea85fb63260b857fc58f771a3a3443ecbcebf2ac1c85532852cf978',
'mac-arm64': 'a877715351f209e99478e9b5fec5a9c152eb2f18243a8cda0e9485f0140ede61',
'mac-x64': '52cc7d43e756738af5eb75cda5497028eb1e47ff67170d244c2cea2f034f5061',
'windows-arm64': '38b44c233cb518dd13a990dffd97efed9691c05df5c2c0b64471af7576bc5304',
'windows-x64': '94266f6bbe7d2331f330276e08cc7a04039a22507bab6ef76c559b6a8f22b696',
'android': '96d57094ac762aa4ab50e19eae623c77bbb2eb02504b4ddb3717a0a396f7ceea',
'ios': 'bef1c186ea5522dc8a6d3c025a53efe210f55673e3cd09e125ca811da67a3f50',
'linux-arm64': '652498b7bdbd2f764535d9c4763c9a752f3ce904109e327596369f1bb6c922dc',
'linux-x64': 'cc34122e83b7218f58e9857747321925f91bc9b696bf72cfad8cd6d5258d371b',
'mac-arm64': '47142365fc812364ffd31827315b489e1c6b287475e5597e792a0b7296d6e04d',
'mac-x64': 'aa0382331b702ca0d770899e3ddc3f052880eb22f6dfd6e34abb1fb727c896b5',
'windows-arm64': 'a401bd83505bf18a2b56cc48cf42e1783e1221fd89cb60fb63caf44d56293c11',
'windows-x64': '6a5535d1a751c7ed231d6170af42d0c74000d53da79b3ea91da44d7f33565644',
}


Expand Down
5 changes: 4 additions & 1 deletion call_sim/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,10 @@ impl Default for CallConfig {
relay_password: "test".to_string(),
force_relay: false,
// By default, all tests will disable the ANY port allocator setting.
field_trials: vec!["RingRTC-AnyAddressPortsKillSwitch/Enabled".to_string()],
field_trials: vec![
"RingRTC-AnyAddressPortsKillSwitch/Enabled".to_string(),
"RingRTC-PruneTurnPorts/Enabled".to_string(),
],
extra_cli_args: vec![],
stats_interval_secs: 1,
stats_initial_offset_secs: 0,
Expand Down
2 changes: 1 addition & 1 deletion config/version.properties
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
webrtc.version=6099b
webrtc.version=6099c

ringrtc.version.major=2
ringrtc.version.minor=37
Expand Down
1 change: 1 addition & 0 deletions src/android/api/org/signal/ringrtc/CallManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ public static void initialize(Context applicationContext, Log.Logger logger, Map

Map<String, String> fieldTrialsWithDefaults = new HashMap<>();
fieldTrialsWithDefaults.put("WebRTC-Audio-OpusSetSignalVoiceWithDtx", "Enabled");
fieldTrialsWithDefaults.put("RingRTC-PruneTurnPorts", "Enabled");
fieldTrialsWithDefaults.putAll(fieldTrials);

String fieldTrialsString = buildFieldTrialsString(fieldTrialsWithDefaults);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ public class CallManagerGlobal {
let fieldTrialsWithDefaults = fieldTrials.merging([
"RingRTC-AnyAddressPortsKillSwitch": "Enabled",
"WebRTC-Audio-OpusSetSignalVoiceWithDtx": "Enabled",
"RingRTC-PruneTurnPorts": "Enabled",
]) { (provided, _) in provided }
RTCInitFieldTrialDictionary(fieldTrialsWithDefaults)
Logger.info("Initialized field trials with \(fieldTrialsWithDefaults)")
Expand Down
14 changes: 5 additions & 9 deletions src/node/ringrtc/Service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class NativeCallManager {
{
'RingRTC-AnyAddressPortsKillSwitch': 'Enabled',
'WebRTC-Audio-OpusSetSignalVoiceWithDtx': 'Enabled',
'RingRTC-PruneTurnPorts': 'Enabled',
},
config.field_trials
);
Expand Down Expand Up @@ -516,10 +517,7 @@ export class RingRTCType {
sillyDeadlockProtection(() => {
this.callManager.proceed(
callId,
settings.iceServer.username || '',
settings.iceServer.password || '',
settings.iceServer.hostname || '',
settings.iceServer.urls,
settings.iceServers,
settings.hideIp,
settings.dataMode,
settings.audioLevelsIntervalMillis || 0
Expand Down Expand Up @@ -1755,7 +1753,7 @@ export class RingRTCType {
}

export interface CallSettings {
iceServer: IceServer;
iceServers: Array<IceServer>;
hideIp: boolean;
dataMode: DataMode;
audioLevelsIntervalMillis?: number;
Expand All @@ -1764,6 +1762,7 @@ export interface CallSettings {
interface IceServer {
username?: string;
password?: string;
/** Provide hostname when urls contain IP addresses instead of hostname */
hostname?: string;
urls: Array<string>;
}
Expand Down Expand Up @@ -2710,10 +2709,7 @@ export interface CallManager {
): CallId;
proceed(
callId: CallId,
iceServerUsername: string,
iceServerPassword: string,
iceServerHostname: string,
iceServerUrls: Array<string>,
iceServers: Array<IceServer>,
hideIp: boolean,
dataMode: DataMode,
audioLevelsIntervalMillis: number
Expand Down
17 changes: 14 additions & 3 deletions src/node/test/CallingClass.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,20 @@ export class CallingClass {
}

return {
iceServer: {
urls: ['stun:turn3.voip.signal.org'],
},
iceServers: [
{
hostname: '',
username: 'name',
password: 'pass',
urls: ['stun:turn3.voip.signal.org'],
},
{
hostname: 'example.org',
username: 'name',
password: 'pass',
urls: ['stun:123.123.123.1'],
},
],
hideIp: false,
dataMode: DataMode.Normal,
};
Expand Down
4 changes: 2 additions & 2 deletions src/rust/src/bin/call_sim-cli/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl CallEndpoint {
let peer_id = PeerId::from(peer_id);

// To send across threads
let ice_server = ice_server.clone();
let ice_servers = vec![ice_server.clone()];

Ok(Self::from_actor(
peer_id.clone(),
Expand Down Expand Up @@ -158,7 +158,7 @@ impl CallEndpoint {
pcf.create_outgoing_video_track(&outgoing_video_source)?;
let call_context = NativeCallContext::new(
hide_ip,
ice_server,
ice_servers,
outgoing_audio_track,
outgoing_video_track,
incoming_video_sink.unwrap_or_else(|| {
Expand Down
6 changes: 3 additions & 3 deletions src/rust/src/core/group_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use crate::{
AudioEncoderConfig, AudioTrack, VideoFrame, VideoFrameMetadata, VideoSink, VideoTrack,
},
peer_connection::{AudioLevel, PeerConnection, ReceivedAudioLevel, SendRates},
peer_connection_factory::{self as pcf, IceServer, PeerConnectionFactory},
peer_connection_factory::{self as pcf, PeerConnectionFactory},
peer_connection_observer::{
IceConnectionState, NetworkRoute, PeerConnectionObserver, PeerConnectionObserverTrait,
},
Expand Down Expand Up @@ -1119,15 +1119,15 @@ impl Client {
let audio_jitter_buffer_max_packets = 50;
let audio_jitter_buffer_max_target_delay_ms = 500;
let audio_rtcp_report_interval_ms = 5000;
let ice_server = IceServer::none();
let ice_servers = vec![];
let peer_connection = peer_connection_factory
.create_peer_connection(
peer_connection_observer,
pcf::RffiPeerConnectionKind::GroupCall,
audio_jitter_buffer_max_packets,
audio_jitter_buffer_max_target_delay_ms,
audio_rtcp_report_interval_ms,
&ice_server,
&ice_servers,
outgoing_audio_track,
outgoing_video_track,
)
Expand Down
57 changes: 32 additions & 25 deletions src/rust/src/electron.rs
Original file line number Diff line number Diff line change
Expand Up @@ -733,33 +733,40 @@ fn cancelGroupRing(mut cx: FunctionContext) -> JsResult<JsValue> {
#[allow(non_snake_case)]
fn proceed(mut cx: FunctionContext) -> JsResult<JsValue> {
let call_id = CallId::new(get_id_arg(&mut cx, 0));
let ice_server_username = cx.argument::<JsString>(1)?.value(&mut cx);
let ice_server_password = cx.argument::<JsString>(2)?.value(&mut cx);
let ice_server_hostname = cx.argument::<JsString>(3)?.value(&mut cx);
let js_ice_server_urls = cx.argument::<JsArray>(4)?;
let hide_ip = cx.argument::<JsBoolean>(5)?.value(&mut cx);
let data_mode = cx.argument::<JsNumber>(6)?.value(&mut cx) as i32;
let audio_levels_interval_millis = cx.argument::<JsNumber>(7)?.value(&mut cx) as u64;

let mut ice_server_urls = Vec::with_capacity(js_ice_server_urls.len(&mut cx) as usize);
for i in 0..js_ice_server_urls.len(&mut cx) {
let url: String = js_ice_server_urls
.get::<JsString, _, _>(&mut cx, i)?
.value(&mut cx);
ice_server_urls.push(url);
}
let js_ice_servers = cx.argument::<JsArray>(1)?;
let hide_ip = cx.argument::<JsBoolean>(2)?.value(&mut cx);
let data_mode = cx.argument::<JsNumber>(3)?.value(&mut cx) as i32;
let audio_levels_interval_millis = cx.argument::<JsNumber>(4)?.value(&mut cx) as u64;

info!("proceed(): callId: {}, hideIp: {}", call_id, hide_ip);
for ice_server_url in &ice_server_urls {
info!(" server: {}", ice_server_url);
}
let mut ice_servers = Vec::new();
for i in 0..js_ice_servers.len(&mut cx) {
let obj = js_ice_servers.get::<JsObject, _, _>(&mut cx, i)?;
let username = obj
.get::<JsString, _, _>(&mut cx, "username")
.map_or("".to_string(), |handle| handle.value(&mut cx));
let password: String = obj
.get::<JsString, _, _>(&mut cx, "password")
.map_or("".to_string(), |handle| handle.value(&mut cx));
let hostname = obj
.get::<JsString, _, _>(&mut cx, "hostname")
.map_or("".to_string(), |handle| handle.value(&mut cx));
let js_ice_server_urls = obj
.get_opt::<JsArray, _, _>(&mut cx, "urls")?
.expect("ice urls");

let mut ice_server_urls = Vec::with_capacity(js_ice_server_urls.len(&mut cx) as usize);
for i in 0..js_ice_server_urls.len(&mut cx) {
let url: String = js_ice_server_urls
.get::<JsString, _, _>(&mut cx, i)?
.value(&mut cx);
info!(" server: {}", url);
ice_server_urls.push(url);
}

let ice_server = IceServer::new(
ice_server_username,
ice_server_password,
ice_server_hostname,
ice_server_urls,
);
let ice_server = IceServer::new(username, password, hostname, ice_server_urls);
ice_servers.push(ice_server);
}

let audio_levels_interval = if audio_levels_interval_millis == 0 {
None
Expand All @@ -770,7 +777,7 @@ fn proceed(mut cx: FunctionContext) -> JsResult<JsValue> {
with_call_endpoint(&mut cx, |endpoint| {
let call_context = NativeCallContext::new(
hide_ip,
ice_server,
ice_servers,
endpoint.outgoing_audio_track.clone(),
endpoint.outgoing_video_track.clone(),
endpoint.incoming_video_sink.clone(),
Expand Down
8 changes: 4 additions & 4 deletions src/rust/src/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use crate::webrtc::peer_connection_observer::{NetworkRoute, PeerConnectionObserv
#[derive(Clone)]
pub struct NativeCallContext {
hide_ip: bool,
ice_server: IceServer,
ice_servers: Vec<IceServer>,
outgoing_audio_track: AudioTrack,
outgoing_video_track: VideoTrack,
incoming_video_sink: Box<dyn VideoSink>,
Expand All @@ -44,14 +44,14 @@ pub struct NativeCallContext {
impl NativeCallContext {
pub fn new(
hide_ip: bool,
ice_server: IceServer,
ice_servers: Vec<IceServer>,
outgoing_audio_track: AudioTrack,
outgoing_video_track: VideoTrack,
incoming_video_sink: Box<dyn VideoSink>,
) -> Self {
Self {
hide_ip,
ice_server,
ice_servers,
outgoing_audio_track,
outgoing_video_track,
incoming_video_sink,
Expand Down Expand Up @@ -455,7 +455,7 @@ impl Platform for NativePlatform {
.call_config()
.audio_jitter_buffer_max_target_delay_ms,
connection.call_config().audio_rtcp_report_interval_ms,
&context.ice_server,
&context.ice_servers,
context.outgoing_audio_track.clone(),
Some(context.outgoing_video_track.clone()),
)?;
Expand Down
4 changes: 2 additions & 2 deletions src/rust/src/webrtc/ffi/peer_connection_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::webrtc::ffi::peer_connection_observer::RffiPeerConnectionObserver;
#[cfg(feature = "injectable_network")]
use crate::webrtc::injectable_network::RffiInjectableNetwork;
use crate::webrtc::peer_connection_factory::{
RffiAudioConfig, RffiIceServer, RffiPeerConnectionKind,
RffiAudioConfig, RffiIceServers, RffiPeerConnectionKind,
};
#[cfg(feature = "native")]
use std::os::raw::c_char;
Expand Down Expand Up @@ -64,7 +64,7 @@ extern "C" {
audio_jitter_buffer_max_packets: isize,
audio_jitter_buffer_max_target_delay_ms: isize,
audio_rtcp_report_interval_ms: isize,
ice_server: RffiIceServer,
ice_servers: RffiIceServers,
outgoing_audio_track: webrtc::ptr::BorrowedRc<RffiAudioTrack>,
outgoing_video_track: webrtc::ptr::BorrowedRc<RffiVideoTrack>,
) -> webrtc::ptr::OwnedRc<RffiPeerConnection>;
Expand Down
15 changes: 13 additions & 2 deletions src/rust/src/webrtc/peer_connection_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ impl IceServer {
}
}

#[repr(C)]
pub struct RffiIceServers {
servers: webrtc::ptr::Borrowed<RffiIceServer>,
servers_size: usize,
}

/// Describes an audio input or output device.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct AudioDevice {
Expand Down Expand Up @@ -272,7 +278,7 @@ impl PeerConnectionFactory {
audio_jitter_buffer_max_packets: isize,
audio_jitter_buffer_max_target_delay_ms: isize,
audio_rtcp_report_interval_ms: isize,
ice_servers: &IceServer,
ice_servers: &[IceServer],
outgoing_audio_track: AudioTrack,
outgoing_video_track: Option<VideoTrack>,
) -> Result<PeerConnection> {
Expand All @@ -288,6 +294,11 @@ impl PeerConnectionFactory {
// we do this by passing a webrtc::ptr::Unique<RffiPeerConnectionObserver> to
// the Rust-level PeerConnection and let it own it.
let pc_observer_rffi = pc_observer.into_rffi();
let servers: Vec<RffiIceServer> = ice_servers.iter().map(|s| s.rffi()).collect();
let rffi_ice_servers = RffiIceServers {
servers: webrtc::ptr::Borrowed::from_ptr(servers.as_ptr()),
servers_size: servers.len(),
};

let rffi = webrtc::Arc::from_owned(unsafe {
pcf::Rust_createPeerConnection(
Expand All @@ -297,7 +308,7 @@ impl PeerConnectionFactory {
audio_jitter_buffer_max_packets,
audio_jitter_buffer_max_target_delay_ms,
audio_rtcp_report_interval_ms,
ice_servers.rffi(),
rffi_ice_servers,
outgoing_audio_track.rffi().as_borrowed(),
outgoing_video_track
.map_or_else(webrtc::ptr::BorrowedRc::null, |outgoing_video_track| {
Expand Down
4 changes: 2 additions & 2 deletions src/rust/src/webrtc/sim/peer_connection_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

use crate::webrtc;
use crate::webrtc::peer_connection_factory::{
RffiAudioConfig, RffiIceServer, RffiPeerConnectionKind,
RffiAudioConfig, RffiIceServers, RffiPeerConnectionKind,
};
use crate::webrtc::sim::media::{
RffiAudioTrack, RffiVideoSource, RffiVideoTrack, FAKE_AUDIO_TRACK, FAKE_VIDEO_SOURCE,
Expand Down Expand Up @@ -51,7 +51,7 @@ pub unsafe fn Rust_createPeerConnection(
_audio_jitter_buffer_max_packets: isize,
_audio_jitter_buffer_max_target_delay_ms: isize,
_audio_rtcp_report_interval_ms: isize,
_ice_server: RffiIceServer,
_ice_servers: RffiIceServers,
_outgoing_audio_track: webrtc::ptr::BorrowedRc<RffiAudioTrack>,
_outgoing_video_track: webrtc::ptr::BorrowedRc<RffiVideoTrack>,
) -> webrtc::ptr::OwnedRc<RffiPeerConnection> {
Expand Down

0 comments on commit 58f0ab4

Please sign in to comment.