From 737336d37afb29e20ea406b875c74fbd9f95f1b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Fri, 29 Jul 2016 12:59:36 +0200 Subject: [PATCH] Add NACK rate throttling for audio channels. Not really used for audio today (already in place for video), but should still function anyway. BUG= R=henrika@webrtc.org, minyue@webrtc.org, stefan@webrtc.org Review URL: https://codereview.webrtc.org/2181383002 . Cr-Commit-Position: refs/heads/master@{#13571} --- webrtc/call/rtc_event_log_unittest.cc | 26 +++--- .../rtp_rtcp/source/nack_rtx_unittest.cc | 6 +- .../source/rtcp_format_remb_unittest.cc | 6 +- .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 32 ++++---- .../rtp_rtcp/source/rtcp_sender_unittest.cc | 6 +- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 2 + .../rtp_rtcp/source/rtp_sender_unittest.cc | 24 +++--- .../modules/rtp_rtcp/test/testAPI/test_api.cc | 6 +- .../rtp_rtcp/test/testAPI/test_api_audio.cc | 6 +- .../rtp_rtcp/test/testAPI/test_api_rtcp.cc | 6 +- .../rtp_rtcp/test/testAPI/test_api_video.cc | 6 +- webrtc/video/end_to_end_tests.cc | 79 ++++++++++++++++++- webrtc/video/rtp_stream_receiver.cc | 11 ++- webrtc/video/rtp_stream_receiver.h | 3 +- webrtc/video/video_receive_stream.cc | 25 +++--- webrtc/video/video_send_stream_tests.cc | 26 ++++-- webrtc/voice_engine/channel.cc | 18 +++++ webrtc/voice_engine/channel.h | 3 +- 18 files changed, 218 insertions(+), 73 deletions(-) diff --git a/webrtc/call/rtc_event_log_unittest.cc b/webrtc/call/rtc_event_log_unittest.cc index f8b555b63d..57d18b7e72 100644 --- a/webrtc/call/rtc_event_log_unittest.cc +++ b/webrtc/call/rtc_event_log_unittest.cc @@ -20,6 +20,7 @@ #include "webrtc/base/buffer.h" #include "webrtc/base/checks.h" #include "webrtc/base/random.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/call.h" #include "webrtc/call/rtc_event_log.h" #include "webrtc/call/rtc_event_log_parser.h" @@ -111,19 +112,20 @@ size_t GenerateRtpPacket(uint32_t extensions_bitvector, Random* prng) { RTC_CHECK_GE(packet_size, 16 + 4 * csrcs_count + 4 * kNumExtensions); Clock* clock = Clock::GetRealTimeClock(); + RateLimiter retranmission_rate_limiter(clock, 1000); - RTPSender rtp_sender(false, // bool audio - clock, // Clock* clock - nullptr, // Transport* - nullptr, // PacedSender* - nullptr, // PacketRouter* - nullptr, // SendTimeObserver* - nullptr, // BitrateStatisticsObserver* - nullptr, // FrameCountObserver* - nullptr, // SendSideDelayObserver* - nullptr, // RtcEventLog* - nullptr, // SendPacketObserver* - nullptr); // NackRateLimiter* + RTPSender rtp_sender(false, // bool audio + clock, // Clock* clock + nullptr, // Transport* + nullptr, // PacedSender* + nullptr, // PacketRouter* + nullptr, // SendTimeObserver* + nullptr, // BitrateStatisticsObserver* + nullptr, // FrameCountObserver* + nullptr, // SendSideDelayObserver* + nullptr, // RtcEventLog* + nullptr, // SendPacketObserver* + &retranmission_rate_limiter); std::vector csrcs; for (unsigned i = 0; i < csrcs_count; i++) { diff --git a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc index b12c08e642..a0e0cecff6 100644 --- a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc @@ -175,7 +175,7 @@ class RtpRtcpRtxNackTest : public ::testing::Test { receiver_(), payload_data_length(sizeof(payload_data)), fake_clock(123456), - retranmission_rate_limiter_(&fake_clock, kMaxRttMs) {} + retransmission_rate_limiter_(&fake_clock, kMaxRttMs) {} ~RtpRtcpRtxNackTest() {} void SetUp() override { @@ -185,7 +185,7 @@ class RtpRtcpRtxNackTest : public ::testing::Test { receive_statistics_.reset(ReceiveStatistics::Create(&fake_clock)); configuration.receive_statistics = receive_statistics_.get(); configuration.outgoing_transport = &transport_; - configuration.retransmission_rate_limiter = &retranmission_rate_limiter_; + configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; rtp_rtcp_module_ = RtpRtcp::CreateRtpRtcp(configuration); rtp_feedback_.reset(new TestRtpFeedback(rtp_rtcp_module_)); @@ -292,7 +292,7 @@ class RtpRtcpRtxNackTest : public ::testing::Test { uint8_t payload_data[65000]; size_t payload_data_length; SimulatedClock fake_clock; - RateLimiter retranmission_rate_limiter_; + RateLimiter retransmission_rate_limiter_; }; TEST_F(RtpRtcpRtxNackTest, LongNackList) { diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc index bbfb52c6cc..0e0d7f5291 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc @@ -12,6 +12,7 @@ #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/common_types.h" #include "webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h" #include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_observer.h" @@ -69,7 +70,8 @@ class RtcpFormatRembTest : public ::testing::Test { remote_bitrate_observer_(), remote_bitrate_estimator_( new RemoteBitrateEstimatorSingleStream(&remote_bitrate_observer_, - system_clock_)) {} + system_clock_)), + retransmission_rate_limiter_(Clock::GetRealTimeClock(), 1000) {} void SetUp() override; void TearDown() override; @@ -83,6 +85,7 @@ class RtcpFormatRembTest : public ::testing::Test { test::NullTransport null_transport_; MockRemoteBitrateObserver remote_bitrate_observer_; std::unique_ptr remote_bitrate_estimator_; + RateLimiter retransmission_rate_limiter_; }; void RtcpFormatRembTest::SetUp() { @@ -91,6 +94,7 @@ void RtcpFormatRembTest::SetUp() { configuration.clock = system_clock_; configuration.remote_bitrate_estimator = remote_bitrate_estimator_.get(); configuration.outgoing_transport = &null_transport_; + configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; dummy_rtp_rtcp_impl_ = new ModuleRtpRtcpImpl(configuration); rtcp_receiver_ = new RTCPReceiver(system_clock_, false, nullptr, nullptr, nullptr, nullptr, dummy_rtp_rtcp_impl_); diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 924d009883..cdb71be003 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -13,6 +13,7 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/common_types.h" #include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_observer.h" #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h" @@ -80,25 +81,25 @@ class RtcpReceiverTest : public ::testing::Test { remote_bitrate_observer_(), remote_bitrate_estimator_( new RemoteBitrateEstimatorSingleStream(&remote_bitrate_observer_, - &system_clock_)) { - test_transport_ = new TestTransport(); + &system_clock_)), + retransmission_rate_limiter_(&system_clock_, 1000) { + test_transport_.reset(new TestTransport()); RtpRtcp::Configuration configuration; configuration.audio = false; configuration.clock = &system_clock_; - configuration.outgoing_transport = test_transport_; + configuration.outgoing_transport = test_transport_.get(); configuration.remote_bitrate_estimator = remote_bitrate_estimator_.get(); - rtp_rtcp_impl_ = new ModuleRtpRtcpImpl(configuration); - rtcp_receiver_ = new RTCPReceiver(&system_clock_, false, nullptr, nullptr, - nullptr, nullptr, rtp_rtcp_impl_); - test_transport_->SetRTCPReceiver(rtcp_receiver_); - } - ~RtcpReceiverTest() { - delete rtcp_receiver_; - delete rtp_rtcp_impl_; - delete test_transport_; + configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; + rtp_rtcp_impl_.reset(new ModuleRtpRtcpImpl(configuration)); + rtcp_receiver_.reset(new RTCPReceiver(&system_clock_, false, nullptr, + nullptr, nullptr, nullptr, + rtp_rtcp_impl_.get())); + test_transport_->SetRTCPReceiver(rtcp_receiver_.get()); } + ~RtcpReceiverTest() {} + // Injects an RTCP packet into the receiver. // Returns 0 for OK, non-0 for failure. int InjectRtcpPacket(const uint8_t* packet, @@ -142,12 +143,13 @@ class RtcpReceiverTest : public ::testing::Test { OverUseDetectorOptions over_use_detector_options_; SimulatedClock system_clock_; - ModuleRtpRtcpImpl* rtp_rtcp_impl_; - RTCPReceiver* rtcp_receiver_; - TestTransport* test_transport_; + std::unique_ptr test_transport_; + std::unique_ptr rtp_rtcp_impl_; + std::unique_ptr rtcp_receiver_; RTCPHelp::RTCPPacketInformation rtcp_packet_info_; MockRemoteBitrateObserver remote_bitrate_observer_; std::unique_ptr remote_bitrate_estimator_; + RateLimiter retransmission_rate_limiter_; }; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index ec8330820c..270088ebac 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -13,6 +13,7 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_sender.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" @@ -229,11 +230,13 @@ class RtcpSenderTest : public ::testing::Test { protected: RtcpSenderTest() : clock_(1335900000), - receive_statistics_(ReceiveStatistics::Create(&clock_)) { + receive_statistics_(ReceiveStatistics::Create(&clock_)), + retransmission_rate_limiter_(&clock_, 1000) { RtpRtcp::Configuration configuration; configuration.audio = false; configuration.clock = &clock_; configuration.outgoing_transport = &test_transport_; + configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; rtp_rtcp_impl_.reset(new ModuleRtpRtcpImpl(configuration)); rtcp_sender_.reset(new RTCPSender(false, &clock_, receive_statistics_.get(), @@ -265,6 +268,7 @@ class RtcpSenderTest : public ::testing::Test { std::unique_ptr receive_statistics_; std::unique_ptr rtp_rtcp_impl_; std::unique_ptr rtcp_sender_; + RateLimiter retransmission_rate_limiter_; }; TEST_F(RtcpSenderTest, SetRtcpStatus) { diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index 0afc8d2438..3f2371e0d5 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -127,6 +127,8 @@ RTPSender::RTPSender( csrcs_(), rtx_(kRtxOff), retransmission_rate_limiter_(retransmission_rate_limiter) { + RTC_DCHECK(retransmission_rate_limiter_ != nullptr); + // We need to seed the random generator for BuildPaddingPacket() below. // TODO(holmer,tommi): Note that TimeInMilliseconds might return 0 on Mac // early on in the process. diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 302f6ddb4d..fed767b687 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -1013,7 +1013,7 @@ TEST_F(RtpSenderTest, OnSendPacketNotUpdatedWithoutSeqNumAllocator) { rtp_sender_.reset(new RTPSender( false, &fake_clock_, &transport_, &mock_paced_sender_, nullptr /* TransportSequenceNumberAllocator */, nullptr, nullptr, nullptr, - nullptr, nullptr, &send_packet_observer_, nullptr)); + nullptr, nullptr, &send_packet_observer_, &retransmission_rate_limiter_)); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionTransportSequenceNumber, kTransportSequenceNumberExtensionId)); @@ -1034,7 +1034,8 @@ TEST_F(RtpSenderTest, SendRedundantPayloads) { MockTransport transport; rtp_sender_.reset(new RTPSender( false, &fake_clock_, &transport, &mock_paced_sender_, nullptr, nullptr, - nullptr, nullptr, nullptr, &mock_rtc_event_log_, nullptr, nullptr)); + nullptr, nullptr, nullptr, &mock_rtc_event_log_, nullptr, + &retransmission_rate_limiter_)); rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload); @@ -1178,9 +1179,10 @@ TEST_F(RtpSenderTest, FrameCountCallbacks) { FrameCounts frame_counts_; } callback; - rtp_sender_.reset(new RTPSender( - false, &fake_clock_, &transport_, &mock_paced_sender_, nullptr, nullptr, - nullptr, &callback, nullptr, nullptr, nullptr, nullptr)); + rtp_sender_.reset(new RTPSender(false, &fake_clock_, &transport_, + &mock_paced_sender_, nullptr, nullptr, + nullptr, &callback, nullptr, nullptr, nullptr, + &retransmission_rate_limiter_)); char payload_name[RTP_PAYLOAD_NAME_SIZE] = "GENERIC"; const uint8_t payload_type = 127; @@ -1239,9 +1241,9 @@ TEST_F(RtpSenderTest, BitrateCallbacks) { uint32_t total_bitrate_; uint32_t retransmit_bitrate_; } callback; - rtp_sender_.reset(new RTPSender(false, &fake_clock_, &transport_, nullptr, - nullptr, nullptr, &callback, nullptr, nullptr, - nullptr, nullptr, nullptr)); + rtp_sender_.reset(new RTPSender( + false, &fake_clock_, &transport_, nullptr, nullptr, nullptr, &callback, + nullptr, nullptr, nullptr, nullptr, &retransmission_rate_limiter_)); // Simulate kNumPackets sent with kPacketInterval ms intervals, with the // number of packets selected so that we fill (but don't overflow) the one @@ -1296,9 +1298,9 @@ class RtpSenderAudioTest : public RtpSenderTest { void SetUp() override { payload_ = kAudioPayload; - rtp_sender_.reset(new RTPSender(true, &fake_clock_, &transport_, nullptr, - nullptr, nullptr, nullptr, nullptr, nullptr, - nullptr, nullptr, nullptr)); + rtp_sender_.reset(new RTPSender( + true, &fake_clock_, &transport_, nullptr, nullptr, nullptr, nullptr, + nullptr, nullptr, nullptr, nullptr, &retransmission_rate_limiter_)); rtp_sender_->SetSequenceNumber(kSeqNum); } }; diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc index d4c11115db..ab666cec2c 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc @@ -14,6 +14,7 @@ #include #include +#include "webrtc/base/rate_limiter.h" #include "webrtc/test/null_transport.h" namespace webrtc { @@ -80,7 +81,8 @@ int32_t TestRtpReceiver::OnReceivedPayloadData( class RtpRtcpAPITest : public ::testing::Test { protected: - RtpRtcpAPITest() : fake_clock_(123456) { + RtpRtcpAPITest() + : fake_clock_(123456), retransmission_rate_limiter_(&fake_clock_, 1000) { test_csrcs_.push_back(1234); test_csrcs_.push_back(2345); test_ssrc_ = 3456; @@ -94,6 +96,7 @@ class RtpRtcpAPITest : public ::testing::Test { configuration.audio = true; configuration.clock = &fake_clock_; configuration.outgoing_transport = &null_transport_; + configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; module_.reset(RtpRtcp::CreateRtpRtcp(configuration)); rtp_payload_registry_.reset(new RTPPayloadRegistry( RTPPayloadStrategy::CreateStrategy(true))); @@ -110,6 +113,7 @@ class RtpRtcpAPITest : public ::testing::Test { std::vector test_csrcs_; SimulatedClock fake_clock_; test::NullTransport null_transport_; + RateLimiter retransmission_rate_limiter_; }; TEST_F(RtpRtcpAPITest, Basic) { diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc index 0fb98456d9..f9e5001587 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc @@ -15,6 +15,7 @@ #include "webrtc/modules/rtp_rtcp/test/testAPI/test_api.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" @@ -77,7 +78,8 @@ class RTPCallback : public NullRtpFeedback { class RtpRtcpAudioTest : public ::testing::Test { protected: - RtpRtcpAudioTest() : fake_clock(123456) { + RtpRtcpAudioTest() + : fake_clock(123456), retransmission_rate_limiter_(&fake_clock, 1000) { test_CSRC[0] = 1234; test_CSRC[2] = 2345; test_ssrc = 3456; @@ -106,6 +108,7 @@ class RtpRtcpAudioTest : public ::testing::Test { configuration.clock = &fake_clock; configuration.receive_statistics = receive_statistics1_.get(); configuration.outgoing_transport = transport1; + configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; module1 = RtpRtcp::CreateRtpRtcp(configuration); rtp_receiver1_.reset(RtpReceiver::CreateAudioReceiver( @@ -152,6 +155,7 @@ class RtpRtcpAudioTest : public ::testing::Test { uint16_t test_sequence_number; uint32_t test_CSRC[webrtc::kRtpCsrcSize]; SimulatedClock fake_clock; + RateLimiter retransmission_rate_limiter_; }; TEST_F(RtpRtcpAudioTest, Basic) { diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc index c1359df864..f33507e358 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_rtcp.cc @@ -14,6 +14,7 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/include/receive_statistics.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" @@ -66,7 +67,8 @@ class TestRtpFeedback : public NullRtpFeedback { class RtpRtcpRtcpTest : public ::testing::Test { protected: - RtpRtcpRtcpTest() : fake_clock(123456) { + RtpRtcpRtcpTest() + : fake_clock(123456), retransmission_rate_limiter_(&fake_clock, 1000) { test_csrcs.push_back(1234); test_csrcs.push_back(2345); test_ssrc = 3456; @@ -91,6 +93,7 @@ class RtpRtcpRtcpTest : public ::testing::Test { configuration.receive_statistics = receive_statistics1_.get(); configuration.outgoing_transport = transport1; configuration.intra_frame_callback = myRTCPFeedback1; + configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; rtp_payload_registry1_.reset(new RTPPayloadRegistry( RTPPayloadStrategy::CreateStrategy(true))); @@ -197,6 +200,7 @@ class RtpRtcpRtcpTest : public ::testing::Test { uint16_t test_sequence_number; std::vector test_csrcs; SimulatedClock fake_clock; + RateLimiter retransmission_rate_limiter_; }; TEST_F(RtpRtcpRtcpTest, RTCP_PLI_RPSI) { diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc index d84ff37be7..e784386676 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_video.cc @@ -15,6 +15,7 @@ #include #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" @@ -38,7 +39,8 @@ class RtpRtcpVideoTest : public ::testing::Test { test_ssrc_(3456), test_timestamp_(4567), test_sequence_number_(2345), - fake_clock(123456) {} + fake_clock(123456), + retransmission_rate_limiter_(&fake_clock, 1000) {} ~RtpRtcpVideoTest() {} virtual void SetUp() { @@ -49,6 +51,7 @@ class RtpRtcpVideoTest : public ::testing::Test { configuration.audio = false; configuration.clock = &fake_clock; configuration.outgoing_transport = transport_; + configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; video_module_ = RtpRtcp::CreateRtpRtcp(configuration); rtp_receiver_.reset(RtpReceiver::CreateVideoReceiver( @@ -139,6 +142,7 @@ class RtpRtcpVideoTest : public ::testing::Test { uint8_t video_frame_[65000]; size_t payload_data_length_; SimulatedClock fake_clock; + RateLimiter retransmission_rate_limiter_; }; TEST_F(RtpRtcpVideoTest, BasicVideo) { diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index 034d063814..b2a29d2c61 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -19,12 +19,15 @@ #include "webrtc/base/checks.h" #include "webrtc/base/event.h" +#include "webrtc/base/optional.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/call.h" #include "webrtc/call/transport_adapter.h" #include "webrtc/common_video/include/frame_callback.h" #include "webrtc/modules/include/module_common_types.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" +#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/nack.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" #include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" @@ -487,6 +490,77 @@ TEST_F(EndToEndTest, ReceivesAndRetransmitsNack) { RunBaseTest(&test); } +TEST_F(EndToEndTest, ReceivesNackAndRetransmitsAudio) { + class NackObserver : public test::EndToEndTest { + public: + NackObserver() + : EndToEndTest(kLongTimeoutMs), + local_ssrc_(0), + remote_ssrc_(0), + receive_transport_(nullptr) {} + + private: + size_t GetNumVideoStreams() const override { return 0; } + size_t GetNumAudioStreams() const override { return 1; } + + test::PacketTransport* CreateReceiveTransport() override { + test::PacketTransport* receive_transport = new test::PacketTransport( + nullptr, this, test::PacketTransport::kReceiver, + FakeNetworkPipe::Config()); + receive_transport_ = receive_transport; + return receive_transport; + } + + Action OnSendRtp(const uint8_t* packet, size_t length) override { + RTPHeader header; + EXPECT_TRUE(parser_->Parse(packet, length, &header)); + + if (!sequence_number_to_retransmit_) { + sequence_number_to_retransmit_ = + rtc::Optional(header.sequenceNumber); + + // Don't ask for retransmission straight away, may be deduped in pacer. + } else if (header.sequenceNumber == *sequence_number_to_retransmit_) { + observation_complete_.Set(); + } else { + // Send a NACK as often as necessary until retransmission is received. + rtcp::Nack nack; + nack.From(local_ssrc_); + nack.To(remote_ssrc_); + uint16_t nack_list[] = {*sequence_number_to_retransmit_}; + nack.WithList(nack_list, 1); + rtc::Buffer buffer = nack.Build(); + + EXPECT_TRUE(receive_transport_->SendRtcp(buffer.data(), buffer.size())); + } + + return SEND_PACKET; + } + + void ModifyAudioConfigs( + AudioSendStream::Config* send_config, + std::vector* receive_configs) override { + send_config->rtp.nack.rtp_history_ms = kNackRtpHistoryMs; + (*receive_configs)[0].rtp.nack.rtp_history_ms = kNackRtpHistoryMs; + local_ssrc_ = (*receive_configs)[0].rtp.local_ssrc; + remote_ssrc_ = (*receive_configs)[0].rtp.remote_ssrc; + } + + void PerformTest() override { + EXPECT_TRUE(Wait()) + << "Timed out waiting for packets to be NACKed, retransmitted and " + "rendered."; + } + + uint32_t local_ssrc_; + uint32_t remote_ssrc_; + Transport* receive_transport_; + rtc::Optional sequence_number_to_retransmit_; + } test; + + RunBaseTest(&test); +} + TEST_F(EndToEndTest, CanReceiveFec) { class FecRenderObserver : public test::EndToEndTest, public rtc::VideoSinkInterface { @@ -1807,7 +1881,8 @@ TEST_F(EndToEndTest, RembWithSendSideBwe) { poller_thread_(&BitrateStatsPollingThread, this, "BitrateStatsPollingThread"), - state_(kWaitForFirstRampUp) {} + state_(kWaitForFirstRampUp), + retransmission_rate_limiter_(clock_, 1000) {} ~BweObserver() {} @@ -1847,6 +1922,7 @@ TEST_F(EndToEndTest, RembWithSendSideBwe) { config.receiver_only = true; config.clock = clock_; config.outgoing_transport = receive_transport_; + config.retransmission_rate_limiter = &retransmission_rate_limiter_; rtp_rtcp_.reset(RtpRtcp::CreateRtpRtcp(config)); rtp_rtcp_->SetRemoteSSRC((*receive_configs)[0].rtp.remote_ssrc); rtp_rtcp_->SetSSRC((*receive_configs)[0].rtp.local_ssrc); @@ -1919,6 +1995,7 @@ TEST_F(EndToEndTest, RembWithSendSideBwe) { rtc::Event event_; rtc::PlatformThread poller_thread_; TestState state_; + RateLimiter retransmission_rate_limiter_; } test; RunBaseTest(&test); diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc index 4caf55aa8b..e7367d8a1a 100644 --- a/webrtc/video/rtp_stream_receiver.cc +++ b/webrtc/video/rtp_stream_receiver.cc @@ -39,7 +39,8 @@ std::unique_ptr CreateRtpRtcpModule( RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer, RemoteBitrateEstimator* remote_bitrate_estimator, RtpPacketSender* paced_sender, - TransportSequenceNumberAllocator* transport_sequence_number_allocator) { + TransportSequenceNumberAllocator* transport_sequence_number_allocator, + RateLimiter* retransmission_rate_limiter) { RtpRtcp::Configuration configuration; configuration.audio = false; configuration.receiver_only = true; @@ -58,7 +59,7 @@ std::unique_ptr CreateRtpRtcpModule( configuration.send_packet_observer = nullptr; configuration.bandwidth_callback = nullptr; configuration.transport_feedback_callback = nullptr; - configuration.retransmission_rate_limiter = nullptr; + configuration.retransmission_rate_limiter = retransmission_rate_limiter; std::unique_ptr rtp_rtcp(RtpRtcp::CreateRtpRtcp(configuration)); rtp_rtcp->SetSendingStatus(false); @@ -80,7 +81,8 @@ RtpStreamReceiver::RtpStreamReceiver( VieRemb* remb, const VideoReceiveStream::Config* config, ReceiveStatisticsProxy* receive_stats_proxy, - ProcessThread* process_thread) + ProcessThread* process_thread, + RateLimiter* retransmission_rate_limiter) : clock_(Clock::GetRealTimeClock()), config_(*config), video_receiver_(video_receiver), @@ -106,7 +108,8 @@ RtpStreamReceiver::RtpStreamReceiver( receive_stats_proxy, remote_bitrate_estimator_, paced_sender, - packet_router)) { + packet_router, + retransmission_rate_limiter)) { packet_router_->AddRtpModule(rtp_rtcp_.get()); rtp_receive_statistics_->RegisterRtpStatisticsCallback(receive_stats_proxy); rtp_receive_statistics_->RegisterRtcpStatisticsCallback(receive_stats_proxy); diff --git a/webrtc/video/rtp_stream_receiver.h b/webrtc/video/rtp_stream_receiver.h index efe71816c2..2af15b0ffc 100644 --- a/webrtc/video/rtp_stream_receiver.h +++ b/webrtc/video/rtp_stream_receiver.h @@ -62,7 +62,8 @@ class RtpStreamReceiver : public RtpData, public RtpFeedback, VieRemb* remb, const VideoReceiveStream::Config* config, ReceiveStatisticsProxy* receive_stats_proxy, - ProcessThread* process_thread); + ProcessThread* process_thread, + RateLimiter* retransmission_rate_limiter); ~RtpStreamReceiver(); bool SetReceiveCodec(const VideoCodec& video_codec); diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 609a789fa2..bc51269b85 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -14,6 +14,7 @@ #include #include +#include #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" @@ -162,17 +163,19 @@ VideoReceiveStream::VideoReceiveStream( call_stats_(call_stats), video_receiver_(clock_, nullptr, this, this, this), stats_proxy_(&config_, clock_), - rtp_stream_receiver_(&video_receiver_, - congestion_controller_->GetRemoteBitrateEstimator( - UseSendSideBwe(config_)), - &transport_adapter_, - call_stats_->rtcp_rtt_stats(), - congestion_controller_->pacer(), - congestion_controller_->packet_router(), - remb, - &config_, - &stats_proxy_, - process_thread_), + rtp_stream_receiver_( + &video_receiver_, + congestion_controller_->GetRemoteBitrateEstimator( + UseSendSideBwe(config_)), + &transport_adapter_, + call_stats_->rtcp_rtt_stats(), + congestion_controller_->pacer(), + congestion_controller_->packet_router(), + remb, + &config_, + &stats_proxy_, + process_thread_, + congestion_controller_->GetRetransmissionRateLimiter()), vie_sync_(&video_receiver_) { LOG(LS_INFO) << "VideoReceiveStream: " << config_.ToString(); diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 3a8acbc5b5..623cd34ac5 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -19,6 +19,7 @@ #include "webrtc/base/event.h" #include "webrtc/base/logging.h" #include "webrtc/base/platform_thread.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/call.h" #include "webrtc/call/transport_adapter.h" #include "webrtc/common_video/include/frame_callback.h" @@ -819,13 +820,13 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) { RembObserver() : SendTest(kDefaultTimeoutMs), clock_(Clock::GetRealTimeClock()), + stream_(nullptr), test_state_(kBeforeSuspend), rtp_count_(0), last_sequence_number_(0), suspended_frame_count_(0), low_remb_bps_(0), - high_remb_bps_(0) { - } + high_remb_bps_(0) {} private: Action OnSendRtp(const uint8_t* packet, size_t length) override { @@ -1049,8 +1050,9 @@ TEST_F(VideoSendStreamTest, MinTransmitBitrateRespectsRemb) { public: BitrateObserver() : SendTest(kDefaultTimeoutMs), - bitrate_capped_(false) { - } + retranmission_rate_limiter_(Clock::GetRealTimeClock(), 1000), + stream_(nullptr), + bitrate_capped_(false) {} private: Action OnSendRtp(const uint8_t* packet, size_t length) override { @@ -1092,6 +1094,7 @@ TEST_F(VideoSendStreamTest, MinTransmitBitrateRespectsRemb) { stream_ = send_stream; RtpRtcp::Configuration config; config.outgoing_transport = feedback_transport_.get(); + config.retransmission_rate_limiter = &retranmission_rate_limiter_; rtp_rtcp_.reset(RtpRtcp::CreateRtpRtcp(config)); rtp_rtcp_->SetREMBStatus(true); rtp_rtcp_->SetRTCPStatus(RtcpMode::kReducedSize); @@ -1114,6 +1117,7 @@ TEST_F(VideoSendStreamTest, MinTransmitBitrateRespectsRemb) { std::unique_ptr rtp_rtcp_; std::unique_ptr feedback_transport_; + RateLimiter retranmission_rate_limiter_; VideoSendStream* stream_; bool bitrate_capped_; } test; @@ -1504,6 +1508,7 @@ TEST_F(VideoSendStreamTest, EncoderIsProperlyInitializedAndDestroyed) { public: EncoderStateObserver() : SendTest(kDefaultTimeoutMs), + stream_(nullptr), initialized_(false), callback_registered_(false), num_releases_(0), @@ -1624,7 +1629,8 @@ TEST_F(VideoSendStreamTest, EncoderSetupPropagatesCommonEncoderConfigValues) { : SendTest(kDefaultTimeoutMs), FakeEncoder(Clock::GetRealTimeClock()), init_encode_event_(false, false), - num_initializations_(0) {} + num_initializations_(0), + stream_(nullptr) {} private: void ModifyVideoConfigs( @@ -1689,7 +1695,8 @@ class VideoCodecConfigObserver : public test::SendTest, video_codec_type_(video_codec_type), codec_name_(codec_name), init_encode_event_(false, false), - num_initializations_(0) { + num_initializations_(0), + stream_(nullptr) { memset(&encoder_settings_, 0, sizeof(encoder_settings_)); } @@ -1931,7 +1938,9 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { : SendTest(kDefaultTimeoutMs), FakeEncoder(Clock::GetRealTimeClock()), init_encode_event_(false, false), - num_initializations_(0) {} + num_initializations_(0), + call_(nullptr), + send_stream_(nullptr) {} private: int32_t InitEncode(const VideoCodec* codecSettings, @@ -2045,7 +2054,8 @@ TEST_F(VideoSendStreamTest, ReportsSentResolution) { public: ScreencastTargetBitrateTest() : SendTest(kDefaultTimeoutMs), - test::FakeEncoder(Clock::GetRealTimeClock()) {} + test::FakeEncoder(Clock::GetRealTimeClock()), + send_stream_(nullptr) {} private: int32_t Encode(const VideoFrame& input_image, diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index dcedd757aa..f5be7ef978 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -17,6 +17,7 @@ #include "webrtc/base/criticalsection.h" #include "webrtc/base/format_macros.h" #include "webrtc/base/logging.h" +#include "webrtc/base/rate_limiter.h" #include "webrtc/base/thread_checker.h" #include "webrtc/base/timeutils.h" #include "webrtc/call/rtc_event_log.h" @@ -47,6 +48,9 @@ namespace voe { namespace { +constexpr int64_t kMaxRetransmissionWindowMs = 1000; +constexpr int64_t kMinRetransmissionWindowMs = 30; + bool RegisterReceiveCodec(std::unique_ptr* acm, acm2::RentACodec* rac, const CodecInst& ci) { @@ -902,6 +906,8 @@ Channel::Channel(int32_t channelId, feedback_observer_proxy_(new TransportFeedbackProxy()), seq_num_allocator_proxy_(new TransportSequenceNumberProxy()), rtp_packet_sender_proxy_(new RtpPacketSenderProxy()), + retransmission_rate_limiter_(new RateLimiter(Clock::GetRealTimeClock(), + kMaxRetransmissionWindowMs)), decoder_factory_(decoder_factory) { WEBRTC_TRACE(kTraceMemory, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::Channel() - ctor"); @@ -933,6 +939,8 @@ Channel::Channel(int32_t channelId, configuration.transport_feedback_callback = feedback_observer_proxy_.get(); } configuration.event_log = &(*event_log_proxy_); + configuration.retransmission_rate_limiter = + retransmission_rate_limiter_.get(); _rtpRtcpModule.reset(RtpRtcp::CreateRtpRtcp(configuration)); _rtpRtcpModule->SetSendingMediaStatus(false); @@ -1352,6 +1360,7 @@ void Channel::SetBitRate(int bitrate_bps) { WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::SetBitRate(bitrate_bps=%d)", bitrate_bps); audio_coding_->SetBitRate(bitrate_bps); + retransmission_rate_limiter_->SetMaxRate(bitrate_bps); } void Channel::OnIncomingFractionLoss(int fraction_lost) { @@ -1710,6 +1719,15 @@ int32_t Channel::ReceivedRTCPPacket(const uint8_t* data, size_t length) { // Waiting for valid RTT. return 0; } + + int64_t nack_window_ms = rtt; + if (nack_window_ms < kMinRetransmissionWindowMs) { + nack_window_ms = kMinRetransmissionWindowMs; + } else if (nack_window_ms > kMaxRetransmissionWindowMs) { + nack_window_ms = kMaxRetransmissionWindowMs; + } + retransmission_rate_limiter_->SetWindowSize(nack_window_ms); + uint32_t ntp_secs = 0; uint32_t ntp_frac = 0; uint32_t rtp_timestamp = 0; diff --git a/webrtc/voice_engine/channel.h b/webrtc/voice_engine/channel.h index b51b7ec68e..34e5c5aff5 100644 --- a/webrtc/voice_engine/channel.h +++ b/webrtc/voice_engine/channel.h @@ -36,7 +36,6 @@ #include "webrtc/voice_engine/voice_engine_defines.h" namespace rtc { - class TimestampWrapAroundHandler; } @@ -47,6 +46,7 @@ class Config; class FileWrapper; class PacketRouter; class ProcessThread; +class RateLimiter; class ReceiveStatistics; class RemoteNtpTimeEstimator; class RtcEventLog; @@ -589,6 +589,7 @@ class Channel std::unique_ptr feedback_observer_proxy_; std::unique_ptr seq_num_allocator_proxy_; std::unique_ptr rtp_packet_sender_proxy_; + std::unique_ptr retransmission_rate_limiter_; // TODO(ossu): Remove once GetAudioDecoderFactory() is no longer needed. rtc::scoped_refptr decoder_factory_;