From 2f69ce949855b8e245c712add3c66276dae6f314 Mon Sep 17 00:00:00 2001 From: danilchap Date: Tue, 16 Aug 2016 03:21:38 -0700 Subject: [PATCH] Cleaned out candidateSet member from TMMBRHelp class leaving that class memberless. BUG=webrtc:5565 Review-Url: https://codereview.webrtc.org/2234783002 Cr-Commit-Position: refs/heads/master@{#13776} --- .../modules/rtp_rtcp/source/rtcp_receiver.cc | 25 ++-- webrtc/modules/rtp_rtcp/source/rtcp_sender.cc | 17 ++- webrtc/modules/rtp_rtcp/source/tmmbr_help.cc | 122 ++++++------------ webrtc/modules/rtp_rtcp/source/tmmbr_help.h | 15 +-- 4 files changed, 63 insertions(+), 116 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc index 31a11e2892..8012f47dc0 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -13,6 +13,8 @@ #include #include +#include + #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" #include "webrtc/base/trace_event.h" @@ -1233,18 +1235,16 @@ void RTCPReceiver::HandleTransportFeedback( rtcp_parser->Iterate(); } int32_t RTCPReceiver::UpdateTMMBR() { - TMMBRHelp tmmbr_help; - uint32_t bitrate = 0; - uint32_t accNumCandidates = 0; - int32_t size = TMMBRReceived(0, 0, NULL); + TMMBRSet candidates; if (size > 0) { - TMMBRSet* candidateSet = tmmbr_help.VerifyAndAllocateCandidateSet(size); + candidates.reserve(size); // Get candidate set from receiver. - accNumCandidates = TMMBRReceived(size, accNumCandidates, candidateSet); + TMMBRReceived(size, 0, &candidates); } // Find bounding set - std::vector bounding = tmmbr_help.FindTMMBRBoundingSet(); + std::vector bounding = + TMMBRHelp::FindBoundingSet(std::move(candidates)); // Set bounding set // Inform remote clients about the new bandwidth // inform the remote client @@ -1256,12 +1256,11 @@ int32_t RTCPReceiver::UpdateTMMBR() { // empty bounding set has been sent return 0; } - // Get net bitrate from bounding set depending on sent packet rate - if (tmmbr_help.CalcMinBitRate(&bitrate)) { - // we have a new bandwidth estimate on this channel - if (_cbRtcpBandwidthObserver) { - _cbRtcpBandwidthObserver->OnReceivedEstimatedBitrate(bitrate * 1000); - } + // We have a new bandwidth estimate on this channel. + if (_cbRtcpBandwidthObserver) { + uint64_t bitrate_bps = TMMBRHelp::CalcMinBitrateBps(bounding); + if (bitrate_bps <= std::numeric_limits::max()) + _cbRtcpBandwidthObserver->OnReceivedEstimatedBitrate(bitrate_bps); } return 0; } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc index 23da9cac95..5365d19abb 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc @@ -589,22 +589,20 @@ std::unique_ptr RTCPSender::BuildTMMBR( // * If the sender is an owner of the TMMBN -> send TMMBR // * If not an owner but the TMMBR would enter the TMMBN -> send TMMBR - TMMBRHelp tmmbr_help; // get current bounding set from RTCP receiver bool tmmbrOwner = false; - // store in candidateSet, allocates one extra slot - TMMBRSet* candidateSet = tmmbr_help.CandidateSet(); + TMMBRSet candidates; // holding critical_section_rtcp_sender_ while calling RTCPreceiver which // will accuire criticalSectionRTCPReceiver_ is a potental deadlock but // since RTCPreceiver is not doing the reverse we should be fine int32_t lengthOfBoundingSet = - ctx.feedback_state_.module->BoundingSet(&tmmbrOwner, candidateSet); + ctx.feedback_state_.module->BoundingSet(&tmmbrOwner, &candidates); if (lengthOfBoundingSet > 0) { for (int32_t i = 0; i < lengthOfBoundingSet; i++) { - if (candidateSet->Tmmbr(i) == tmmbr_send_ && - candidateSet->PacketOH(i) == packet_oh_send_) { + if (candidates.Tmmbr(i) == tmmbr_send_ && + candidates.PacketOH(i) == packet_oh_send_) { // Do not send the same tuple. return nullptr; } @@ -612,11 +610,12 @@ std::unique_ptr RTCPSender::BuildTMMBR( if (!tmmbrOwner) { // use received bounding set as candidate set // add current tuple - candidateSet->SetEntry(lengthOfBoundingSet, tmmbr_send_, packet_oh_send_, - ssrc_); + candidates.SetEntry(lengthOfBoundingSet, tmmbr_send_, packet_oh_send_, + ssrc_); // find bounding set - std::vector bounding = tmmbr_help.FindTMMBRBoundingSet(); + std::vector bounding = + TMMBRHelp::FindBoundingSet(std::move(candidates)); tmmbrOwner = TMMBRHelp::IsOwner(bounding, ssrc_); if (!tmmbrOwner) { // Did not enter bounding set, no meaning to send this request. diff --git a/webrtc/modules/rtp_rtcp/source/tmmbr_help.cc b/webrtc/modules/rtp_rtcp/source/tmmbr_help.cc index 5aa8f945aa..7c29ed8776 100644 --- a/webrtc/modules/rtp_rtcp/source/tmmbr_help.cc +++ b/webrtc/modules/rtp_rtcp/source/tmmbr_help.cc @@ -12,10 +12,8 @@ #include #include -#include #include "webrtc/base/checks.h" -#include "webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h" namespace webrtc { void TMMBRSet::VerifyAndAllocateSet(uint32_t minimumSize) { @@ -52,57 +50,21 @@ void TMMBRSet::RemoveEntry(uint32_t sourceIdx) { erase(begin() + sourceIdx); } -TMMBRSet* TMMBRHelp::VerifyAndAllocateCandidateSet(uint32_t minimumSize) { - _candidateSet.VerifyAndAllocateSet(minimumSize); - return &_candidateSet; -} - -TMMBRSet* TMMBRHelp::CandidateSet() { - return &_candidateSet; -} - -std::vector TMMBRHelp::FindTMMBRBoundingSet() { - // Work on local variable, will be modified - TMMBRSet candidateSet; - candidateSet.VerifyAndAllocateSet(_candidateSet.capacity()); - - for (size_t i = 0; i < _candidateSet.size(); i++) { - if (_candidateSet.Tmmbr(i)) { - candidateSet.AddEntry(_candidateSet.Tmmbr(i), _candidateSet.PacketOH(i), - _candidateSet.Ssrc(i)); - } else { - // make sure this is zero if tmmbr = 0 - RTC_DCHECK_EQ(_candidateSet.PacketOH(i), 0u); - // Old code: - // _candidateSet.ptrPacketOHSet[i] = 0; - } +std::vector TMMBRHelp::FindBoundingSet( + std::vector candidates) { + // Filter out candidates with 0 bitrate. + for (auto it = candidates.begin(); it != candidates.end();) { + if (!it->bitrate_bps()) + it = candidates.erase(it); + else + ++it; } - // Number of set candidates - int32_t numSetCandidates = candidateSet.lengthOfSet(); - // Find bounding set - std::vector bounding; - if (numSetCandidates > 0) { - FindBoundingSet(std::move(candidateSet), &bounding); - size_t numBoundingSet = bounding.size(); - RTC_DCHECK_GE(numBoundingSet, 1u); - RTC_DCHECK_LE(numBoundingSet, _candidateSet.size()); - } - return bounding; -} + if (candidates.size() <= 1) + return candidates; -void TMMBRHelp::FindBoundingSet(std::vector candidates, - std::vector* bounding_set) { - RTC_DCHECK(bounding_set); - RTC_DCHECK(!candidates.empty()); size_t num_candidates = candidates.size(); - if (num_candidates == 1) { - RTC_DCHECK(candidates[0].bitrate_bps()); - *bounding_set = std::move(candidates); - return; - } - // 1. Sort by increasing packet overhead. std::sort(candidates.begin(), candidates.end(), [](const rtcp::TmmbItem& lhs, const rtcp::TmmbItem& rhs) { @@ -130,7 +92,7 @@ void TMMBRHelp::FindBoundingSet(std::vector candidates, it = next_it; } - // 3. Select and remove tuple with lowest tmmbr. + // 3. Select and remove tuple with lowest bitrate. // (If more than 1, choose the one with highest overhead). auto min_bitrate_it = candidates.end(); for (auto it = candidates.begin(); it != candidates.end(); ++it) { @@ -148,22 +110,22 @@ void TMMBRHelp::FindBoundingSet(std::vector candidates, } } - bounding_set->clear(); - bounding_set->reserve(num_candidates); + std::vector bounding_set; + bounding_set.reserve(num_candidates); std::vector intersection(num_candidates); std::vector max_packet_rate(num_candidates); // First member of selected list. - bounding_set->push_back(*min_bitrate_it); + bounding_set.push_back(*min_bitrate_it); intersection[0] = 0; // Calculate its maximum packet rate (where its line crosses x-axis). - uint16_t packet_overhead = bounding_set->back().packet_overhead(); + uint16_t packet_overhead = bounding_set.back().packet_overhead(); if (packet_overhead == 0) { // Avoid division by zero. max_packet_rate[0] = std::numeric_limits::max(); } else { - max_packet_rate[0] = bounding_set->back().bitrate_bps() / - static_cast(packet_overhead); + max_packet_rate[0] = + bounding_set.back().bitrate_bps() / static_cast(packet_overhead); } // Remove from candidate list. min_bitrate_it->set_bitrate_bps(0); @@ -173,7 +135,7 @@ void TMMBRHelp::FindBoundingSet(std::vector candidates, // (next tuple must be steeper). for (auto it = candidates.begin(); it != candidates.end(); ++it) { if (it->bitrate_bps() && - it->packet_overhead() < bounding_set->front().packet_overhead()) { + it->packet_overhead() < bounding_set.front().packet_overhead()) { it->set_bitrate_bps(0); --num_candidates; } @@ -196,30 +158,30 @@ void TMMBRHelp::FindBoundingSet(std::vector candidates, // 6. Calculate packet rate and intersection of the current // line with line of last tuple in selected list. RTC_DCHECK_NE(cur_candidate.packet_overhead(), - bounding_set->back().packet_overhead()); + bounding_set.back().packet_overhead()); float packet_rate = static_cast(cur_candidate.bitrate_bps() - - bounding_set->back().bitrate_bps()) / + bounding_set.back().bitrate_bps()) / (cur_candidate.packet_overhead() - - bounding_set->back().packet_overhead()); + bounding_set.back().packet_overhead()); // 7. If the packet rate is equal or lower than intersection of // last tuple in selected list, // remove last tuple in selected list & go back to step 6. - if (packet_rate <= intersection[bounding_set->size() - 1]) { + if (packet_rate <= intersection[bounding_set.size() - 1]) { // Remove last tuple and goto step 6. - bounding_set->pop_back(); + bounding_set.pop_back(); get_new_candidate = false; } else { // 8. If packet rate is lower than maximum packet rate of // last tuple in selected list, add current tuple to selected // list. - if (packet_rate < max_packet_rate[bounding_set->size() - 1]) { - bounding_set->push_back(cur_candidate); - intersection[bounding_set->size() - 1] = packet_rate; - uint16_t packet_overhead = bounding_set->back().packet_overhead(); + if (packet_rate < max_packet_rate[bounding_set.size() - 1]) { + bounding_set.push_back(cur_candidate); + intersection[bounding_set.size() - 1] = packet_rate; + uint16_t packet_overhead = bounding_set.back().packet_overhead(); RTC_DCHECK_NE(packet_overhead, 0); - max_packet_rate[bounding_set->size() - 1] = - bounding_set->back().bitrate_bps() / + max_packet_rate[bounding_set.size() - 1] = + bounding_set.back().bitrate_bps() / static_cast(packet_overhead); } --num_candidates; @@ -228,6 +190,8 @@ void TMMBRHelp::FindBoundingSet(std::vector candidates, // 9. Go back to step 5 if any tuple remains in candidate list. } + RTC_DCHECK(!bounding_set.empty()); + return bounding_set; } bool TMMBRHelp::IsOwner(const std::vector& bounding, @@ -240,21 +204,13 @@ bool TMMBRHelp::IsOwner(const std::vector& bounding, return false; } -bool TMMBRHelp::CalcMinBitRate(uint32_t* minBitrateKbit) const { - if (_candidateSet.size() == 0) { - // Empty bounding set. - return false; - } - *minBitrateKbit = std::numeric_limits::max(); - - for (size_t i = 0; i < _candidateSet.lengthOfSet(); ++i) { - uint32_t curNetBitRateKbit = _candidateSet.Tmmbr(i); - if (curNetBitRateKbit < MIN_VIDEO_BW_MANAGEMENT_BITRATE) { - curNetBitRateKbit = MIN_VIDEO_BW_MANAGEMENT_BITRATE; - } - *minBitrateKbit = curNetBitRateKbit < *minBitrateKbit ? curNetBitRateKbit - : *minBitrateKbit; - } - return true; +uint64_t TMMBRHelp::CalcMinBitrateBps( + const std::vector& candidates) { + RTC_DCHECK(!candidates.empty()); + uint64_t min_bitrate_bps = std::numeric_limits::max(); + for (const rtcp::TmmbItem& item : candidates) + if (item.bitrate_bps() < min_bitrate_bps) + min_bitrate_bps = item.bitrate_bps(); + return min_bitrate_bps; } } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/tmmbr_help.h b/webrtc/modules/rtp_rtcp/source/tmmbr_help.h index ecae946f97..e08856e40c 100644 --- a/webrtc/modules/rtp_rtcp/source/tmmbr_help.h +++ b/webrtc/modules/rtp_rtcp/source/tmmbr_help.h @@ -41,21 +41,14 @@ class TMMBRSet : public std::vector { class TMMBRHelp { public: - TMMBRSet* CandidateSet(); - - TMMBRSet* VerifyAndAllocateCandidateSet(const uint32_t minimumSize); - std::vector FindTMMBRBoundingSet(); + static std::vector FindBoundingSet( + std::vector candidates); static bool IsOwner(const std::vector& bounding, uint32_t ssrc); - bool CalcMinBitRate(uint32_t* minBitrateKbit) const; - - static void FindBoundingSet(std::vector candidates, - std::vector* bounding_set); - - private: - TMMBRSet _candidateSet; + static uint64_t CalcMinBitrateBps( + const std::vector& candidates); }; } // namespace webrtc