From 287e54820b30764ea95efd46208ea932efdbef69 Mon Sep 17 00:00:00 2001 From: danilchap Date: Tue, 16 Aug 2016 15:15:39 -0700 Subject: [PATCH] Cleanup RtcpReceiver::TMMBRReceived function BUG=webrtc:951 Review-Url: https://codereview.webrtc.org/2250633002 Cr-Commit-Position: refs/heads/master@{#13786} --- .../modules/rtp_rtcp/source/rtcp_receiver.cc | 54 ++++--------------- .../modules/rtp_rtcp/source/rtcp_receiver.h | 4 +- .../rtp_rtcp/source/rtcp_receiver_help.cc | 39 ++++++-------- .../rtp_rtcp/source/rtcp_receiver_help.h | 6 +-- .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 31 +++++------ 5 files changed, 43 insertions(+), 91 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc index 8012f47dc0..340fb990de 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -1234,17 +1234,11 @@ void RTCPReceiver::HandleTransportFeedback( rtcp_parser->Iterate(); } + int32_t RTCPReceiver::UpdateTMMBR() { - int32_t size = TMMBRReceived(0, 0, NULL); - TMMBRSet candidates; - if (size > 0) { - candidates.reserve(size); - // Get candidate set from receiver. - TMMBRReceived(size, 0, &candidates); - } // Find bounding set std::vector bounding = - TMMBRHelp::FindBoundingSet(std::move(candidates)); + TMMBRHelp::FindBoundingSet(TMMBRReceived()); // Set bounding set // Inform remote clients about the new bandwidth // inform the remote client @@ -1399,44 +1393,18 @@ int32_t RTCPReceiver::CNAME(uint32_t remoteSSRC, return 0; } -// no callbacks allowed inside this function -int32_t RTCPReceiver::TMMBRReceived(uint32_t size, - uint32_t accNumCandidates, - TMMBRSet* candidateSet) const { +std::vector RTCPReceiver::TMMBRReceived() const { rtc::CritScope lock(&_criticalSectionRTCPReceiver); + std::vector candidates; - std::map::const_iterator - receiveInfoIt = _receivedInfoMap.begin(); - if (receiveInfoIt == _receivedInfoMap.end()) { - return -1; + int64_t now_ms = _clock->TimeInMilliseconds(); + + for (const auto& kv : _receivedInfoMap) { + RTCPReceiveInformation* receive_info = kv.second; + RTC_DCHECK(receive_info); + receive_info->GetTMMBRSet(now_ms, &candidates); } - uint32_t num = accNumCandidates; - if (candidateSet) { - while( num < size && receiveInfoIt != _receivedInfoMap.end()) { - RTCPReceiveInformation* receiveInfo = receiveInfoIt->second; - if (receiveInfo == NULL) { - return 0; - } - for (uint32_t i = 0; - (num < size) && (i < receiveInfo->TmmbrSet.lengthOfSet()); i++) { - if (receiveInfo->GetTMMBRSet(i, num, candidateSet, - _clock->TimeInMilliseconds()) == 0) { - num++; - } - } - receiveInfoIt++; - } - } else { - while (receiveInfoIt != _receivedInfoMap.end()) { - RTCPReceiveInformation* receiveInfo = receiveInfoIt->second; - if(receiveInfo == NULL) { - return -1; - } - num += receiveInfo->TmmbrSet.lengthOfSet(); - receiveInfoIt++; - } - } - return num; + return candidates; } } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h index f64baeb197..f66df031bc 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h @@ -92,9 +92,7 @@ public: bool RtcpRrSequenceNumberTimeout(int64_t rtcp_interval_ms); // Get TMMBR - int32_t TMMBRReceived(uint32_t size, - uint32_t accNumCandidates, - TMMBRSet* candidateSet) const; + std::vector TMMBRReceived() const; bool UpdateRTCPReceiveInformationTimers(); diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc index bfcc1bdfde..4ad1ddeb76 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc @@ -159,30 +159,23 @@ void RTCPReceiveInformation::InsertTMMBRItem( _tmmbrSetTimeouts.push_back(currentTimeMS); } -int32_t RTCPReceiveInformation::GetTMMBRSet( - const uint32_t sourceIdx, - const uint32_t targetIdx, - TMMBRSet* candidateSet, - const int64_t currentTimeMS) { - if (sourceIdx >= TmmbrSet.lengthOfSet()) { - return -1; +void RTCPReceiveInformation::GetTMMBRSet( + int64_t current_time_ms, + std::vector* candidates) { + // Erase timeout entries. + for (size_t source_idx = 0; source_idx < TmmbrSet.size();) { + // Use audio define since we don't know what interval the remote peer is + // using. + if (current_time_ms - _tmmbrSetTimeouts[source_idx] > + 5 * RTCP_INTERVAL_AUDIO_MS) { + // Value timed out. + TmmbrSet.erase(TmmbrSet.begin() + source_idx); + _tmmbrSetTimeouts.erase(_tmmbrSetTimeouts.begin() + source_idx); + continue; + } + candidates->push_back(TmmbrSet[source_idx]); + ++source_idx; } - if (targetIdx >= candidateSet->sizeOfSet()) { - return -1; - } - // use audio define since we don't know what interval the remote peer is using - if (currentTimeMS - _tmmbrSetTimeouts[sourceIdx] > - 5 * RTCP_INTERVAL_AUDIO_MS) { - // value timed out - TmmbrSet.RemoveEntry(sourceIdx); - _tmmbrSetTimeouts.erase(_tmmbrSetTimeouts.begin() + sourceIdx); - return -1; - } - candidateSet->SetEntry(targetIdx, - TmmbrSet.Tmmbr(sourceIdx), - TmmbrSet.PacketOH(sourceIdx), - TmmbrSet.Ssrc(sourceIdx)); - return 0; } void RTCPReceiveInformation::VerifyAndAllocateBoundingSet( diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.h index 40d1220069..d9febae04b 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.h @@ -109,10 +109,8 @@ public: const int64_t currentTimeMS); // get - int32_t GetTMMBRSet(const uint32_t sourceIdx, - const uint32_t targetIdx, - TMMBRSet* candidateSet, - const int64_t currentTimeMS); + void GetTMMBRSet(int64_t current_time_ms, + std::vector* candidates); int64_t lastTimeReceived; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index cdb71be003..6a2cb22b22 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -1033,7 +1033,7 @@ TEST_F(RtcpReceiverTest, ReceiveReportTimeout) { TEST_F(RtcpReceiverTest, TmmbrReceivedWithNoIncomingPacket) { // This call is expected to fail because no data has arrived. - EXPECT_EQ(-1, rtcp_receiver_->TMMBRReceived(0, 0, nullptr)); + EXPECT_EQ(0u, rtcp_receiver_->TMMBRReceived().size()); } TEST_F(RtcpReceiverTest, TmmbrPacketAccepted) { @@ -1055,12 +1055,10 @@ TEST_F(RtcpReceiverTest, TmmbrPacketAccepted) { rtc::Buffer packet = compound.Build(); EXPECT_EQ(0, InjectRtcpPacket(packet.data(), packet.size())); - EXPECT_EQ(1, rtcp_receiver_->TMMBRReceived(0, 0, nullptr)); - TMMBRSet candidate_set; - candidate_set.VerifyAndAllocateSet(1); - EXPECT_EQ(1, rtcp_receiver_->TMMBRReceived(1, 0, &candidate_set)); - EXPECT_LT(0U, candidate_set.Tmmbr(0)); - EXPECT_EQ(kSenderSsrc, candidate_set.Ssrc(0)); + std::vector candidate_set = rtcp_receiver_->TMMBRReceived(); + EXPECT_EQ(1u, candidate_set.size()); + EXPECT_LT(0U, candidate_set[0].bitrate_bps()); + EXPECT_EQ(kSenderSsrc, candidate_set[0].ssrc()); } TEST_F(RtcpReceiverTest, TmmbrPacketNotForUsIgnored) { @@ -1083,7 +1081,7 @@ TEST_F(RtcpReceiverTest, TmmbrPacketNotForUsIgnored) { ssrcs.insert(kMediaFlowSsrc); rtcp_receiver_->SetSsrcs(kMediaFlowSsrc, ssrcs); EXPECT_EQ(0, InjectRtcpPacket(packet.data(), packet.size())); - EXPECT_EQ(0, rtcp_receiver_->TMMBRReceived(0, 0, nullptr)); + EXPECT_EQ(0u, rtcp_receiver_->TMMBRReceived().size()); } TEST_F(RtcpReceiverTest, TmmbrPacketZeroRateIgnored) { @@ -1105,7 +1103,7 @@ TEST_F(RtcpReceiverTest, TmmbrPacketZeroRateIgnored) { rtc::Buffer packet = compound.Build(); EXPECT_EQ(0, InjectRtcpPacket(packet.data(), packet.size())); - EXPECT_EQ(0, rtcp_receiver_->TMMBRReceived(0, 0, nullptr)); + EXPECT_EQ(0u, rtcp_receiver_->TMMBRReceived().size()); } TEST_F(RtcpReceiverTest, TmmbrThreeConstraintsTimeOut) { @@ -1133,18 +1131,15 @@ TEST_F(RtcpReceiverTest, TmmbrThreeConstraintsTimeOut) { system_clock_.AdvanceTimeMilliseconds(5000); } // It is now starttime + 15. - EXPECT_EQ(3, rtcp_receiver_->TMMBRReceived(0, 0, nullptr)); - TMMBRSet candidate_set; - candidate_set.VerifyAndAllocateSet(3); - EXPECT_EQ(3, rtcp_receiver_->TMMBRReceived(3, 0, &candidate_set)); - EXPECT_LT(0U, candidate_set.Tmmbr(0)); + std::vector candidate_set = rtcp_receiver_->TMMBRReceived(); + EXPECT_EQ(3u, candidate_set.size()); + EXPECT_LT(0U, candidate_set[0].bitrate_bps()); // We expect the timeout to be 25 seconds. Advance the clock by 12 // seconds, timing out the first packet. system_clock_.AdvanceTimeMilliseconds(12000); - // Odd behaviour: Just counting them does not trigger the timeout. - EXPECT_EQ(3, rtcp_receiver_->TMMBRReceived(0, 0, nullptr)); - EXPECT_EQ(2, rtcp_receiver_->TMMBRReceived(3, 0, &candidate_set)); - EXPECT_EQ(kSenderSsrc + 1, candidate_set.Ssrc(0)); + candidate_set = rtcp_receiver_->TMMBRReceived(); + EXPECT_EQ(2u, candidate_set.size()); + EXPECT_EQ(kSenderSsrc + 1, candidate_set[0].ssrc()); } TEST_F(RtcpReceiverTest, Callbacks) {