diff --git a/src/video_engine/vie_remb.cc b/src/video_engine/vie_remb.cc index b8b6d074cf..4e18f250eb 100644 --- a/src/video_engine/vie_remb.cc +++ b/src/video_engine/vie_remb.cc @@ -23,9 +23,10 @@ namespace webrtc { const int kRembSendIntervallMs = 1000; const int kRembTimeOutThresholdMs = 2000; +const unsigned int kRembMinimumBitrateKbps = 50; // % threshold for if we should send a new REMB asap. -const int kSendThresholdPercent = 97; +const unsigned int kSendThresholdPercent = 97; VieRemb::VieRemb(ProcessThread* process_thread) : process_thread_(process_thread), @@ -121,12 +122,17 @@ void VieRemb::OnReceiveBitrateChanged(unsigned int ssrc, unsigned int bitrate) { TickTime::MillisecondTimestamp(), bitrate); } - int new_remb_bitrate = last_send_bitrate_ - - update_time_bitrates_[ssrc].second + 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_ = TickTime::MillisecondTimestamp() - kRembSendIntervallMs; + // If we already have an estimate, check if the new total estimate is below + // kSendThresholdPercent of the previous estimate. + if (last_send_bitrate_ > 0) { + unsigned int new_remb_bitrate = last_send_bitrate_ - + update_time_bitrates_[ssrc].second + 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_ = TickTime::MillisecondTimestamp() - kRembSendIntervallMs; + } } update_time_bitrates_[ssrc] = std::make_pair( TickTime::MillisecondTimestamp(), bitrate); @@ -150,16 +156,6 @@ WebRtc_Word32 VieRemb::Process() { // Calculate total receive bitrate estimate. list_crit_->Enter(); - int total_bitrate = 0; - int num_bitrates = update_time_bitrates_.size(); - - if (num_bitrates == 0) { - list_crit_->Leave(); - return 0; - } - - // TODO(mflodman) Use std::vector and change RTP module API. - unsigned int* ssrcs = new unsigned int[num_bitrates]; // Remove any timed out estimates. SsrcTimeBitrate::iterator it = update_time_bitrates_.begin(); @@ -172,6 +168,17 @@ WebRtc_Word32 VieRemb::Process() { } } + int num_bitrates = update_time_bitrates_.size(); + + if (num_bitrates == 0) { + list_crit_->Leave(); + return 0; + } + + // TODO(mflodman) Use std::vector and change RTP module API. + unsigned int* ssrcs = new unsigned int[num_bitrates]; + + unsigned int total_bitrate = 0; int idx = 0; for (it = update_time_bitrates_.begin(); it != update_time_bitrates_.end(); ++it, ++idx) { @@ -187,6 +194,11 @@ WebRtc_Word32 VieRemb::Process() { sender = receive_modules_.front(); } last_send_bitrate_ = total_bitrate; + + // Never send a REMB lower than last_send_bitrate_. + if (last_send_bitrate_ < kRembMinimumBitrateKbps) { + last_send_bitrate_ = kRembMinimumBitrateKbps; + } list_crit_->Leave(); if (sender) { diff --git a/src/video_engine/vie_remb.h b/src/video_engine/vie_remb.h index 3f530a641b..7fe12f884e 100644 --- a/src/video_engine/vie_remb.h +++ b/src/video_engine/vie_remb.h @@ -75,7 +75,7 @@ class VieRemb : public RemoteBitrateObserver, public Module { // The last time a REMB was sent. int64_t last_remb_time_; - int last_send_bitrate_; + unsigned int last_send_bitrate_; // All RtpRtcp modules to include in the REMB packet. RtpModules receive_modules_; diff --git a/src/video_engine/vie_remb_unittest.cc b/src/video_engine/vie_remb_unittest.cc index f48a988d94..b024877a89 100644 --- a/src/video_engine/vie_remb_unittest.cc +++ b/src/video_engine/vie_remb_unittest.cc @@ -88,6 +88,11 @@ TEST_F(ViERembTest, LowerEstimateToSendRemb) { vie_remb_->OnReceiveBitrateChanged(ssrc[0], bitrate_estimate); EXPECT_CALL(rtp, RemoteSSRC()) .WillRepeatedly(Return(ssrc[0])); + // Call process to get a first estimate. + SleepMs(1010); + EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, 1, _)) + .Times(1); + vie_remb_->Process(); // Lower the estimate with more than 3% to trigger a call to SetREMBData right // away. @@ -113,6 +118,12 @@ TEST_F(ViERembTest, VerifyCombinedBitrateEstimate) { .Times(AnyNumber()) .WillRepeatedly(Return(ssrc[0])); + // Call process to get a first estimate. + EXPECT_CALL(rtp_0, SetREMBData(bitrate_estimate[0], 1, _)) + .Times(1); + SleepMs(1010); + vie_remb_->Process(); + vie_remb_->OnReceiveBitrateChanged(ssrc[1], bitrate_estimate[1] + 100); EXPECT_CALL(rtp_1, RemoteSSRC()) .Times(AnyNumber()) @@ -195,6 +206,13 @@ TEST_F(ViERembTest, ChangeSendRtpModule) { .Times(AnyNumber()) .WillRepeatedly(Return(ssrc[1])); + // Call process to get a first estimate. + SleepMs(1010); + EXPECT_CALL(rtp_0, SetREMBData(bitrate_estimate[0] + bitrate_estimate[1], 2, + _)) + .Times(1); + vie_remb_->Process(); + // Decrease estimate to trigger a REMB. bitrate_estimate[0] = bitrate_estimate[0] - 100; EXPECT_CALL(rtp_0, SetREMBData(bitrate_estimate[0] + bitrate_estimate[1], 2, @@ -231,6 +249,12 @@ TEST_F(ViERembTest, OnlyOneRembForDoubleProcess) { EXPECT_CALL(rtp, RemoteSSRC()) .WillRepeatedly(Return(ssrc[0])); + // Call process to get a first estimate. + SleepMs(1010); + EXPECT_CALL(rtp, SetREMBData(_, _, _)) + .Times(1); + vie_remb_->Process(); + // Lower the estimate, should trigger a call to SetREMBData right away. bitrate_estimate = bitrate_estimate - 100; EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, 1, _)) @@ -277,6 +301,12 @@ TEST_F(ViERembTest, NoSendingRtpModule) { EXPECT_CALL(rtp, RemoteSSRC()) .WillRepeatedly(Return(ssrc[0])); + // Call process to get a first estimate. + SleepMs(1010); + EXPECT_CALL(rtp, SetREMBData(_, _, _)) + .Times(1); + vie_remb_->Process(); + // Lower the estimate to trigger a new packet REMB packet. bitrate_estimate = bitrate_estimate - 100; EXPECT_CALL(rtp, SetREMBData(_, _, _))