From dbcf5d39188f24adda82d3d79235da9ff2a09e4e Mon Sep 17 00:00:00 2001 From: Tomas Gunnarsson Date: Fri, 23 Apr 2021 20:31:08 +0200 Subject: [PATCH] Change return type of SetSendingStatus to be void. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The eventual implementation of changing the status will be async so the return value isn't that useful and was in fact only being used to log a warning if an error occured. This change is to facilitate upcoming changes related to media engine. Bug: webrtc:11993 Change-Id: Ia7f85a9ea18b2648b511fa356918cf32a201461f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215975 Reviewed-by: Erik Språng Commit-Queue: Tommi Cr-Commit-Position: refs/heads/master@{#33825} --- modules/rtp_rtcp/source/rtcp_sender.cc | 21 ++++++++++--------- modules/rtp_rtcp/source/rtcp_sender.h | 4 ++-- .../rtp_rtcp/source/rtcp_sender_unittest.cc | 12 +++++------ modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 4 +--- modules/rtp_rtcp/source/rtp_rtcp_impl2.cc | 4 +--- 5 files changed, 21 insertions(+), 24 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc index 8b519b5c7d..ba63fd036f 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -187,8 +187,8 @@ bool RTCPSender::Sending() const { return sending_; } -int32_t RTCPSender::SetSendingStatus(const FeedbackState& feedback_state, - bool sending) { +void RTCPSender::SetSendingStatus(const FeedbackState& feedback_state, + bool sending) { bool sendRTCPBye = false; { MutexLock lock(&mutex_rtcp_sender_); @@ -201,9 +201,11 @@ int32_t RTCPSender::SetSendingStatus(const FeedbackState& feedback_state, } sending_ = sending; } - if (sendRTCPBye) - return SendRTCP(feedback_state, kRtcpBye); - return 0; + if (sendRTCPBye) { + if (SendRTCP(feedback_state, kRtcpBye) != 0) { + RTC_LOG(LS_WARNING) << "Failed to send RTCP BYE"; + } + } } int32_t RTCPSender::SendLossNotification(const FeedbackState& feedback_state, @@ -213,11 +215,10 @@ int32_t RTCPSender::SendLossNotification(const FeedbackState& feedback_state, bool buffering_allowed) { int32_t error_code = -1; auto callback = [&](rtc::ArrayView packet) { - if (transport_->SendRtcp(packet.data(), packet.size())) { - error_code = 0; - if (event_log_) { - event_log_->Log(std::make_unique(packet)); - } + transport_->SendRtcp(packet.data(), packet.size()); + error_code = 0; + if (event_log_) { + event_log_->Log(std::make_unique(packet)); } }; absl::optional sender; diff --git a/modules/rtp_rtcp/source/rtcp_sender.h b/modules/rtp_rtcp/source/rtcp_sender.h index 463666a22a..aab2c9051f 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.h +++ b/modules/rtp_rtcp/source/rtcp_sender.h @@ -75,8 +75,8 @@ class RTCPSender final { void SetRTCPStatus(RtcpMode method) RTC_LOCKS_EXCLUDED(mutex_rtcp_sender_); bool Sending() const RTC_LOCKS_EXCLUDED(mutex_rtcp_sender_); - int32_t SetSendingStatus(const FeedbackState& feedback_state, - bool enabled) + void SetSendingStatus(const FeedbackState& feedback_state, + bool enabled) RTC_LOCKS_EXCLUDED(mutex_rtcp_sender_); // combine the functions int32_t SetNackStatus(bool enable) RTC_LOCKS_EXCLUDED(mutex_rtcp_sender_); diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index 2c0bb2e2c4..90523a7770 100644 --- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -139,7 +139,7 @@ TEST_F(RtcpSenderTest, SetRtcpStatus) { TEST_F(RtcpSenderTest, SetSendingStatus) { auto rtcp_sender = CreateRtcpSender(GetDefaultConfig()); EXPECT_FALSE(rtcp_sender->Sending()); - EXPECT_EQ(0, rtcp_sender->SetSendingStatus(feedback_state(), true)); + rtcp_sender->SetSendingStatus(feedback_state(), true); EXPECT_TRUE(rtcp_sender->Sending()); } @@ -315,8 +315,8 @@ TEST_F(RtcpSenderTest, SendBye) { TEST_F(RtcpSenderTest, StopSendingTriggersBye) { auto rtcp_sender = CreateRtcpSender(GetDefaultConfig()); rtcp_sender->SetRTCPStatus(RtcpMode::kReducedSize); - EXPECT_EQ(0, rtcp_sender->SetSendingStatus(feedback_state(), true)); - EXPECT_EQ(0, rtcp_sender->SetSendingStatus(feedback_state(), false)); + rtcp_sender->SetSendingStatus(feedback_state(), true); + rtcp_sender->SetSendingStatus(feedback_state(), false); EXPECT_EQ(1, parser()->bye()->num_packets()); EXPECT_EQ(kSenderSsrc, parser()->bye()->sender_ssrc()); } @@ -513,7 +513,7 @@ TEST_F(RtcpSenderTest, SendXrWithRrtr) { config.non_sender_rtt_measurement = true; auto rtcp_sender = CreateRtcpSender(config); rtcp_sender->SetRTCPStatus(RtcpMode::kCompound); - EXPECT_EQ(0, rtcp_sender->SetSendingStatus(feedback_state(), false)); + rtcp_sender->SetSendingStatus(feedback_state(), false); NtpTime ntp = TimeMicrosToNtp(clock_.TimeInMicroseconds()); EXPECT_EQ(0, rtcp_sender->SendRTCP(feedback_state(), kRtcpReport)); EXPECT_EQ(1, parser()->xr()->num_packets()); @@ -528,7 +528,7 @@ TEST_F(RtcpSenderTest, TestNoXrRrtrSentIfSending) { config.non_sender_rtt_measurement = true; auto rtcp_sender = CreateRtcpSender(config); rtcp_sender->SetRTCPStatus(RtcpMode::kCompound); - EXPECT_EQ(0, rtcp_sender->SetSendingStatus(feedback_state(), true)); + rtcp_sender->SetSendingStatus(feedback_state(), true); EXPECT_EQ(0, rtcp_sender->SendRTCP(feedback_state(), kRtcpReport)); EXPECT_EQ(0, parser()->xr()->num_packets()); } @@ -538,7 +538,7 @@ TEST_F(RtcpSenderTest, TestNoXrRrtrSentIfNotEnabled) { config.non_sender_rtt_measurement = false; auto rtcp_sender = CreateRtcpSender(config); rtcp_sender->SetRTCPStatus(RtcpMode::kCompound); - EXPECT_EQ(0, rtcp_sender->SetSendingStatus(feedback_state(), false)); + rtcp_sender->SetSendingStatus(feedback_state(), false); EXPECT_EQ(0, rtcp_sender->SendRTCP(feedback_state(), kRtcpReport)); EXPECT_EQ(0, parser()->xr()->num_packets()); } diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index fa4af1dedc..5a79f55d33 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -336,9 +336,7 @@ RTCPSender::FeedbackState ModuleRtpRtcpImpl::GetFeedbackState() { int32_t ModuleRtpRtcpImpl::SetSendingStatus(const bool sending) { if (rtcp_sender_.Sending() != sending) { // Sends RTCP BYE when going from true to false - if (rtcp_sender_.SetSendingStatus(GetFeedbackState(), sending) != 0) { - RTC_LOG(LS_WARNING) << "Failed to send RTCP BYE"; - } + rtcp_sender_.SetSendingStatus(GetFeedbackState(), sending); } return 0; } diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index 78ccf9907f..e526bac659 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -286,9 +286,7 @@ RTCPSender::FeedbackState ModuleRtpRtcpImpl2::GetFeedbackState() { int32_t ModuleRtpRtcpImpl2::SetSendingStatus(const bool sending) { if (rtcp_sender_.Sending() != sending) { // Sends RTCP BYE when going from true to false - if (rtcp_sender_.SetSendingStatus(GetFeedbackState(), sending) != 0) { - RTC_LOG(LS_WARNING) << "Failed to send RTCP BYE"; - } + rtcp_sender_.SetSendingStatus(GetFeedbackState(), sending); } return 0; }