Skip to content

Commit

Permalink
[flatbuffers] Fix missing options in router.createWebRtcTransport()
Browse files Browse the repository at this point in the history
Fixes #1184

### Details

- No idea how, but flatbuffers branch misses `enableUdp`, `enableTcp`, and `preferUdp` and `preferTcp` options when creating a `WebRtcTransport` with a `WebRtcServer`. This was implemented in v3.

### TODO

Rust side, but I prefer to have the Node + C++ sides approved first.

### NOTE

I'm extremely worried that something else may have been lost, specially in `WebRtcServer` cpp and hpp files.
  • Loading branch information
ibc committed Oct 19, 2023
1 parent 7f07c51 commit 5c9291e
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 39 deletions.
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
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
86 changes: 55 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,64 @@ 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 };

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

0 comments on commit 5c9291e

Please sign in to comment.