From 971b9197714b2ed6e4033e5ea62f925296927e17 Mon Sep 17 00:00:00 2001
From: Mohamed Chibani <mohamed.chibani@savoirfairelinux.com>
Date: Wed, 17 Jun 2020 12:32:11 -0400
Subject: [PATCH] ice-turn: Fixes the issue of audio loss after 5mn.
Send outgoing packets to the default remote address (the address
present in the 'c=' or 'a=rtcp' lines in case of RTP connections)
instead of the ICE nominated address.
Gitlab: #223
Change-Id: I4b17ee55ebd92292f994baf7a972514d95ae728b
---
src/ice_socket.h | 1 +
src/ice_transport.cpp | 57 ++++++++++++++++++++++++++-
src/ice_transport.h | 2 +
src/media/audio/audio_rtp_session.cpp | 7 +++-
src/media/media_codec.h | 3 ++
src/media/video/video_rtp_session.cpp | 6 ++-
src/sip/sdp.cpp | 12 ++++++
7 files changed, 84 insertions(+), 4 deletions(-)
diff --git a/src/ice_socket.h b/src/ice_socket.h
index cd46a1a8c6..39aa813ef0 100644
--- a/src/ice_socket.h
+++ b/src/ice_socket.h
@@ -49,6 +49,7 @@ class IceSocket
ssize_t waitForData(std::chrono::milliseconds timeout);
void setOnRecv(IceRecvCb cb);
uint16_t getTransportOverhead();
+ void setDefaultRemoteAddress(const IpAddr& addr);
};
/// ICE transport as a GenericSocket.
diff --git a/src/ice_transport.cpp b/src/ice_transport.cpp
index a8b165e06d..74ea713228 100644
--- a/src/ice_transport.cpp
+++ b/src/ice_transport.cpp
@@ -174,6 +174,9 @@ public:
*/
void selectUPnPIceCandidates();
+ void setDefaultRemoteAddress(int comp_id, const IpAddr& addr);
+ const IpAddr& getDefaultRemoteAddress(int comp_id) const;
+
std::unique_ptr<upnp::Controller> upnp_;
std::mutex upnpMutex_;
@@ -189,6 +192,9 @@ public:
std::condition_variable waitDataCv_ = {};
onShutdownCb scb;
+
+ // Default remote adresses
+ std::vector<IpAddr> iceDefaultRemoteAddr_;
};
//==============================================================================
@@ -284,6 +290,7 @@ IceTransport::Impl::Impl(const char* name, int component_count, bool master,
, compIO_(component_count)
, initiatorSession_(master)
, thread_()
+ , iceDefaultRemoteAddr_(component_count)
{
sip_utils::register_thread();
if (options.upnpEnable)
@@ -396,6 +403,9 @@ IceTransport::Impl::Impl(const char* name, int component_count, bool master,
}
}
});
+
+ // Init to invalid adresses
+ iceDefaultRemoteAddr_.reserve(component_count);
}
IceTransport::Impl::~Impl()
@@ -825,6 +835,25 @@ IceTransport::Impl::selectUPnPIceCandidates()
}
}
+void
+IceTransport::Impl::setDefaultRemoteAddress(int comp_id, const IpAddr& addr)
+{
+ // Component ID must be valid.
+ assert(comp_id < component_count_);
+
+ iceDefaultRemoteAddr_[comp_id] = addr;
+ // The port does not matter. Set it 0 to avoid confusion.
+ iceDefaultRemoteAddr_[comp_id].setPort(0);
+}
+
+const IpAddr&
+IceTransport::Impl::getDefaultRemoteAddress(int comp_id) const
+{
+ // Component ID must be valid.
+ assert(comp_id < component_count_);
+
+ return iceDefaultRemoteAddr_[comp_id];
+}
void
IceTransport::Impl::onReceiveData(unsigned comp_id, void *pkt, pj_size_t size)
@@ -1031,6 +1060,16 @@ IceTransport::getLocalAddress(unsigned comp_id) const
IpAddr
IceTransport::getRemoteAddress(unsigned comp_id) const
{
+
+ // Return the default remote address if set.
+ // Note that the default remote addresses are the addresses
+ // set in the 'c=' and 'a=rtcp' lines of the received SDP.
+ // See pj_ice_strans_sendto2() for more details.
+
+ if (pimpl_->getDefaultRemoteAddress(comp_id)) {
+ return pimpl_->getDefaultRemoteAddress(comp_id);
+ }
+
return pimpl_->getRemoteAddress(comp_id);
}
@@ -1047,7 +1086,6 @@ IceTransport::getLocalCandidates(unsigned comp_id) const
pj_ice_sess_cand cand[PJ_ARRAY_SIZE(pimpl_->cand_)];
unsigned cand_cnt = PJ_ARRAY_SIZE(cand);
-
{
std::lock_guard<std::mutex> lk {pimpl_->iceMutex_};
if (!pimpl_->icest_) return res;
@@ -1305,6 +1343,7 @@ IceTransport::send(int comp_id, const unsigned char* buf, size_t len)
{
sip_utils::register_thread();
auto remote = getRemoteAddress(comp_id);
+
if (!remote) {
JAMI_ERR("[ice:%p] can't find remote address for component %d", this, comp_id);
errno = EINVAL;
@@ -1440,6 +1479,12 @@ IceTransport::parse_SDP(const std::string& sdp_msg, const IceTransport& ice)
return res;
}
+void
+IceTransport::setDefaultRemoteAddress(int comp_id, const IpAddr& addr)
+{
+ pimpl_->setDefaultRemoteAddress(comp_id, addr);
+}
+
//==============================================================================
IceTransportFactory::IceTransportFactory()
@@ -1607,8 +1652,16 @@ IceSocket::setOnRecv(IceRecvCb cb)
}
uint16_t
-IceSocket::getTransportOverhead(){
+IceSocket::getTransportOverhead()
+{
return (ice_transport_->getRemoteAddress(compId_).getFamily() == AF_INET) ? IPV4_HEADER_SIZE : IPV6_HEADER_SIZE;
}
+void
+IceSocket::setDefaultRemoteAddress(const IpAddr& addr)
+{
+ if (ice_transport_)
+ ice_transport_->setDefaultRemoteAddress(compId_, addr);
+}
+
} // namespace jami
diff --git a/src/ice_transport.h b/src/ice_transport.h
index 7ac2f2a40f..3b9ce36453 100644
--- a/src/ice_transport.h
+++ b/src/ice_transport.h
@@ -229,6 +229,8 @@ public:
static ICESDP parse_SDP(const std::string& sdp_msg, const IceTransport& ice);
+ void setDefaultRemoteAddress(int comp_id, const IpAddr& addr);
+
private:
class Impl;
std::unique_ptr<Impl> pimpl_;
diff --git a/src/media/audio/audio_rtp_session.cpp b/src/media/audio/audio_rtp_session.cpp
index c53a717588..f6325c46a7 100644
--- a/src/media/audio/audio_rtp_session.cpp
+++ b/src/media/audio/audio_rtp_session.cpp
@@ -149,8 +149,13 @@ AudioRtpSession::start(std::unique_ptr<IceSocket> rtp_sock, std::unique_ptr<IceS
}
try {
- if (rtp_sock and rtcp_sock)
+ if (rtp_sock and rtcp_sock) {
+
+ rtp_sock->setDefaultRemoteAddress(send_.addr);
+ rtcp_sock->setDefaultRemoteAddress(send_.rtcp_addr);
+
socketPair_.reset(new SocketPair(std::move(rtp_sock), std::move(rtcp_sock)));
+ }
else
socketPair_.reset(new SocketPair(getRemoteRtpUri().c_str(), receive_.addr.getPort()));
diff --git a/src/media/media_codec.h b/src/media/media_codec.h
index 6068ef3377..38034abf0f 100644
--- a/src/media/media_codec.h
+++ b/src/media/media_codec.h
@@ -272,6 +272,9 @@ struct MediaDescription {
/** Endpoint socket address */
IpAddr addr {};
+ /** RTCP socket address */
+ IpAddr rtcp_addr {};
+
/** RTP */
std::shared_ptr<AccountCodecInfo> codec {};
unsigned payload_type {};
diff --git a/src/media/video/video_rtp_session.cpp b/src/media/video/video_rtp_session.cpp
index f242d9f093..2d50cb0791 100644
--- a/src/media/video/video_rtp_session.cpp
+++ b/src/media/video/video_rtp_session.cpp
@@ -208,8 +208,12 @@ void VideoRtpSession::start(std::unique_ptr<IceSocket> rtp_sock,
}
try {
- if (rtp_sock and rtcp_sock)
+ if (rtp_sock and rtcp_sock) {
+ rtp_sock->setDefaultRemoteAddress(send_.addr);
+ rtcp_sock->setDefaultRemoteAddress(send_.rtcp_addr);
+
socketPair_.reset(new SocketPair(std::move(rtp_sock), std::move(rtcp_sock)));
+ }
else
socketPair_.reset(new SocketPair(getRemoteRtpUri().c_str(), receive_.addr.getPort()));
diff --git a/src/sip/sdp.cpp b/src/sip/sdp.cpp
index acfa2bac82..b57a5efbe3 100644
--- a/src/sip/sdp.cpp
+++ b/src/sip/sdp.cpp
@@ -597,6 +597,18 @@ Sdp::getMediaSlots(const pjmedia_sdp_session* session, bool remote) const
descr.addr = std::string(conn->addr.ptr, conn->addr.slen);
descr.addr.setPort(media->desc.port);
+ // Get the "rtcp" address from the SDP if present. Otherwise,
+ // infere it from endpoint (RTP) address.
+ auto attr = pjmedia_sdp_attr_find2(media->attr_count, media->attr, "rtcp", NULL);
+ if (attr) {
+ pjmedia_sdp_rtcp_attr rtcp;
+ auto status = pjmedia_sdp_attr_get_rtcp(attr, &rtcp);
+ if (status == PJ_SUCCESS && rtcp.addr.slen) {
+ descr.rtcp_addr = std::string(rtcp.addr.ptr, rtcp.addr.slen);
+ descr.rtcp_addr.setPort(rtcp.port);
+ }
+ }
+
descr.holding = pjmedia_sdp_attr_find2(media->attr_count, media->attr, "sendonly", nullptr)
|| pjmedia_sdp_attr_find2(media->attr_count, media->attr, "inactive", nullptr);
--
GitLab