From cf6e24a12d30c6a64d8925ad03218f6bc1014343 Mon Sep 17 00:00:00 2001 From: Zhi Huang Date: Wed, 21 Feb 2018 10:40:07 -0800 Subject: [PATCH] Forward the SignalNetworkRouteChanged from DtlsSrtpTransport to BaseChannel. In current implementation, the DtlsSrtpTransport listens to the SignalNetworkRouteChanged but doesn't forward it to the BaseChannel which makes it impossible for the media engine to update the network route and the transport overhead. The BaseChannel unit tests failed to catch this issue because it used a plain unencrypted RTP transport for testing. This CL fix that issue and update the BaseChannel tests. Bug: webrtc:7013, b/73645191 Change-Id: I417b58ff9af4e3c4fac442ff10b5a85bc2093530 Reviewed-on: https://webrtc-review.googlesource.com/55940 Commit-Queue: Zhi Huang Reviewed-by: Steve Anton Cr-Commit-Position: refs/heads/master@{#22140} --- pc/channel_unittest.cc | 6 ++++-- pc/dtlssrtptransport.cc | 7 +++++++ pc/dtlssrtptransport.h | 1 + pc/srtptransport.cc | 1 - 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index e7a50dcf9a..92d3b24450 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -1030,8 +1030,10 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { static constexpr int kLastPacketId = 100; // Ipv4(20) + UDP(8). static constexpr int kTransportOverheadPerPacket = 28; + static constexpr int kSrtpOverheadPerPacket = 10; - CreateChannels(0, 0); + CreateChannels(DTLS, DTLS); + SendInitiate(); typename T::MediaChannel* media_channel1 = static_cast(channel1_->media_channel()); @@ -1073,7 +1075,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_EQ(expected_network_route, media_channel1->last_network_route()); EXPECT_EQ(kLastPacketId, media_channel1->last_network_route().last_sent_packet_id); - EXPECT_EQ(kTransportOverheadPerPacket, + EXPECT_EQ(kTransportOverheadPerPacket + kSrtpOverheadPerPacket, media_channel1->transport_overhead_per_packet()); } diff --git a/pc/dtlssrtptransport.cc b/pc/dtlssrtptransport.cc index 03771f4988..f6cbf4d288 100644 --- a/pc/dtlssrtptransport.cc +++ b/pc/dtlssrtptransport.cc @@ -33,6 +33,8 @@ DtlsSrtpTransport::DtlsSrtpTransport( this, &DtlsSrtpTransport::OnPacketReceived); srtp_transport_->SignalReadyToSend.connect(this, &DtlsSrtpTransport::OnReadyToSend); + srtp_transport_->SignalNetworkRouteChanged.connect( + this, &DtlsSrtpTransport::OnNetworkRouteChanged); srtp_transport_->SignalWritableState.connect( this, &DtlsSrtpTransport::OnWritableState); srtp_transport_->SignalSentPacket.connect(this, @@ -356,4 +358,9 @@ void DtlsSrtpTransport::OnReadyToSend(bool ready) { SignalReadyToSend(ready); } +void DtlsSrtpTransport::OnNetworkRouteChanged( + rtc::Optional network_route) { + SignalNetworkRouteChanged(network_route); +} + } // namespace webrtc diff --git a/pc/dtlssrtptransport.h b/pc/dtlssrtptransport.h index 55b96c1645..4f6e697555 100644 --- a/pc/dtlssrtptransport.h +++ b/pc/dtlssrtptransport.h @@ -88,6 +88,7 @@ class DtlsSrtpTransport : public RtpTransportInternalAdapter { rtc::CopyOnWriteBuffer* packet, const rtc::PacketTime& packet_time); void OnReadyToSend(bool ready); + void OnNetworkRouteChanged(rtc::Optional network_route); bool writable_ = false; std::unique_ptr srtp_transport_; diff --git a/pc/srtptransport.cc b/pc/srtptransport.cc index 625034dfab..2c827afa58 100644 --- a/pc/srtptransport.cc +++ b/pc/srtptransport.cc @@ -177,7 +177,6 @@ void SrtpTransport::OnPacketReceived(bool rtcp, } void SrtpTransport::OnNetworkRouteChanged( - rtc::Optional network_route) { // Only append the SRTP overhead when there is a selected network route. if (network_route) {