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

[flatbuffers] Fix missing options in router.createWebRtcTransport() #1185

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
23 changes: 20 additions & 3 deletions node/src/Router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,8 @@ export class Router<RouterAppData extends AppData = AppData>
listenInfos,
listenIps,
port,
enableUdp = true,
enableTcp = false,
enableUdp,
enableTcp,
preferUdp = false,
preferTcp = false,
initialAvailableOutgoingBitrate = 600000,
Expand Down Expand Up @@ -440,6 +440,19 @@ export class Router<RouterAppData extends AppData = AppData>
throw new TypeError('if given, appData must be an object');
}

// If webRtcServer is given, then do not force default values for enableUdp
// and enableTcp. Otherwise set them if unset.
if (webRtcServer)
{
enableUdp ??= true;
enableTcp ??= true;
}
else
{
enableUdp ??= true;
enableTcp ??= false;
}

// Convert deprecated TransportListenIps to TransportListenInfos.
if (listenIps)
{
Expand Down Expand Up @@ -545,7 +558,11 @@ export class Router<RouterAppData extends AppData = AppData>
webRtcServer ?
FbsWebRtcTransport.Listen.ListenServer :
FbsWebRtcTransport.Listen.ListenIndividual,
webRtcServer ? webRtcTransportListenServer : webRtcTransportListenIndividual
webRtcServer ? webRtcTransportListenServer : webRtcTransportListenIndividual,
enableUdp,
enableTcp,
preferUdp,
preferTcp
);

const requestOffset = new FbsRouter.CreateWebRtcTransportRequestT(
Expand Down
6 changes: 1 addition & 5 deletions node/src/WebRtcTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,25 +78,21 @@ export type WebRtcTransportOptionsBase<WebRtcTransportAppData> =
{
/**
* Listen in UDP. Default true.
* @deprecated
*/
enableUdp?: boolean;

/**
* Listen in TCP. Default false.
* @deprecated
* Listen in TCP. Default true if webrtcServer is given, false otherwise.
*/
enableTcp?: boolean;

/**
* Prefer UDP. Default false.
* @deprecated
*/
preferUdp?: boolean;

/**
* Prefer TCP. Default false.
* @deprecated
*/
preferTcp?: boolean;

Expand Down
305 changes: 262 additions & 43 deletions rust/src/fbs.rs

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions rust/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,10 @@ pub(crate) struct RouterCreateWebrtcTransportData {
#[serde(flatten)]
listen: RouterCreateWebrtcTransportListen,
initial_available_outgoing_bitrate: u32,
enable_udp: bool,
enable_tcp: bool,
prefer_udp: bool,
prefer_tcp: bool,
enable_sctp: bool,
num_sctp_streams: NumSctpStreams,
max_sctp_message_size: u32,
Expand Down Expand Up @@ -679,6 +683,10 @@ impl RouterCreateWebrtcTransportData {
},
initial_available_outgoing_bitrate: webrtc_transport_options
.initial_available_outgoing_bitrate,
enable_udp: webrtc_transport_options.enable_udp,
enable_tcp: webrtc_transport_options.enable_tcp,
prefer_udp: webrtc_transport_options.prefer_udp,
prefer_tcp: webrtc_transport_options.prefer_tcp,
enable_sctp: webrtc_transport_options.enable_sctp,
num_sctp_streams: webrtc_transport_options.num_sctp_streams,
max_sctp_message_size: webrtc_transport_options.max_sctp_message_size,
Expand All @@ -700,6 +708,10 @@ impl RouterCreateWebrtcTransportData {
is_data_channel: true,
}),
listen: self.listen.to_fbs(),
enable_udp: self.enable_udp,
ibc marked this conversation as resolved.
Show resolved Hide resolved
enable_tcp: self.enable_tcp,
prefer_udp: self.prefer_udp,
prefer_tcp: self.prefer_tcp,
}
}
}
Expand Down
22 changes: 21 additions & 1 deletion rust/src/router/webrtc_transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,18 @@ pub struct WebRtcTransportOptions {
/// Initial available outgoing bitrate (in bps).
/// Default 600000.
pub initial_available_outgoing_bitrate: u32,
/// Enable UDP.
/// Default true.
pub enable_udp: bool,
/// Enable TCP.
/// Default true if webrtc_server is given, false otherwise.
pub enable_tcp: bool,
/// Prefer UDP.
/// Default false.
pub prefer_udp: bool,
/// Prefer TCP.
/// Default false.
pub prefer_tcp: bool,
/// Create a SCTP association.
/// Default false.
pub enable_sctp: bool,
Expand All @@ -133,12 +145,16 @@ pub struct WebRtcTransportOptions {
}

impl WebRtcTransportOptions {
/// Create [`WebRtcTransport`] options with given listen IPs.
/// Create [`WebRtcTransport`] options with given listen infos.
#[must_use]
pub fn new(listen_infos: WebRtcTransportListenInfos) -> Self {
Self {
listen: WebRtcTransportListen::Individual { listen_infos },
initial_available_outgoing_bitrate: 600_000,
enable_udp: true,
enable_tcp: false,
prefer_udp: false,
prefer_tcp: false,
enable_sctp: false,
num_sctp_streams: NumSctpStreams::default(),
max_sctp_message_size: 262_144,
Expand All @@ -152,6 +168,10 @@ impl WebRtcTransportOptions {
Self {
listen: WebRtcTransportListen::Server { webrtc_server },
initial_available_outgoing_bitrate: 600_000,
enable_udp: true,
ibc marked this conversation as resolved.
Show resolved Hide resolved
enable_tcp: true,
prefer_udp: false,
prefer_tcp: false,
enable_sctp: false,
num_sctp_streams: NumSctpStreams::default(),
max_sctp_message_size: 262_144,
Expand Down
2 changes: 1 addition & 1 deletion rust/tests/integration/webrtc_transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ fn create_succeeds() {
.try_into()
.unwrap(),
);
// TODO: I wonder how this was not needed before...
webrtc_transport_options.enable_sctp = true;
webrtc_transport_options.enable_udp = false;
webrtc_transport_options.num_sctp_streams = NumSctpStreams {
os: 2048,
mis: 2048,
Expand Down
1 change: 0 additions & 1 deletion worker/fbs/transport.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ table RecvRtpHeaderExtensions {

table Options {
direct: bool = false;

/// Only needed for DirectTransport. This value is handled by base Transport.
max_message_size: uint32 = null;
initial_available_outgoing_bitrate: uint32 = null;
Expand Down
4 changes: 4 additions & 0 deletions worker/fbs/webRtcTransport.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ union Listen {
table WebRtcTransportOptions {
base: FBS.Transport.Options (required);
listen: Listen (required);
enable_udp: bool = true;
enable_tcp: bool = true;
prefer_udp: bool = false;
prefer_tcp: bool = false;
}

enum FingerprintAlgorithm: uint8 {
Expand Down
5 changes: 2 additions & 3 deletions worker/include/RTC/WebRtcServer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ namespace RTC
public:
flatbuffers::Offset<FBS::WebRtcServer::DumpResponse> FillBuffer(
flatbuffers::FlatBufferBuilder& builder) const;
const std::vector<RTC::IceCandidate>& GetIceCandidates() const;
const std::vector<RTC::IceCandidate> GetIceCandidates(
bool enableUdp, bool enableTcp, bool preferUdp, bool preferTcp) const;

/* Methods inherited from Channel::ChannelSocket::RequestHandler. */
public:
Expand Down Expand Up @@ -102,8 +103,6 @@ namespace RTC
absl::flat_hash_map<std::string, RTC::WebRtcTransport*> mapLocalIceUsernameFragmentWebRtcTransport;
// Map of WebRtcTransports indexed by TransportTuple.hash.
absl::flat_hash_map<uint64_t, RTC::WebRtcTransport*> mapTupleWebRtcTransport;
// ICE candidates.
std::vector<RTC::IceCandidate> iceCandidates;
};
} // namespace RTC

Expand Down
3 changes: 2 additions & 1 deletion worker/src/RTC/Router.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ namespace RTC
if (!webRtcServer)
MS_THROW_ERROR("wrong webRtcServerId (no associated WebRtcServer found)");

auto& iceCandidates = webRtcServer->GetIceCandidates();
auto iceCandidates = webRtcServer->GetIceCandidates(
options->enableUdp(), options->enableTcp(), options->preferUdp(), options->preferTcp());

// This may throw.
auto* webRtcTransport = new RTC::WebRtcTransport(
Expand Down
91 changes: 60 additions & 31 deletions worker/src/RTC/WebRtcServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ namespace RTC

try
{
uint16_t iceLocalPreferenceDecrement{ 0u };

this->iceCandidates.reserve(listenInfos->size());

for (const auto* listenInfo : *listenInfos)
{
auto ip = listenInfo->ip()->str();
Expand All @@ -64,10 +60,6 @@ namespace RTC
announcedIp = listenInfo->announcedIp()->str();
}

const uint16_t iceLocalPreference =
IceCandidateDefaultLocalPriority - iceLocalPreferenceDecrement;
const uint32_t icePriority = generateIceCandidatePriority(iceLocalPreference);

if (listenInfo->protocol() == FBS::Transport::Protocol::UDP)
{
// This may throw.
Expand All @@ -84,15 +76,6 @@ namespace RTC

this->udpSocketOrTcpServers.emplace_back(udpSocket, nullptr, announcedIp);

if (announcedIp.size() == 0)
{
this->iceCandidates.emplace_back(udpSocket, icePriority);
}
else
{
this->iceCandidates.emplace_back(udpSocket, icePriority, announcedIp);
}

if (listenInfo->sendBufferSize() != 0)
{
udpSocket->SetSendBufferSize(listenInfo->sendBufferSize());
Expand Down Expand Up @@ -125,15 +108,6 @@ namespace RTC

this->udpSocketOrTcpServers.emplace_back(nullptr, tcpServer, announcedIp);

if (announcedIp.size() == 0)
{
this->iceCandidates.emplace_back(tcpServer, icePriority);
}
else
{
this->iceCandidates.emplace_back(tcpServer, icePriority, announcedIp);
}

if (listenInfo->sendBufferSize() != 0)
{
tcpServer->SetSendBufferSize(listenInfo->sendBufferSize());
Expand All @@ -150,9 +124,6 @@ namespace RTC
tcpServer->GetSendBufferSize(),
tcpServer->GetRecvBufferSize());
}

// Decrement initial ICE local preference for next IP.
iceLocalPreferenceDecrement += 100;
}

// NOTE: This may throw.
Expand Down Expand Up @@ -289,11 +260,69 @@ namespace RTC
}
}

const std::vector<RTC::IceCandidate>& WebRtcServer::GetIceCandidates() const
const std::vector<RTC::IceCandidate> WebRtcServer::GetIceCandidates(
bool enableUdp, bool enableTcp, bool preferUdp, bool preferTcp) const
{
MS_TRACE();

return this->iceCandidates;
std::vector<RTC::IceCandidate> iceCandidates;
uint16_t iceLocalPreferenceDecrement{ 0 };

if (!enableUdp)
MS_DUMP_STD("---------------------- no enableUdp");
else
MS_DUMP_STD("---------------------- YES enableUdp");

for (auto& item : this->udpSocketOrTcpServers)
{
if (item.udpSocket && enableUdp)
{
uint16_t iceLocalPreference = IceCandidateDefaultLocalPriority - iceLocalPreferenceDecrement;

if (preferUdp)
{
iceLocalPreference += 1000;
}

uint32_t icePriority = generateIceCandidatePriority(iceLocalPreference);

if (item.announcedIp.empty())
{
iceCandidates.emplace_back(item.udpSocket, icePriority);
}
else
{
iceCandidates.emplace_back(
item.udpSocket, icePriority, const_cast<std::string&>(item.announcedIp));
}
}
else if (item.tcpServer && enableTcp)
{
uint16_t iceLocalPreference = IceCandidateDefaultLocalPriority - iceLocalPreferenceDecrement;

if (preferTcp)
{
iceLocalPreference += 1000;
}

uint32_t icePriority = generateIceCandidatePriority(iceLocalPreference);

if (item.announcedIp.empty())
{
iceCandidates.emplace_back(item.tcpServer, icePriority);
}
else
{
iceCandidates.emplace_back(
item.tcpServer, icePriority, const_cast<std::string&>(item.announcedIp));
}
}

// Decrement initial ICE local preference for next IP.
iceLocalPreferenceDecrement += 100;
}

return iceCandidates;
}

inline std::string WebRtcServer::GetLocalIceUsernameFragmentFromReceivedStunPacket(
Expand Down
3 changes: 3 additions & 0 deletions worker/src/RTC/WebRtcTransport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ namespace RTC

/* Instance methods. */

/**
* This constructor is used when the WebRtcTransport doesn't use a WebRtcServer.
*/
WebRtcTransport::WebRtcTransport(
RTC::Shared* shared,
const std::string& id,
Expand Down