From 05843312195912a0fab761702991f5041aba9be3 Mon Sep 17 00:00:00 2001 From: nisse Date: Tue, 18 Apr 2017 23:38:35 -0700 Subject: [PATCH] Delete VieRemb class, move functionality to PacketRouter. Also rename SendFeedback --> SendTransportFeedback. BUG=webrtc:7135 Review-Url: https://codereview.webrtc.org/2789843002 Cr-Commit-Position: refs/heads/master@{#17755} --- .../valgrind/memcheck/suppressions.txt | 15 +- webrtc/call/call.cc | 16 +- webrtc/call/rtp_transport_controller_send.h | 1 - .../congestion_controller_unittest.cc | 29 +- .../include/congestion_controller.h | 6 +- .../receive_side_congestion_controller.h | 1 - .../receive_side_congestion_controller.cc | 4 +- webrtc/modules/pacing/BUILD.gn | 2 + webrtc/modules/pacing/packet_router.cc | 90 ++++++- webrtc/modules/pacing/packet_router.h | 25 +- .../modules/pacing/packet_router_unittest.cc | 230 +++++++++++++++- .../remote_estimator_proxy.cc | 2 +- .../remote_estimator_proxy_unittest.cc | 27 +- webrtc/video/BUILD.gn | 3 - webrtc/video/rtp_stream_receiver.cc | 11 - webrtc/video/rtp_stream_receiver.h | 3 - webrtc/video/rtp_stream_receiver_unittest.cc | 2 +- webrtc/video/video_receive_stream.cc | 4 +- webrtc/video/video_receive_stream.h | 4 +- webrtc/video/video_receive_stream_unittest.cc | 3 +- webrtc/video/video_send_stream.cc | 20 +- webrtc/video/video_send_stream.h | 2 - webrtc/video/vie_remb.cc | 135 ---------- webrtc/video/vie_remb.h | 78 ------ webrtc/video/vie_remb_unittest.cc | 249 ------------------ webrtc/video_receive_stream.h | 9 + 26 files changed, 398 insertions(+), 573 deletions(-) delete mode 100644 webrtc/video/vie_remb.cc delete mode 100644 webrtc/video/vie_remb.h delete mode 100644 webrtc/video/vie_remb_unittest.cc diff --git a/tools-webrtc/valgrind/memcheck/suppressions.txt b/tools-webrtc/valgrind/memcheck/suppressions.txt index 6182f56d70..6242afce55 100644 --- a/tools-webrtc/valgrind/memcheck/suppressions.txt +++ b/tools-webrtc/valgrind/memcheck/suppressions.txt @@ -311,19 +311,6 @@ fun:WebRtcNetEQ_GetSpeechOutputType ... } -{ - vfprintf - Memcheck:Uninitialized - fun:vfprintf - fun:vsnprintf - fun:snprintf - ... - fun:_ZNK7testing8internal18FunctionMockerBaseIFbRKN6webrtc4rtcp17TransportFeedbackEEE32UntypedDescribeUninterestingCallEPKvPSo - fun:_ZN7testing8internal25UntypedFunctionMockerBase17UntypedInvokeWithEPKv - fun:_ZN6webrtc11MockRtpRtcp18SendFeedbackPacketERKNS_4rtcp17TransportFeedbackE - fun:_ZN6webrtc12PacketRouter12SendFeedbackEPNS_4rtcp17TransportFeedbackE - fun:_ZN6webrtc34PacketRouterTest_SendFeedback_Test8TestBodyEv -} #----------------------------------------------------------------------- @@ -541,4 +528,4 @@ fun:_ZN7testing8internal25UntypedFunctionMockerBase17UntypedInvokeWithEPKv fun:_ZNK6webrtc11MockRtpRtcp11FlexfecSsrcEv ... -} \ No newline at end of file +} diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index e381183b12..45f32dd7dd 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -56,7 +56,6 @@ #include "webrtc/video/stats_counter.h" #include "webrtc/video/video_receive_stream.h" #include "webrtc/video/video_send_stream.h" -#include "webrtc/video/vie_remb.h" namespace webrtc { @@ -308,7 +307,6 @@ class Call : public webrtc::Call, std::map network_routes_; std::unique_ptr transport_send_; - VieRemb remb_; ReceiveSideCongestionController receive_side_cc_; const std::unique_ptr video_send_delay_stats_; const int64_t start_ms_; @@ -366,8 +364,7 @@ Call::Call(const Call::Config& config, configured_max_padding_bitrate_bps_(0), estimated_send_bitrate_kbps_counter_(clock_, nullptr, true), pacer_bitrate_kbps_counter_(clock_, nullptr, true), - remb_(clock_), - receive_side_cc_(clock_, &remb_, transport_send->packet_router()), + receive_side_cc_(clock_, transport_send->packet_router()), video_send_delay_stats_(new SendDelayStats(clock_)), start_ms_(clock_->TimeInMilliseconds()), worker_queue_("call_worker_queue") { @@ -405,7 +402,6 @@ Call::Call(const Call::Config& config, } Call::~Call() { - RTC_DCHECK(!remb_.InUse()); RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread()); RTC_CHECK(audio_send_ssrcs_.empty()); @@ -664,7 +660,7 @@ webrtc::VideoSendStream* Call::CreateVideoSendStream( VideoSendStream* send_stream = new VideoSendStream( num_cpu_cores_, module_process_thread_.get(), &worker_queue_, call_stats_.get(), transport_send_.get(), bitrate_allocator_.get(), - video_send_delay_stats_.get(), &remb_, event_log_, std::move(config), + video_send_delay_stats_.get(), event_log_, std::move(config), std::move(encoder_config), suspended_video_send_ssrcs_); { @@ -721,10 +717,10 @@ webrtc::VideoReceiveStream* Call::CreateVideoReceiveStream( TRACE_EVENT0("webrtc", "Call::CreateVideoReceiveStream"); RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread()); - VideoReceiveStream* receive_stream = new VideoReceiveStream( - num_cpu_cores_, transport_send_->packet_router(), - std::move(configuration), module_process_thread_.get(), call_stats_.get(), - &remb_); + VideoReceiveStream* receive_stream = + new VideoReceiveStream(num_cpu_cores_, transport_send_->packet_router(), + std::move(configuration), + module_process_thread_.get(), call_stats_.get()); const webrtc::VideoReceiveStream::Config& config = receive_stream->config(); ReceiveRtpConfig receive_config(config.rtp.extensions, diff --git a/webrtc/call/rtp_transport_controller_send.h b/webrtc/call/rtp_transport_controller_send.h index f4384d4560..117bca069c 100644 --- a/webrtc/call/rtp_transport_controller_send.h +++ b/webrtc/call/rtp_transport_controller_send.h @@ -18,7 +18,6 @@ class PacketRouter; class RtpPacketSender; class SendSideCongestionController; class TransportFeedbackObserver; -class VieRemb; // An RtpTransportController should own everything related to the RTP // transport to/from a remote endpoint. We should have separate diff --git a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc index 56219363fd..3f140684f0 100644 --- a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc +++ b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc @@ -28,6 +28,8 @@ using testing::Return; using testing::SaveArg; using testing::StrictMock; +namespace webrtc { + namespace { const webrtc::PacedPacketInfo kPacingInfo0(0, 5, 2000); const webrtc::PacedPacketInfo kPacingInfo1(1, 8, 4000); @@ -40,9 +42,17 @@ uint32_t AbsSendTime(int64_t t, int64_t denom) { return (((t << 18) + (denom >> 1)) / denom) & 0x00fffffful; } +class MockPacketRouter : public PacketRouter { + public: + MOCK_METHOD2(OnReceiveBitrateChanged, + void(const std::vector& ssrcs, + uint32_t bitrate)); +}; + +const uint32_t kInitialBitrateBps = 60000; + } // namespace -namespace webrtc { namespace test { class CongestionControllerTest : public ::testing::Test { @@ -143,7 +153,6 @@ class CongestionControllerTest : public ::testing::Test { std::unique_ptr bandwidth_observer_; PacketRouter packet_router_; std::unique_ptr controller_; - const uint32_t kInitialBitrateBps = 60000; rtc::Optional target_bitrate_bps_; }; @@ -335,13 +344,11 @@ TEST_F(CongestionControllerTest, GetProbingInterval) { controller_->Process(); } -TEST_F(CongestionControllerTest, OnReceivedPacketWithAbsSendTime) { - NiceMock observer; - StrictMock remote_bitrate_observer; - std::unique_ptr pacer(new NiceMock()); - controller_.reset( - new CongestionController(&clock_, &observer, &remote_bitrate_observer, - &event_log_, &packet_router_, std::move(pacer))); +TEST(ReceiveSideCongestionControllerTest, OnReceivedPacketWithAbsSendTime) { + StrictMock packet_router; + SimulatedClock clock_(123456); + + ReceiveSideCongestionController controller(&clock_, &packet_router); size_t payload_size = 1000; RTPHeader header; @@ -349,14 +356,14 @@ TEST_F(CongestionControllerTest, OnReceivedPacketWithAbsSendTime) { header.extension.hasAbsoluteSendTime = true; std::vector ssrcs; - EXPECT_CALL(remote_bitrate_observer, OnReceiveBitrateChanged(_, _)) + EXPECT_CALL(packet_router, OnReceiveBitrateChanged(_, _)) .WillRepeatedly(SaveArg<0>(&ssrcs)); for (int i = 0; i < 10; ++i) { clock_.AdvanceTimeMilliseconds((1000 * payload_size) / kInitialBitrateBps); int64_t now_ms = clock_.TimeInMilliseconds(); header.extension.absoluteSendTime = AbsSendTime(now_ms, 1000); - controller_->OnReceivedPacket(now_ms, payload_size, header); + controller.OnReceivedPacket(now_ms, payload_size, header); } ASSERT_EQ(1u, ssrcs.size()); diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h index 3bb1c53fa7..0e69047b9a 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -51,11 +51,11 @@ class CongestionController : public CallStatsObserver, CongestionController(const Clock* clock, Observer* observer, - RemoteBitrateObserver* remote_bitrate_observer, + RemoteBitrateObserver* /* remote_bitrate_observer */, RtcEventLog* event_log, PacketRouter* packet_router) : send_side_cc_(clock, observer, event_log, packet_router), - receive_side_cc_(clock, remote_bitrate_observer, packet_router) {} + receive_side_cc_(clock, packet_router) {} CongestionController(const Clock* clock, Observer* observer, RemoteBitrateObserver* remote_bitrate_observer, @@ -63,7 +63,7 @@ class CongestionController : public CallStatsObserver, PacketRouter* packet_router, std::unique_ptr pacer) : send_side_cc_(clock, observer, event_log, std::move(pacer)), - receive_side_cc_(clock, remote_bitrate_observer, packet_router) {} + receive_side_cc_(clock, packet_router) {} virtual ~CongestionController() {} diff --git a/webrtc/modules/congestion_controller/include/receive_side_congestion_controller.h b/webrtc/modules/congestion_controller/include/receive_side_congestion_controller.h index 1d205b59c5..2fa5919f8c 100644 --- a/webrtc/modules/congestion_controller/include/receive_side_congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/receive_side_congestion_controller.h @@ -32,7 +32,6 @@ class ReceiveSideCongestionController : public CallStatsObserver, public: ReceiveSideCongestionController( const Clock* clock, - RemoteBitrateObserver* remote_bitrate_observer, PacketRouter* packet_router); virtual ~ReceiveSideCongestionController() {} diff --git a/webrtc/modules/congestion_controller/receive_side_congestion_controller.cc b/webrtc/modules/congestion_controller/receive_side_congestion_controller.cc index eb5771ab42..b481b26565 100644 --- a/webrtc/modules/congestion_controller/receive_side_congestion_controller.cc +++ b/webrtc/modules/congestion_controller/receive_side_congestion_controller.cc @@ -11,6 +11,7 @@ #include "webrtc/modules/congestion_controller/include/receive_side_congestion_controller.h" #include "webrtc/base/logging.h" +#include "webrtc/modules/pacing/packet_router.h" #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h" #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h" @@ -115,9 +116,8 @@ void ReceiveSideCongestionController::WrappingBitrateEstimator:: ReceiveSideCongestionController::ReceiveSideCongestionController( const Clock* clock, - RemoteBitrateObserver* remote_bitrate_observer, PacketRouter* packet_router) - : remote_bitrate_estimator_(remote_bitrate_observer, clock), + : remote_bitrate_estimator_(packet_router, clock), remote_estimator_proxy_(clock, packet_router) {} void ReceiveSideCongestionController::OnReceivedPacket( diff --git a/webrtc/modules/pacing/BUILD.gn b/webrtc/modules/pacing/BUILD.gn index b8fa8b5f13..d1cf656e6d 100644 --- a/webrtc/modules/pacing/BUILD.gn +++ b/webrtc/modules/pacing/BUILD.gn @@ -30,6 +30,7 @@ rtc_static_library("pacing") { "../../base:rtc_base_approved", "../../logging:rtc_event_log_api", "../../system_wrappers", + "../remote_bitrate_estimator", "../rtp_rtcp", "../utility", ] @@ -47,6 +48,7 @@ if (rtc_include_tests) { deps = [ ":pacing", "../../base:rtc_base_approved", + "../../base:rtc_base_tests_utils", "../../system_wrappers:system_wrappers", "../../test:test_support", "../rtp_rtcp", diff --git a/webrtc/modules/pacing/packet_router.cc b/webrtc/modules/pacing/packet_router.cc index ab86bfa4bd..871ba0fcec 100644 --- a/webrtc/modules/pacing/packet_router.cc +++ b/webrtc/modules/pacing/packet_router.cc @@ -12,13 +12,17 @@ #include "webrtc/base/atomicops.h" #include "webrtc/base/checks.h" +#include "webrtc/base/timeutils.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" namespace webrtc { -PacketRouter::PacketRouter() : transport_seq_(0) { +PacketRouter::PacketRouter() + : last_remb_time_ms_(rtc::TimeMillis()), + last_send_bitrate_bps_(0), + transport_seq_(0) { pacer_thread_checker_.DetachFromThread(); } @@ -31,11 +35,23 @@ void PacketRouter::AddSendRtpModule(RtpRtcp* rtp_module) { rtc::CritScope cs(&modules_crit_); RTC_DCHECK(std::find(rtp_send_modules_.begin(), rtp_send_modules_.end(), rtp_module) == rtp_send_modules_.end()); + if (rtp_send_modules_.empty() && !rtp_receive_modules_.empty()) { + rtp_receive_modules_.front()->SetREMBStatus(false); + } + // Put modules which can use regular payload packets (over rtx) instead of // padding first as it's less of a waste if ((rtp_module->RtxSendStatus() & kRtxRedundantPayloads) > 0) { + if (!rtp_send_modules_.empty()) { + rtp_send_modules_.front()->SetREMBStatus(false); + } rtp_send_modules_.push_front(rtp_module); + rtp_module->SetREMBStatus(true); } else { + if (rtp_send_modules_.empty()) { + rtp_module->SetREMBStatus(true); + } + rtp_send_modules_.push_back(rtp_module); } } @@ -45,12 +61,21 @@ void PacketRouter::RemoveSendRtpModule(RtpRtcp* rtp_module) { RTC_DCHECK(std::find(rtp_send_modules_.begin(), rtp_send_modules_.end(), rtp_module) != rtp_send_modules_.end()); rtp_send_modules_.remove(rtp_module); + rtp_module->SetREMBStatus(false); + if (!rtp_send_modules_.empty()) { + rtp_send_modules_.front()->SetREMBStatus(true); + } else if (!rtp_receive_modules_.empty()) { + rtp_receive_modules_.front()->SetREMBStatus(true); + } } void PacketRouter::AddReceiveRtpModule(RtpRtcp* rtp_module) { rtc::CritScope cs(&modules_crit_); RTC_DCHECK(std::find(rtp_receive_modules_.begin(), rtp_receive_modules_.end(), rtp_module) == rtp_receive_modules_.end()); + if (rtp_send_modules_.empty() && rtp_receive_modules_.empty()) { + rtp_module->SetREMBStatus(true); + } rtp_receive_modules_.push_back(rtp_module); } @@ -60,6 +85,12 @@ void PacketRouter::RemoveReceiveRtpModule(RtpRtcp* rtp_module) { rtp_receive_modules_.end(), rtp_module); RTC_DCHECK(it != rtp_receive_modules_.end()); rtp_receive_modules_.erase(it); + if (rtp_send_modules_.empty()) { + rtp_module->SetREMBStatus(false); + if (!rtp_receive_modules_.empty()) { + rtp_receive_modules_.front()->SetREMBStatus(true); + } + } } bool PacketRouter::TimeToSendPacket(uint32_t ssrc, @@ -121,7 +152,62 @@ uint16_t PacketRouter::AllocateSequenceNumber() { return new_seq; } -bool PacketRouter::SendFeedback(rtcp::TransportFeedback* packet) { +void PacketRouter::OnReceiveBitrateChanged(const std::vector& ssrcs, + uint32_t bitrate_bps) { + const int kRembSendIntervalMs = 200; + + // % threshold for if we should send a new REMB asap. + const uint32_t kSendThresholdPercent = 97; + + int64_t now_ms = rtc::TimeMillis(); + { + rtc::CritScope lock(&remb_crit_); + + // If we already have an estimate, check if the new total estimate is below + // kSendThresholdPercent of the previous estimate. + if (last_send_bitrate_bps_ > 0) { + uint32_t new_remb_bitrate_bps = + last_send_bitrate_bps_ - bitrate_bps_ + bitrate_bps; + + if (new_remb_bitrate_bps < + kSendThresholdPercent * last_send_bitrate_bps_ / 100) { + // The new bitrate estimate is less than kSendThresholdPercent % of the + // last report. Send a REMB asap. + last_remb_time_ms_ = now_ms - kRembSendIntervalMs; + } + } + bitrate_bps_ = bitrate_bps; + + if (now_ms - last_remb_time_ms_ < kRembSendIntervalMs) { + return; + } + // NOTE: Updated if we intend to send the data; we might not have + // a module to actually send it. + last_remb_time_ms_ = now_ms; + last_send_bitrate_bps_ = bitrate_bps; + } + SendRemb(bitrate_bps, ssrcs); +} + +bool PacketRouter::SendRemb(uint32_t bitrate_bps, + const std::vector& ssrcs) { + rtc::CritScope lock(&modules_crit_); + RtpRtcp* remb_module; + if (!rtp_send_modules_.empty()) + remb_module = rtp_send_modules_.front(); + else if (!rtp_receive_modules_.empty()) + remb_module = rtp_receive_modules_.front(); + else + return false; + // The Add* and Remove* methods above ensure that this (and only this) module + // has REMB enabled. REMB should be disabled on all other modules, because + // otherwise, they will send REMB with stale info. + RTC_DCHECK(remb_module->REMB()); + remb_module->SetREMBData(bitrate_bps, ssrcs); + return true; +} + +bool PacketRouter::SendTransportFeedback(rtcp::TransportFeedback* packet) { RTC_DCHECK(pacer_thread_checker_.CalledOnValidThread()); rtc::CritScope cs(&modules_crit_); // Prefer send modules. diff --git a/webrtc/modules/pacing/packet_router.h b/webrtc/modules/pacing/packet_router.h index ffc0c1a59d..ea05d45d77 100644 --- a/webrtc/modules/pacing/packet_router.h +++ b/webrtc/modules/pacing/packet_router.h @@ -20,6 +20,7 @@ #include "webrtc/base/thread_checker.h" #include "webrtc/common_types.h" #include "webrtc/modules/pacing/paced_sender.h" +#include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" namespace webrtc { @@ -35,7 +36,8 @@ class TransportFeedback; // (receiver report). For the latter case, we also keep track of the // receive modules. class PacketRouter : public PacedSender::PacketSender, - public TransportSequenceNumberAllocator { + public TransportSequenceNumberAllocator, + public RemoteBitrateObserver { public: PacketRouter(); virtual ~PacketRouter(); @@ -66,8 +68,20 @@ class PacketRouter : public PacedSender::PacketSender, void SetTransportWideSequenceNumber(uint16_t sequence_number); uint16_t AllocateSequenceNumber() override; + // Called every time there is a new bitrate estimate for a receive channel + // group. This call will trigger a new RTCP REMB packet if the bitrate + // estimate has decreased or if no RTCP REMB packet has been sent for + // a certain time interval. + // Implements RtpReceiveBitrateUpdate. + void OnReceiveBitrateChanged(const std::vector& ssrcs, + uint32_t bitrate_bps) override; + + // Send REMB feedback. + virtual bool SendRemb(uint32_t bitrate_bps, + const std::vector& ssrcs); + // Send transport feedback packet to send-side. - virtual bool SendFeedback(rtcp::TransportFeedback* packet); + virtual bool SendTransportFeedback(rtcp::TransportFeedback* packet); private: rtc::ThreadChecker pacer_thread_checker_; @@ -75,6 +89,13 @@ class PacketRouter : public PacedSender::PacketSender, std::list rtp_send_modules_ GUARDED_BY(modules_crit_); std::vector rtp_receive_modules_ GUARDED_BY(modules_crit_); + rtc::CriticalSection remb_crit_; + // The last time a REMB was sent. + int64_t last_remb_time_ms_ GUARDED_BY(remb_crit_); + uint32_t last_send_bitrate_bps_ GUARDED_BY(remb_crit_); + // The last bitrate update. + uint32_t bitrate_bps_ GUARDED_BY(remb_crit_); + volatile int transport_seq_; RTC_DISALLOW_COPY_AND_ASSIGN(PacketRouter); diff --git a/webrtc/modules/pacing/packet_router_unittest.cc b/webrtc/modules/pacing/packet_router_unittest.cc index be28bf1dbe..28fea1ddc4 100644 --- a/webrtc/modules/pacing/packet_router_unittest.cc +++ b/webrtc/modules/pacing/packet_router_unittest.cc @@ -12,6 +12,7 @@ #include #include "webrtc/base/checks.h" +#include "webrtc/base/fakeclock.h" #include "webrtc/modules/pacing/packet_router.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h" @@ -242,18 +243,237 @@ TEST_F(PacketRouterTest, AllocateSequenceNumbers) { } } -TEST_F(PacketRouterTest, SendFeedback) { +TEST_F(PacketRouterTest, SendTransportFeedback) { MockRtpRtcp rtp_1; MockRtpRtcp rtp_2; packet_router_->AddSendRtpModule(&rtp_1); packet_router_->AddReceiveRtpModule(&rtp_2); rtcp::TransportFeedback feedback; - EXPECT_CALL(rtp_1, SendFeedbackPacket(_)).Times(1); - packet_router_->SendFeedback(&feedback); + EXPECT_CALL(rtp_1, SendFeedbackPacket(_)).Times(1).WillOnce(Return(true)); + packet_router_->SendTransportFeedback(&feedback); packet_router_->RemoveSendRtpModule(&rtp_1); - EXPECT_CALL(rtp_2, SendFeedbackPacket(_)).Times(1); - packet_router_->SendFeedback(&feedback); + EXPECT_CALL(rtp_2, SendFeedbackPacket(_)).Times(1).WillOnce(Return(true)); + packet_router_->SendTransportFeedback(&feedback); packet_router_->RemoveReceiveRtpModule(&rtp_2); } + +TEST(PacketRouterRembTest, PreferSendModuleOverReceiveModule) { + rtc::ScopedFakeClock clock; + MockRtpRtcp rtp_recv; + MockRtpRtcp rtp_send; + PacketRouter packet_router; + + EXPECT_CALL(rtp_recv, SetREMBStatus(true)).Times(1); + packet_router.AddReceiveRtpModule(&rtp_recv); + + const uint32_t bitrate_estimate = 456; + const std::vector ssrcs = {1234}; + + ON_CALL(rtp_recv, REMB()).WillByDefault(Return(true)); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + // Call OnReceiveBitrateChanged twice to get a first estimate. + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + EXPECT_CALL(rtp_recv, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + // Add a send module, which should be preferred over the receive module. + EXPECT_CALL(rtp_recv, SetREMBStatus(false)).Times(1); + EXPECT_CALL(rtp_send, SetREMBStatus(true)).Times(1); + packet_router.AddSendRtpModule(&rtp_send); + ON_CALL(rtp_recv, REMB()).WillByDefault(Return(false)); + ON_CALL(rtp_send, REMB()).WillByDefault(Return(true)); + + // Lower bitrate to send another REMB packet. + EXPECT_CALL(rtp_send, SetREMBData(bitrate_estimate - 100, ssrcs)).Times(1); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate - 100); + + EXPECT_CALL(rtp_send, SetREMBStatus(false)).Times(1); + EXPECT_CALL(rtp_recv, SetREMBStatus(true)).Times(1); + packet_router.RemoveSendRtpModule(&rtp_send); + EXPECT_CALL(rtp_recv, SetREMBStatus(false)).Times(1); + packet_router.RemoveReceiveRtpModule(&rtp_recv); +} + +TEST(PacketRouterRembTest, LowerEstimateToSendRemb) { + rtc::ScopedFakeClock clock; + MockRtpRtcp rtp; + PacketRouter packet_router; + + EXPECT_CALL(rtp, SetREMBStatus(true)).Times(1); + packet_router.AddSendRtpModule(&rtp); + + uint32_t bitrate_estimate = 456; + const std::vector ssrcs = {1234}; + + ON_CALL(rtp, REMB()).WillByDefault(Return(true)); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + // Call OnReceiveBitrateChanged twice to get a first estimate. + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + // Lower the estimate with more than 3% to trigger a call to SetREMBData right + // away. + bitrate_estimate = bitrate_estimate - 100; + EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + EXPECT_CALL(rtp, SetREMBStatus(false)).Times(1); + packet_router.RemoveSendRtpModule(&rtp); +} + +TEST(PacketRouterRembTest, VerifyIncreasingAndDecreasing) { + rtc::ScopedFakeClock clock; + MockRtpRtcp rtp; + PacketRouter packet_router; + packet_router.AddSendRtpModule(&rtp); + + uint32_t bitrate_estimate[] = {456, 789}; + std::vector ssrcs = {1234, 5678}; + + ON_CALL(rtp, REMB()).WillByDefault(Return(true)); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate[0]); + + // Call OnReceiveBitrateChanged twice to get a first estimate. + EXPECT_CALL(rtp, SetREMBData(bitrate_estimate[0], ssrcs)).Times(1); + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate[0]); + + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate[1] + 100); + + // Lower the estimate to trigger a callback. + EXPECT_CALL(rtp, SetREMBData(bitrate_estimate[1], ssrcs)).Times(1); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate[1]); + + packet_router.RemoveSendRtpModule(&rtp); +} + +TEST(PacketRouterRembTest, NoRembForIncreasedBitrate) { + rtc::ScopedFakeClock clock; + MockRtpRtcp rtp; + PacketRouter packet_router; + packet_router.AddSendRtpModule(&rtp); + + uint32_t bitrate_estimate = 456; + std::vector ssrcs = {1234, 5678}; + + ON_CALL(rtp, REMB()).WillByDefault(Return(true)); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + // Call OnReceiveBitrateChanged twice to get a first estimate. + EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + // Increased estimate shouldn't trigger a callback right away. + EXPECT_CALL(rtp, SetREMBData(_, _)).Times(0); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate + 1); + + // Decreasing the estimate less than 3% shouldn't trigger a new callback. + EXPECT_CALL(rtp, SetREMBData(_, _)).Times(0); + int lower_estimate = bitrate_estimate * 98 / 100; + packet_router.OnReceiveBitrateChanged(ssrcs, lower_estimate); + + packet_router.RemoveSendRtpModule(&rtp); +} + +TEST(PacketRouterRembTest, ChangeSendRtpModule) { + rtc::ScopedFakeClock clock; + MockRtpRtcp rtp_send; + MockRtpRtcp rtp_recv; + PacketRouter packet_router; + packet_router.AddSendRtpModule(&rtp_send); + packet_router.AddReceiveRtpModule(&rtp_recv); + + uint32_t bitrate_estimate = 456; + std::vector ssrcs = {1234, 5678}; + + ON_CALL(rtp_send, REMB()).WillByDefault(Return(true)); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + // Call OnReceiveBitrateChanged twice to get a first estimate. + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + EXPECT_CALL(rtp_send, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + // Decrease estimate to trigger a REMB. + bitrate_estimate = bitrate_estimate - 100; + EXPECT_CALL(rtp_send, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + // Remove the sending module -> should get remb on the second module. + packet_router.RemoveSendRtpModule(&rtp_send); + + ON_CALL(rtp_send, REMB()).WillByDefault(Return(false)); + ON_CALL(rtp_recv, REMB()).WillByDefault(Return(true)); + + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + bitrate_estimate = bitrate_estimate - 100; + EXPECT_CALL(rtp_recv, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + packet_router.RemoveReceiveRtpModule(&rtp_recv); +} + +TEST(PacketRouterRembTest, OnlyOneRembForRepeatedOnReceiveBitrateChanged) { + rtc::ScopedFakeClock clock; + MockRtpRtcp rtp; + PacketRouter packet_router; + packet_router.AddSendRtpModule(&rtp); + + uint32_t bitrate_estimate = 456; + const std::vector ssrcs = {1234}; + + ON_CALL(rtp, REMB()).WillByDefault(Return(true)); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + // Call OnReceiveBitrateChanged twice to get a first estimate. + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + EXPECT_CALL(rtp, SetREMBData(_, _)).Times(1); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + // Lower the estimate, should trigger a call to SetREMBData right away. + bitrate_estimate = bitrate_estimate - 100; + EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + // Call OnReceiveBitrateChanged again, this should not trigger a new callback. + EXPECT_CALL(rtp, SetREMBData(_, _)).Times(0); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + packet_router.RemoveSendRtpModule(&rtp); +} + +// Only register receiving modules and make sure we fallback to trigger a REMB +// packet on this one. +TEST(PacketRouterRembTest, NoSendingRtpModule) { + rtc::ScopedFakeClock clock; + MockRtpRtcp rtp; + PacketRouter packet_router; + + EXPECT_CALL(rtp, SetREMBStatus(true)).Times(1); + packet_router.AddReceiveRtpModule(&rtp); + + uint32_t bitrate_estimate = 456; + const std::vector ssrcs = {1234}; + + ON_CALL(rtp, REMB()).WillByDefault(Return(true)); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + // Call OnReceiveBitrateChanged twice to get a first estimate. + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); + + // Lower the estimate to trigger a new packet REMB packet. + EXPECT_CALL(rtp, SetREMBData(bitrate_estimate - 100, ssrcs)).Times(1); + packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate - 100); + + EXPECT_CALL(rtp, SetREMBStatus(false)).Times(1); + packet_router.RemoveReceiveRtpModule(&rtp); +} + } // namespace webrtc diff --git a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc index 74d2ee1025..3700fc428d 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc @@ -83,7 +83,7 @@ void RemoteEstimatorProxy::Process() { rtcp::TransportFeedback feedback_packet; if (BuildFeedbackPacket(&feedback_packet)) { RTC_DCHECK(packet_router_ != nullptr); - packet_router_->SendFeedback(&feedback_packet); + packet_router_->SendTransportFeedback(&feedback_packet); } else { more_to_build = false; } diff --git a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc index 1aeb55bcea..66571083e9 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc @@ -52,7 +52,8 @@ std::vector TimestampsMs( class MockPacketRouter : public PacketRouter { public: - MOCK_METHOD1(SendFeedback, bool(rtcp::TransportFeedback* feedback_packet)); + MOCK_METHOD1(SendTransportFeedback, + bool(rtcp::TransportFeedback* feedback_packet)); }; class RemoteEstimatorProxyTest : public ::testing::Test { @@ -82,7 +83,7 @@ class RemoteEstimatorProxyTest : public ::testing::Test { TEST_F(RemoteEstimatorProxyTest, SendsSinglePacketFeedback) { IncomingPacket(kBaseSeq, kBaseTimeMs); - EXPECT_CALL(router_, SendFeedback(_)) + EXPECT_CALL(router_, SendTransportFeedback(_)) .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence()); EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); @@ -99,7 +100,7 @@ TEST_F(RemoteEstimatorProxyTest, DuplicatedPackets) { IncomingPacket(kBaseSeq, kBaseTimeMs); IncomingPacket(kBaseSeq, kBaseTimeMs + 1000); - EXPECT_CALL(router_, SendFeedback(_)) + EXPECT_CALL(router_, SendTransportFeedback(_)) .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence()); EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); @@ -116,13 +117,13 @@ TEST_F(RemoteEstimatorProxyTest, FeedbackWithMissingStart) { // First feedback. IncomingPacket(kBaseSeq, kBaseTimeMs); IncomingPacket(kBaseSeq + 1, kBaseTimeMs + 1000); - EXPECT_CALL(router_, SendFeedback(_)).WillOnce(Return(true)); + EXPECT_CALL(router_, SendTransportFeedback(_)).WillOnce(Return(true)); Process(); // Second feedback starts with a missing packet (DROP kBaseSeq + 2). IncomingPacket(kBaseSeq + 3, kBaseTimeMs + 3000); - EXPECT_CALL(router_, SendFeedback(_)) + EXPECT_CALL(router_, SendTransportFeedback(_)) .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { EXPECT_EQ(kBaseSeq + 2, feedback_packet->GetBaseSequence()); EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); @@ -142,7 +143,7 @@ TEST_F(RemoteEstimatorProxyTest, SendsFeedbackWithVaryingDeltas) { IncomingPacket(kBaseSeq + 1, kBaseTimeMs + kMaxSmallDeltaMs); IncomingPacket(kBaseSeq + 2, kBaseTimeMs + (2 * kMaxSmallDeltaMs) + 1); - EXPECT_CALL(router_, SendFeedback(_)) + EXPECT_CALL(router_, SendTransportFeedback(_)) .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence()); EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); @@ -165,7 +166,7 @@ TEST_F(RemoteEstimatorProxyTest, SendsFragmentedFeedback) { IncomingPacket(kBaseSeq, kBaseTimeMs); IncomingPacket(kBaseSeq + 1, kBaseTimeMs + kTooLargeDelta); - EXPECT_CALL(router_, SendFeedback(_)) + EXPECT_CALL(router_, SendTransportFeedback(_)) .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence()); EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); @@ -194,7 +195,7 @@ TEST_F(RemoteEstimatorProxyTest, GracefullyHandlesReorderingAndWrap) { IncomingPacket(kBaseSeq, kBaseTimeMs); IncomingPacket(kLargeSeq, kBaseTimeMs + kDeltaMs); - EXPECT_CALL(router_, SendFeedback(_)) + EXPECT_CALL(router_, SendTransportFeedback(_)) .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence()); EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); @@ -210,7 +211,7 @@ TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) { IncomingPacket(kBaseSeq, kBaseTimeMs); IncomingPacket(kBaseSeq + 2, kBaseTimeMs + 2); - EXPECT_CALL(router_, SendFeedback(_)) + EXPECT_CALL(router_, SendTransportFeedback(_)) .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence()); EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); @@ -226,7 +227,7 @@ TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) { IncomingPacket(kBaseSeq + 1, kBaseTimeMs + 1); - EXPECT_CALL(router_, SendFeedback(_)) + EXPECT_CALL(router_, SendTransportFeedback(_)) .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { EXPECT_EQ(kBaseSeq + 1, feedback_packet->GetBaseSequence()); EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); @@ -247,7 +248,7 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { IncomingPacket(kBaseSeq + 2, kBaseTimeMs); - EXPECT_CALL(router_, SendFeedback(_)) + EXPECT_CALL(router_, SendTransportFeedback(_)) .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { EXPECT_EQ(kBaseSeq + 2, feedback_packet->GetBaseSequence()); @@ -259,7 +260,7 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { IncomingPacket(kBaseSeq + 3, kTimeoutTimeMs); // kBaseSeq + 2 times out here. - EXPECT_CALL(router_, SendFeedback(_)) + EXPECT_CALL(router_, SendTransportFeedback(_)) .WillOnce( Invoke([kTimeoutTimeMs](rtcp::TransportFeedback* feedback_packet) { EXPECT_EQ(kBaseSeq + 3, feedback_packet->GetBaseSequence()); @@ -276,7 +277,7 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { IncomingPacket(kBaseSeq, kBaseTimeMs - 1); IncomingPacket(kBaseSeq + 1, kTimeoutTimeMs - 1); - EXPECT_CALL(router_, SendFeedback(_)) + EXPECT_CALL(router_, SendTransportFeedback(_)) .WillOnce( Invoke([kTimeoutTimeMs](rtcp::TransportFeedback* feedback_packet) { EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence()); diff --git a/webrtc/video/BUILD.gn b/webrtc/video/BUILD.gn index c9d9a334e1..524f84b368 100644 --- a/webrtc/video/BUILD.gn +++ b/webrtc/video/BUILD.gn @@ -46,8 +46,6 @@ rtc_static_library("video") { "video_stream_decoder.h", "vie_encoder.cc", "vie_encoder.h", - "vie_remb.cc", - "vie_remb.h", ] if (!build_with_chromium && is_clang) { @@ -204,7 +202,6 @@ if (rtc_include_tests) { "video_receive_stream_unittest.cc", "video_send_stream_tests.cc", "vie_encoder_unittest.cc", - "vie_remb_unittest.cc", ] deps = [ ":video", diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc index 00f1c46efa..88d86ef986 100644 --- a/webrtc/video/rtp_stream_receiver.cc +++ b/webrtc/video/rtp_stream_receiver.cc @@ -38,7 +38,6 @@ #include "webrtc/system_wrappers/include/metrics.h" #include "webrtc/system_wrappers/include/timestamp_extrapolator.h" #include "webrtc/video/receive_statistics_proxy.h" -#include "webrtc/video/vie_remb.h" namespace webrtc { @@ -83,7 +82,6 @@ RtpStreamReceiver::RtpStreamReceiver( Transport* transport, RtcpRttStats* rtt_stats, PacketRouter* packet_router, - VieRemb* remb, const VideoReceiveStream::Config* config, ReceiveStatisticsProxy* receive_stats_proxy, ProcessThread* process_thread, @@ -94,7 +92,6 @@ RtpStreamReceiver::RtpStreamReceiver( : clock_(Clock::GetRealTimeClock()), config_(*config), packet_router_(packet_router), - remb_(remb), process_thread_(process_thread), ntp_estimator_(clock_), rtp_header_parser_(RtpHeaderParser::Create()), @@ -130,10 +127,6 @@ RtpStreamReceiver::RtpStreamReceiver( rtp_rtcp_->SetRTCPStatus(config_.rtp.rtcp_mode); rtp_rtcp_->SetSSRC(config_.rtp.local_ssrc); rtp_rtcp_->SetKeyFrameRequestMethod(kKeyFrameReqPliRtcp); - if (config_.rtp.remb) { - rtp_rtcp_->SetREMBStatus(true); - remb_->AddReceiveChannel(rtp_rtcp_.get()); - } for (size_t i = 0; i < config_.rtp.extensions.size(); ++i) { EnableReceiveRtpHeaderExtension(config_.rtp.extensions[i].uri, @@ -203,10 +196,6 @@ RtpStreamReceiver::~RtpStreamReceiver() { process_thread_->DeRegisterModule(rtp_rtcp_.get()); packet_router_->RemoveReceiveRtpModule(rtp_rtcp_.get()); - rtp_rtcp_->SetREMBStatus(false); - if (config_.rtp.remb) { - remb_->RemoveReceiveChannel(rtp_rtcp_.get()); - } UpdateHistograms(); } diff --git a/webrtc/video/rtp_stream_receiver.h b/webrtc/video/rtp_stream_receiver.h index 42fa30eed0..afd19a7add 100644 --- a/webrtc/video/rtp_stream_receiver.h +++ b/webrtc/video/rtp_stream_receiver.h @@ -50,7 +50,6 @@ class RtpReceiver; class Transport; class UlpfecReceiver; class VCMTiming; -class VieRemb; namespace vcm { class VideoReceiver; @@ -68,7 +67,6 @@ class RtpStreamReceiver : public RtpData, Transport* transport, RtcpRttStats* rtt_stats, PacketRouter* packet_router, - VieRemb* remb, const VideoReceiveStream::Config* config, ReceiveStatisticsProxy* receive_stats_proxy, ProcessThread* process_thread, @@ -160,7 +158,6 @@ class RtpStreamReceiver : public RtpData, // Ownership of this object lies with VideoReceiveStream, which owns |this|. const VideoReceiveStream::Config& config_; PacketRouter* const packet_router_; - VieRemb* const remb_; ProcessThread* const process_thread_; RemoteNtpTimeEstimator ntp_estimator_; diff --git a/webrtc/video/rtp_stream_receiver_unittest.cc b/webrtc/video/rtp_stream_receiver_unittest.cc index aace1b7bfa..c28ae81042 100644 --- a/webrtc/video/rtp_stream_receiver_unittest.cc +++ b/webrtc/video/rtp_stream_receiver_unittest.cc @@ -104,7 +104,7 @@ class RtpStreamReceiverTest : public testing::Test { void SetUp() { rtp_stream_receiver_.reset(new RtpStreamReceiver( - &mock_transport_, nullptr, &packet_router_, nullptr, &config_, + &mock_transport_, nullptr, &packet_router_, &config_, nullptr, process_thread_.get(), &mock_nack_sender_, &mock_key_frame_request_sender_, &mock_on_complete_frame_callback_, &timing_)); diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index c59af899a4..cee62d297c 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -173,8 +173,7 @@ VideoReceiveStream::VideoReceiveStream( PacketRouter* packet_router, VideoReceiveStream::Config config, ProcessThread* process_thread, - CallStats* call_stats, - VieRemb* remb) + CallStats* call_stats) : transport_adapter_(config.rtcp_send_transport), config_(std::move(config)), num_cpu_cores_(num_cpu_cores), @@ -188,7 +187,6 @@ VideoReceiveStream::VideoReceiveStream( rtp_stream_receiver_(&transport_adapter_, call_stats_->rtcp_rtt_stats(), packet_router, - remb, &config_, &stats_proxy_, process_thread_, diff --git a/webrtc/video/video_receive_stream.h b/webrtc/video/video_receive_stream.h index cb2c73fe28..73bacdd520 100644 --- a/webrtc/video/video_receive_stream.h +++ b/webrtc/video/video_receive_stream.h @@ -35,7 +35,6 @@ class CallStats; class IvfFileWriter; class ProcessThread; class RTPFragmentationHeader; -class VieRemb; class VCMTiming; class VCMJitterEstimator; @@ -53,8 +52,7 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, PacketRouter* packet_router, VideoReceiveStream::Config config, ProcessThread* process_thread, - CallStats* call_stats, - VieRemb* remb); + CallStats* call_stats); ~VideoReceiveStream() override; const Config& config() const { return config_; } diff --git a/webrtc/video/video_receive_stream_unittest.cc b/webrtc/video/video_receive_stream_unittest.cc index 29f2497b23..d536bf1dc6 100644 --- a/webrtc/video/video_receive_stream_unittest.cc +++ b/webrtc/video/video_receive_stream_unittest.cc @@ -93,8 +93,7 @@ class VideoReceiveStreamTest : public testing::Test { video_receive_stream_.reset(new webrtc::internal::VideoReceiveStream( kDefaultNumCpuCores, - &packet_router_, config_.Copy(), process_thread_.get(), &call_stats_, - nullptr)); // remb + &packet_router_, config_.Copy(), process_thread_.get(), &call_stats_)); } protected: diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 24e7e89688..411f2796b4 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -34,7 +34,6 @@ #include "webrtc/system_wrappers/include/field_trial.h" #include "webrtc/video/call_stats.h" #include "webrtc/video/payload_router.h" -#include "webrtc/video/vie_remb.h" #include "webrtc/video_send_stream.h" namespace webrtc { @@ -334,7 +333,6 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, RtpTransportControllerSendInterface* transport, BitrateAllocator* bitrate_allocator, SendDelayStats* send_delay_stats, - VieRemb* remb, ViEEncoder* vie_encoder, RtcEventLog* event_log, const VideoSendStream::Config* config, @@ -418,7 +416,6 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, CallStats* const call_stats_; RtpTransportControllerSendInterface* const transport_; BitrateAllocator* const bitrate_allocator_; - VieRemb* const remb_; // TODO(brandtr): Move ownership to PayloadRouter. std::unique_ptr flexfec_sender_; @@ -467,7 +464,6 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { RtpTransportControllerSendInterface* transport, BitrateAllocator* bitrate_allocator, SendDelayStats* send_delay_stats, - VieRemb* remb, RtcEventLog* event_log, const VideoSendStream::Config* config, int initial_encoder_max_bitrate, @@ -480,7 +476,6 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { transport_(transport), bitrate_allocator_(bitrate_allocator), send_delay_stats_(send_delay_stats), - remb_(remb), event_log_(event_log), config_(config), initial_encoder_max_bitrate_(initial_encoder_max_bitrate), @@ -492,7 +487,7 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { bool Run() override { send_stream_->reset(new VideoSendStreamImpl( stats_proxy_, rtc::TaskQueue::Current(), call_stats_, transport_, - bitrate_allocator_, send_delay_stats_, remb_, vie_encoder_, event_log_, + bitrate_allocator_, send_delay_stats_, vie_encoder_, event_log_, config_, initial_encoder_max_bitrate_, std::move(suspended_ssrcs_))); return true; } @@ -505,7 +500,6 @@ class VideoSendStream::ConstructionTask : public rtc::QueuedTask { RtpTransportControllerSendInterface* const transport_; BitrateAllocator* const bitrate_allocator_; SendDelayStats* const send_delay_stats_; - VieRemb* const remb_; RtcEventLog* const event_log_; const VideoSendStream::Config* config_; int initial_encoder_max_bitrate_; @@ -619,7 +613,6 @@ VideoSendStream::VideoSendStream( RtpTransportControllerSendInterface* transport, BitrateAllocator* bitrate_allocator, SendDelayStats* send_delay_stats, - VieRemb* remb, RtcEventLog* event_log, VideoSendStream::Config config, VideoEncoderConfig encoder_config, @@ -637,7 +630,7 @@ VideoSendStream::VideoSendStream( worker_queue_->PostTask(std::unique_ptr(new ConstructionTask( &send_stream_, &thread_sync_event_, &stats_proxy_, vie_encoder_.get(), module_process_thread, call_stats, transport, bitrate_allocator, - send_delay_stats, remb, event_log, &config_, + send_delay_stats, event_log, &config_, encoder_config.max_bitrate_bps, suspended_ssrcs))); // Wait for ConstructionTask to complete so that |send_stream_| can be used. @@ -752,7 +745,6 @@ VideoSendStreamImpl::VideoSendStreamImpl( RtpTransportControllerSendInterface* transport, BitrateAllocator* bitrate_allocator, SendDelayStats* send_delay_stats, - VieRemb* remb, ViEEncoder* vie_encoder, RtcEventLog* event_log, const VideoSendStream::Config* config, @@ -769,7 +761,6 @@ VideoSendStreamImpl::VideoSendStreamImpl( call_stats_(call_stats), transport_(transport), bitrate_allocator_(bitrate_allocator), - remb_(remb), flexfec_sender_(MaybeCreateFlexfecSender(*config_)), max_padding_bitrate_(0), encoder_min_bitrate_bps_(0), @@ -808,7 +799,6 @@ VideoSendStreamImpl::VideoSendStreamImpl( RTC_DCHECK(!config_->rtp.ssrcs.empty()); RTC_DCHECK(call_stats_); - RTC_DCHECK(remb_); RTC_DCHECK(transport_); RTC_DCHECK(transport_->send_side_cc()); @@ -836,9 +826,6 @@ VideoSendStreamImpl::VideoSendStreamImpl( } } - remb_->AddRembSender(rtp_rtcp_modules_[0]); - rtp_rtcp_modules_[0]->SetREMBStatus(true); - ConfigureProtection(); ConfigureSsrcs(); @@ -896,9 +883,6 @@ VideoSendStreamImpl::~VideoSendStreamImpl() { << "VideoSendStreamImpl::Stop not called"; LOG(LS_INFO) << "~VideoSendStreamInternal: " << config_->ToString(); - rtp_rtcp_modules_[0]->SetREMBStatus(false); - remb_->RemoveRembSender(rtp_rtcp_modules_[0]); - for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) { transport_->packet_router()->RemoveSendRtpModule(rtp_rtcp); delete rtp_rtcp; diff --git a/webrtc/video/video_send_stream.h b/webrtc/video/video_send_stream.h index 0cc1a34611..dcdc71c2d4 100644 --- a/webrtc/video/video_send_stream.h +++ b/webrtc/video/video_send_stream.h @@ -37,7 +37,6 @@ class ProcessThread; class RtpRtcp; class RtpTransportControllerSendInterface; class RtcEventLog; -class VieRemb; namespace internal { @@ -55,7 +54,6 @@ class VideoSendStream : public webrtc::VideoSendStream { RtpTransportControllerSendInterface* transport, BitrateAllocator* bitrate_allocator, SendDelayStats* send_delay_stats, - VieRemb* remb, RtcEventLog* event_log, VideoSendStream::Config config, VideoEncoderConfig encoder_config, diff --git a/webrtc/video/vie_remb.cc b/webrtc/video/vie_remb.cc deleted file mode 100644 index be751016d1..0000000000 --- a/webrtc/video/vie_remb.cc +++ /dev/null @@ -1,135 +0,0 @@ -/* - * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "webrtc/video/vie_remb.h" - -#include - -#include - -#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" -#include "webrtc/modules/utility/include/process_thread.h" - -namespace webrtc { - -const int kRembSendIntervalMs = 200; - -// % threshold for if we should send a new REMB asap. -const uint32_t kSendThresholdPercent = 97; - -VieRemb::VieRemb(Clock* clock) - : clock_(clock), - last_remb_time_(clock_->TimeInMilliseconds()), - last_send_bitrate_(0), - bitrate_(0) {} - -VieRemb::~VieRemb() {} - -void VieRemb::AddReceiveChannel(RtpRtcp* rtp_rtcp) { - assert(rtp_rtcp); - - rtc::CritScope lock(&list_crit_); - if (std::find(receive_modules_.begin(), receive_modules_.end(), rtp_rtcp) != - receive_modules_.end()) - return; - - // The module probably doesn't have a remote SSRC yet, so don't add it to the - // map. - receive_modules_.push_back(rtp_rtcp); -} - -void VieRemb::RemoveReceiveChannel(RtpRtcp* rtp_rtcp) { - assert(rtp_rtcp); - - rtc::CritScope lock(&list_crit_); - for (RtpModules::iterator it = receive_modules_.begin(); - it != receive_modules_.end(); ++it) { - if ((*it) == rtp_rtcp) { - receive_modules_.erase(it); - break; - } - } -} - -void VieRemb::AddRembSender(RtpRtcp* rtp_rtcp) { - assert(rtp_rtcp); - - rtc::CritScope lock(&list_crit_); - - // Verify this module hasn't been added earlier. - if (std::find(rtcp_sender_.begin(), rtcp_sender_.end(), rtp_rtcp) != - rtcp_sender_.end()) - return; - rtcp_sender_.push_back(rtp_rtcp); -} - -void VieRemb::RemoveRembSender(RtpRtcp* rtp_rtcp) { - assert(rtp_rtcp); - - rtc::CritScope lock(&list_crit_); - for (RtpModules::iterator it = rtcp_sender_.begin(); - it != rtcp_sender_.end(); ++it) { - if ((*it) == rtp_rtcp) { - rtcp_sender_.erase(it); - return; - } - } -} - -bool VieRemb::InUse() const { - rtc::CritScope lock(&list_crit_); - return !receive_modules_.empty() || !rtcp_sender_.empty(); -} - -void VieRemb::OnReceiveBitrateChanged(const std::vector& ssrcs, - uint32_t bitrate) { - RtpRtcp* sender = nullptr; - { - rtc::CritScope lock(&list_crit_); - // If we already have an estimate, check if the new total estimate is below - // kSendThresholdPercent of the previous estimate. - if (last_send_bitrate_ > 0) { - uint32_t new_remb_bitrate = last_send_bitrate_ - bitrate_ + bitrate; - - if (new_remb_bitrate < kSendThresholdPercent * last_send_bitrate_ / 100) { - // The new bitrate estimate is less than kSendThresholdPercent % of the - // last report. Send a REMB asap. - last_remb_time_ = clock_->TimeInMilliseconds() - kRembSendIntervalMs; - } - } - bitrate_ = bitrate; - - // Calculate total receive bitrate estimate. - int64_t now = clock_->TimeInMilliseconds(); - - if (now - last_remb_time_ < kRembSendIntervalMs) { - return; - } - last_remb_time_ = now; - - if (ssrcs.empty() || receive_modules_.empty()) { - return; - } - - // Send a REMB packet. - if (!rtcp_sender_.empty()) { - sender = rtcp_sender_.front(); - } else { - sender = receive_modules_.front(); - } - last_send_bitrate_ = bitrate_; - } - - if (sender) { - sender->SetREMBData(bitrate_, ssrcs); - } -} - -} // namespace webrtc diff --git a/webrtc/video/vie_remb.h b/webrtc/video/vie_remb.h deleted file mode 100644 index d2c60dbeb1..0000000000 --- a/webrtc/video/vie_remb.h +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef WEBRTC_VIDEO_VIE_REMB_H_ -#define WEBRTC_VIDEO_VIE_REMB_H_ - -#include -#include -#include - -#include "webrtc/base/criticalsection.h" -#include "webrtc/modules/include/module.h" -#include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" -#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" - -namespace webrtc { - -class ProcessThread; -class RtpRtcp; - -class VieRemb : public RemoteBitrateObserver { - public: - explicit VieRemb(Clock* clock); - ~VieRemb(); - - // Called to add a receive channel to include in the REMB packet. - void AddReceiveChannel(RtpRtcp* rtp_rtcp); - - // Removes the specified channel from REMB estimate. - void RemoveReceiveChannel(RtpRtcp* rtp_rtcp); - - // Called to add a module that can generate and send REMB RTCP. - void AddRembSender(RtpRtcp* rtp_rtcp); - - // Removes a REMB RTCP sender. - void RemoveRembSender(RtpRtcp* rtp_rtcp); - - // Returns true if the instance is in use, false otherwise. - bool InUse() const; - - // Called every time there is a new bitrate estimate for a receive channel - // group. This call will trigger a new RTCP REMB packet if the bitrate - // estimate has decreased or if no RTCP REMB packet has been sent for - // a certain time interval. - // Implements RtpReceiveBitrateUpdate. - virtual void OnReceiveBitrateChanged(const std::vector& ssrcs, - uint32_t bitrate); - - private: - typedef std::list RtpModules; - - Clock* const clock_; - rtc::CriticalSection list_crit_; - - // The last time a REMB was sent. - int64_t last_remb_time_; - uint32_t last_send_bitrate_; - - // All RtpRtcp modules to include in the REMB packet. - RtpModules receive_modules_; - - // All modules that can send REMB RTCP. - RtpModules rtcp_sender_; - - // The last bitrate update. - uint32_t bitrate_; -}; - -} // namespace webrtc - -#endif // WEBRTC_VIDEO_VIE_REMB_H_ diff --git a/webrtc/video/vie_remb_unittest.cc b/webrtc/video/vie_remb_unittest.cc deleted file mode 100644 index 46ede3f9b5..0000000000 --- a/webrtc/video/vie_remb_unittest.cc +++ /dev/null @@ -1,249 +0,0 @@ -/* - * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include -#include - -#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" -#include "webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h" -#include "webrtc/modules/utility/include/mock/mock_process_thread.h" -#include "webrtc/test/gmock.h" -#include "webrtc/test/gtest.h" -#include "webrtc/video/vie_remb.h" - -using ::testing::_; -using ::testing::AnyNumber; -using ::testing::NiceMock; -using ::testing::Return; - -namespace webrtc { - -class ViERembTest : public ::testing::Test { - public: - ViERembTest() : fake_clock_(12345) {} - - protected: - virtual void SetUp() { - process_thread_.reset(new NiceMock); - vie_remb_.reset(new VieRemb(&fake_clock_)); - } - SimulatedClock fake_clock_; - std::unique_ptr process_thread_; - std::unique_ptr vie_remb_; -}; - -TEST_F(ViERembTest, OneModuleTestForSendingRemb) { - MockRtpRtcp rtp; - vie_remb_->AddReceiveChannel(&rtp); - vie_remb_->AddRembSender(&rtp); - - const uint32_t bitrate_estimate = 456; - uint32_t ssrc = 1234; - std::vector ssrcs(&ssrc, &ssrc + 1); - - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - - fake_clock_.AdvanceTimeMilliseconds(1000); - EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, ssrcs)) - .Times(1); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - - // Lower bitrate to send another REMB packet. - EXPECT_CALL(rtp, SetREMBData(bitrate_estimate - 100, ssrcs)) - .Times(1); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate - 100); - - vie_remb_->RemoveReceiveChannel(&rtp); - vie_remb_->RemoveRembSender(&rtp); -} - -TEST_F(ViERembTest, LowerEstimateToSendRemb) { - MockRtpRtcp rtp; - vie_remb_->AddReceiveChannel(&rtp); - vie_remb_->AddRembSender(&rtp); - - uint32_t bitrate_estimate = 456; - uint32_t ssrc = 1234; - std::vector ssrcs(&ssrc, &ssrc + 1); - - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - // Call OnReceiveBitrateChanged twice to get a first estimate. - fake_clock_.AdvanceTimeMilliseconds(1000); - EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, ssrcs)) - .Times(1); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - - // Lower the estimate with more than 3% to trigger a call to SetREMBData right - // away. - bitrate_estimate = bitrate_estimate - 100; - EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, ssrcs)) - .Times(1); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); -} - -TEST_F(ViERembTest, VerifyIncreasingAndDecreasing) { - MockRtpRtcp rtp_0; - MockRtpRtcp rtp_1; - vie_remb_->AddReceiveChannel(&rtp_0); - vie_remb_->AddRembSender(&rtp_0); - vie_remb_->AddReceiveChannel(&rtp_1); - - uint32_t bitrate_estimate[] = {456, 789}; - uint32_t ssrc[] = {1234, 5678}; - std::vector ssrcs(ssrc, ssrc + sizeof(ssrc) / sizeof(ssrc[0])); - - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate[0]); - - // Call OnReceiveBitrateChanged twice to get a first estimate. - EXPECT_CALL(rtp_0, SetREMBData(bitrate_estimate[0], ssrcs)) - .Times(1); - fake_clock_.AdvanceTimeMilliseconds(1000); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate[0]); - - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate[1] + 100); - - // Lower the estimate to trigger a callback. - EXPECT_CALL(rtp_0, SetREMBData(bitrate_estimate[1], ssrcs)) - .Times(1); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate[1]); - - vie_remb_->RemoveReceiveChannel(&rtp_0); - vie_remb_->RemoveRembSender(&rtp_0); - vie_remb_->RemoveReceiveChannel(&rtp_1); -} - -TEST_F(ViERembTest, NoRembForIncreasedBitrate) { - MockRtpRtcp rtp_0; - MockRtpRtcp rtp_1; - vie_remb_->AddReceiveChannel(&rtp_0); - vie_remb_->AddRembSender(&rtp_0); - vie_remb_->AddReceiveChannel(&rtp_1); - - uint32_t bitrate_estimate = 456; - uint32_t ssrc[] = {1234, 5678}; - std::vector ssrcs(ssrc, ssrc + sizeof(ssrc) / sizeof(ssrc[0])); - - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - // Call OnReceiveBitrateChanged twice to get a first estimate. - fake_clock_.AdvanceTimeMilliseconds(1000); - EXPECT_CALL(rtp_0, SetREMBData(bitrate_estimate, ssrcs)) - .Times(1); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - - // Increased estimate shouldn't trigger a callback right away. - EXPECT_CALL(rtp_0, SetREMBData(_, _)) - .Times(0); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate + 1); - - // Decreasing the estimate less than 3% shouldn't trigger a new callback. - EXPECT_CALL(rtp_0, SetREMBData(_, _)) - .Times(0); - int lower_estimate = bitrate_estimate * 98 / 100; - vie_remb_->OnReceiveBitrateChanged(ssrcs, lower_estimate); - - vie_remb_->RemoveReceiveChannel(&rtp_1); - vie_remb_->RemoveReceiveChannel(&rtp_0); - vie_remb_->RemoveRembSender(&rtp_0); -} - -TEST_F(ViERembTest, ChangeSendRtpModule) { - MockRtpRtcp rtp_0; - MockRtpRtcp rtp_1; - vie_remb_->AddReceiveChannel(&rtp_0); - vie_remb_->AddRembSender(&rtp_0); - vie_remb_->AddReceiveChannel(&rtp_1); - - uint32_t bitrate_estimate = 456; - uint32_t ssrc[] = {1234, 5678}; - std::vector ssrcs(ssrc, ssrc + sizeof(ssrc) / sizeof(ssrc[0])); - - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - // Call OnReceiveBitrateChanged twice to get a first estimate. - fake_clock_.AdvanceTimeMilliseconds(1000); - EXPECT_CALL(rtp_0, SetREMBData(bitrate_estimate, ssrcs)) - .Times(1); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - - // Decrease estimate to trigger a REMB. - bitrate_estimate = bitrate_estimate - 100; - EXPECT_CALL(rtp_0, SetREMBData(bitrate_estimate, ssrcs)) - .Times(1); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - - // Remove the sending module, add it again -> should get remb on the second - // module. - vie_remb_->RemoveRembSender(&rtp_0); - vie_remb_->AddRembSender(&rtp_1); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - - bitrate_estimate = bitrate_estimate - 100; - EXPECT_CALL(rtp_1, SetREMBData(bitrate_estimate, ssrcs)) - .Times(1); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - - vie_remb_->RemoveReceiveChannel(&rtp_0); - vie_remb_->RemoveReceiveChannel(&rtp_1); -} - -TEST_F(ViERembTest, OnlyOneRembForDoubleProcess) { - MockRtpRtcp rtp; - uint32_t bitrate_estimate = 456; - uint32_t ssrc = 1234; - std::vector ssrcs(&ssrc, &ssrc + 1); - - vie_remb_->AddReceiveChannel(&rtp); - vie_remb_->AddRembSender(&rtp); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - // Call OnReceiveBitrateChanged twice to get a first estimate. - fake_clock_.AdvanceTimeMilliseconds(1000); - EXPECT_CALL(rtp, SetREMBData(_, _)) - .Times(1); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - - // Lower the estimate, should trigger a call to SetREMBData right away. - bitrate_estimate = bitrate_estimate - 100; - EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, ssrcs)) - .Times(1); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - - // Call OnReceiveBitrateChanged again, this should not trigger a new callback. - EXPECT_CALL(rtp, SetREMBData(_, _)) - .Times(0); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - vie_remb_->RemoveReceiveChannel(&rtp); - vie_remb_->RemoveRembSender(&rtp); -} - -// Only register receiving modules and make sure we fallback to trigger a REMB -// packet on this one. -TEST_F(ViERembTest, NoSendingRtpModule) { - MockRtpRtcp rtp; - vie_remb_->AddReceiveChannel(&rtp); - - uint32_t bitrate_estimate = 456; - uint32_t ssrc = 1234; - std::vector ssrcs(&ssrc, &ssrc + 1); - - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - - // Call OnReceiveBitrateChanged twice to get a first estimate. - fake_clock_.AdvanceTimeMilliseconds(1000); - EXPECT_CALL(rtp, SetREMBData(_, _)) - .Times(1); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - - // Lower the estimate to trigger a new packet REMB packet. - bitrate_estimate = bitrate_estimate - 100; - EXPECT_CALL(rtp, SetREMBData(_, _)) - .Times(1); - vie_remb_->OnReceiveBitrateChanged(ssrcs, bitrate_estimate); -} - -} // namespace webrtc diff --git a/webrtc/video_receive_stream.h b/webrtc/video_receive_stream.h index 33d779d9b3..b3a98c617a 100644 --- a/webrtc/video_receive_stream.h +++ b/webrtc/video_receive_stream.h @@ -132,6 +132,15 @@ class VideoReceiveStream { bool receiver_reference_time_report = false; } rtcp_xr; + // TODO(nisse): This remb setting is currently set but never + // applied. REMB logic is now the responsibility of + // PacketRouter, and it will generate REMB feedback if + // OnReceiveBitrateChanged is used, which depends on how the + // estimators belonging to the ReceiveSideCongestionController + // are configured. Decide if this setting should be deleted, and + // if it needs to be replaced by a setting in PacketRouter to + // disable REMB feedback. + // See draft-alvestrand-rmcat-remb for information. bool remb = false;