From ba6aa90c0417ebe30bd674f34c5180c74932f3df Mon Sep 17 00:00:00 2001 From: danilchap Date: Tue, 18 Apr 2017 06:57:02 -0700 Subject: [PATCH] Ensure SetREMBStatus(false) disables sending REMB support CL for upcoming https://codereview.webrtc.org/2789843002/ BUG=webrtc:7135 Review-Url: https://codereview.webrtc.org/2813693003 Cr-Commit-Position: refs/heads/master@{#17741} --- webrtc/modules/rtp_rtcp/source/rtcp_sender.cc | 4 ++++ .../rtp_rtcp/source/rtcp_sender_unittest.cc | 22 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc index e3cf65ef08..97cf8caa92 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc @@ -244,6 +244,10 @@ bool RTCPSender::REMB() const { void RTCPSender::SetREMBStatus(bool enable) { rtc::CritScope lock(&critical_section_rtcp_sender_); remb_enabled_ = enable; + if (!enable) { + // Stop sending remb each report until it is reenabled and remb data set. + ConsumeFlag(kRtcpRemb, true); + } } void RTCPSender::SetREMBData(uint32_t bitrate, diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index c79cbb7483..1d1877f793 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -498,11 +498,33 @@ TEST_F(RtcpSenderTest, SendNack) { } TEST_F(RtcpSenderTest, RembStatus) { + const uint64_t kBitrate = 261011; + const std::vector kSsrcs = {kRemoteSsrc, kRemoteSsrc + 1}; + rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize); + EXPECT_FALSE(rtcp_sender_->REMB()); + rtcp_sender_->SendRTCP(feedback_state(), kRtcpRr); + ASSERT_EQ(1, parser()->receiver_report()->num_packets()); + EXPECT_EQ(0, parser()->remb()->num_packets()); + rtcp_sender_->SetREMBStatus(true); EXPECT_TRUE(rtcp_sender_->REMB()); + rtcp_sender_->SetREMBData(kBitrate, kSsrcs); + rtcp_sender_->SendRTCP(feedback_state(), kRtcpRr); + ASSERT_EQ(2, parser()->receiver_report()->num_packets()); + EXPECT_EQ(1, parser()->remb()->num_packets()); + + // Sending another report sends remb again, even if no new remb data was set. + rtcp_sender_->SendRTCP(feedback_state(), kRtcpRr); + ASSERT_EQ(3, parser()->receiver_report()->num_packets()); + EXPECT_EQ(2, parser()->remb()->num_packets()); + + // Turn off remb. rtcp_sender no longer should send it. rtcp_sender_->SetREMBStatus(false); EXPECT_FALSE(rtcp_sender_->REMB()); + rtcp_sender_->SendRTCP(feedback_state(), kRtcpRr); + ASSERT_EQ(4, parser()->receiver_report()->num_packets()); + EXPECT_EQ(2, parser()->remb()->num_packets()); } TEST_F(RtcpSenderTest, SendRemb) {