From 853ecb21f77a4758e465b889f645f89e955c539a Mon Sep 17 00:00:00 2001 From: danilchap Date: Mon, 22 Aug 2016 08:26:15 -0700 Subject: [PATCH] Style cleanup in UpdateTmmbr: function names style updated, unused return type removed. Comment style fixed, redundant comments removed. pass-by-pointer parameter changed to pass-by-value because can't be nullptr any more. NOTRY=true BUG=webrtc:5565 Review-Url: https://codereview.webrtc.org/2258523005 Cr-Commit-Position: refs/heads/master@{#13848} --- .../modules/rtp_rtcp/source/rtcp_receiver.cc | 28 +++++++------------ .../modules/rtp_rtcp/source/rtcp_receiver.h | 5 ++-- .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 12 ++++---- webrtc/modules/rtp_rtcp/source/rtcp_sender.cc | 9 ++---- webrtc/modules/rtp_rtcp/source/rtcp_sender.h | 2 +- .../rtp_rtcp/source/rtcp_sender_unittest.cc | 4 +-- .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 9 +++--- .../modules/rtp_rtcp/source/rtp_rtcp_impl.h | 2 +- 8 files changed, 28 insertions(+), 43 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc index d57432cea4..28c6867a34 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -1210,28 +1210,20 @@ void RTCPReceiver::HandleTransportFeedback( rtcp_parser->Iterate(); } -int32_t RTCPReceiver::UpdateTMMBR() { - // Find bounding set +void RTCPReceiver::UpdateTmmbr() { + // Find bounding set. std::vector bounding = - TMMBRHelp::FindBoundingSet(TMMBRReceived()); - // Set bounding set - // Inform remote clients about the new bandwidth - // inform the remote client - _rtpRtcp.SetTMMBN(&bounding); + TMMBRHelp::FindBoundingSet(TmmbrReceived()); - // might trigger a TMMBN - if (bounding.empty()) { - // owner of max bitrate request has timed out - // empty bounding set has been sent - return 0; - } - // We have a new bandwidth estimate on this channel. - if (_cbRtcpBandwidthObserver) { + if (!bounding.empty() && _cbRtcpBandwidthObserver) { + // We have a new bandwidth estimate on this channel. uint64_t bitrate_bps = TMMBRHelp::CalcMinBitrateBps(bounding); if (bitrate_bps <= std::numeric_limits::max()) _cbRtcpBandwidthObserver->OnReceivedEstimatedBitrate(bitrate_bps); } - return 0; + + // Set bounding set: inform remote clients about the new bandwidth. + _rtpRtcp.SetTmmbn(std::move(bounding)); } void RTCPReceiver::RegisterRtcpStatisticsCallback( @@ -1252,7 +1244,7 @@ void RTCPReceiver::TriggerCallbacksFromRTCPPacket( // to OnNetworkChanged. if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpTmmbr) { // Might trigger a OnReceivedBandwidthEstimateUpdate. - UpdateTMMBR(); + UpdateTmmbr(); } uint32_t local_ssrc; std::set registered_ssrcs; @@ -1366,7 +1358,7 @@ int32_t RTCPReceiver::CNAME(uint32_t remoteSSRC, return 0; } -std::vector RTCPReceiver::TMMBRReceived() const { +std::vector RTCPReceiver::TmmbrReceived() const { rtc::CritScope lock(&_criticalSectionRTCPReceiver); std::vector candidates; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h index 072eb08826..747a75e87b 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h @@ -88,14 +88,13 @@ class RTCPReceiver { // returns true once until a new RR is received. bool RtcpRrSequenceNumberTimeout(int64_t rtcp_interval_ms); - // Get TMMBR - std::vector TMMBRReceived() const; + std::vector TmmbrReceived() const; bool UpdateRTCPReceiveInformationTimers(); std::vector BoundingSet(bool* tmmbr_owner); - int32_t UpdateTMMBR(); + void UpdateTmmbr(); void RegisterRtcpStatisticsCallback(RtcpStatisticsCallback* callback); RtcpStatisticsCallback* GetRtcpStatisticsCallback(); diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 6a2cb22b22..ec7c30e508 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(0u, rtcp_receiver_->TMMBRReceived().size()); + EXPECT_EQ(0u, rtcp_receiver_->TmmbrReceived().size()); } TEST_F(RtcpReceiverTest, TmmbrPacketAccepted) { @@ -1055,7 +1055,7 @@ TEST_F(RtcpReceiverTest, TmmbrPacketAccepted) { rtc::Buffer packet = compound.Build(); EXPECT_EQ(0, InjectRtcpPacket(packet.data(), packet.size())); - std::vector candidate_set = rtcp_receiver_->TMMBRReceived(); + 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()); @@ -1081,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(0u, rtcp_receiver_->TMMBRReceived().size()); + EXPECT_EQ(0u, rtcp_receiver_->TmmbrReceived().size()); } TEST_F(RtcpReceiverTest, TmmbrPacketZeroRateIgnored) { @@ -1103,7 +1103,7 @@ TEST_F(RtcpReceiverTest, TmmbrPacketZeroRateIgnored) { rtc::Buffer packet = compound.Build(); EXPECT_EQ(0, InjectRtcpPacket(packet.data(), packet.size())); - EXPECT_EQ(0u, rtcp_receiver_->TMMBRReceived().size()); + EXPECT_EQ(0u, rtcp_receiver_->TmmbrReceived().size()); } TEST_F(RtcpReceiverTest, TmmbrThreeConstraintsTimeOut) { @@ -1131,13 +1131,13 @@ TEST_F(RtcpReceiverTest, TmmbrThreeConstraintsTimeOut) { system_clock_.AdvanceTimeMilliseconds(5000); } // It is now starttime + 15. - std::vector candidate_set = rtcp_receiver_->TMMBRReceived(); + 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); - candidate_set = rtcp_receiver_->TMMBRReceived(); + candidate_set = rtcp_receiver_->TmmbrReceived(); EXPECT_EQ(2u, candidate_set.size()); EXPECT_EQ(kSenderSsrc + 1, candidate_set[0].ssrc()); } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc index a66c686d0e..7762e66bf2 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc @@ -982,14 +982,9 @@ bool RTCPSender::RtcpXrReceiverReferenceTime() const { return xr_send_receiver_reference_time_enabled_; } -// no callbacks allowed inside this function -void RTCPSender::SetTMMBN(const std::vector* bounding_set) { +void RTCPSender::SetTmmbn(std::vector bounding_set) { rtc::CritScope lock(&critical_section_rtcp_sender_); - if (bounding_set) { - tmmbn_to_send_ = *bounding_set; - } else { - tmmbn_to_send_.clear(); - } + tmmbn_to_send_ = std::move(bounding_set); SetFlag(kRtcpTmmbn, true); } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h index b9794918c9..1e88ce897d 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h @@ -135,7 +135,7 @@ class RTCPSender { void SetMaxPayloadLength(size_t max_payload_length); - void SetTMMBN(const std::vector* boundingSet); + void SetTmmbn(std::vector bounding_set); int32_t SetApplicationSpecificData(uint8_t subType, uint32_t name, diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index 5d230dc6f1..381908d5fe 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -738,7 +738,7 @@ TEST_F(RtcpSenderTest, SendTmmbn) { const uint32_t kSourceSsrc = 12345; const rtcp::TmmbItem tmmbn(kSourceSsrc, kBitrateKbps * 1000, kPacketOh); bounding_set.push_back(tmmbn); - rtcp_sender_->SetTMMBN(&bounding_set); + rtcp_sender_->SetTmmbn(bounding_set); EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpSr)); EXPECT_EQ(1, parser()->sender_report()->num_packets()); @@ -760,7 +760,7 @@ TEST_F(RtcpSenderTest, SendsTmmbnIfSetAndEmpty) { rtcp_sender_->SetRTCPStatus(RtcpMode::kCompound); rtcp_sender_->SetSendingStatus(feedback_state(), true); std::vector bounding_set; - rtcp_sender_->SetTMMBN(&bounding_set); + rtcp_sender_->SetTmmbn(bounding_set); EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpSr)); EXPECT_EQ(1, parser()->sender_report()->num_packets()); EXPECT_EQ(1, parser()->tmmbn()->num_packets()); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index e0b25f0f40..bf151d27ab 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -198,8 +198,8 @@ void ModuleRtpRtcpImpl::Process() { rtcp_sender_.SendRTCP(GetFeedbackState(), kRtcpReport); if (UpdateRTCPReceiveInformationTimers()) { - // A receiver has timed out - rtcp_receiver_.UpdateTMMBR(); + // A receiver has timed out. + rtcp_receiver_.UpdateTmmbr(); } } @@ -661,9 +661,8 @@ void ModuleRtpRtcpImpl::SetTMMBRStatus(const bool enable) { rtcp_sender_.SetTMMBRStatus(enable); } -void ModuleRtpRtcpImpl::SetTMMBN( - const std::vector* bounding_set) { - rtcp_sender_.SetTMMBN(bounding_set); +void ModuleRtpRtcpImpl::SetTmmbn(std::vector bounding_set) { + rtcp_sender_.SetTmmbn(std::move(bounding_set)); } // Returns the currently configured retransmission mode. diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 4e69993f22..f8fb21130c 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -205,7 +205,7 @@ class ModuleRtpRtcpImpl : public RtpRtcp { void SetTMMBRStatus(bool enable) override; - void SetTMMBN(const std::vector* bounding_set); + void SetTmmbn(std::vector bounding_set); uint16_t MaxPayloadLength() const override;