admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Simplify setting/unsetting REMB in RtcpSender

Browse Source 
follow up of https://webrtc-review.googlesource.com/c/src/+/7983

Bug: None
Change-Id: I408c21408478d801a769e2e9d5f2eb9408430a4b
Reviewed-on: https://webrtc-review.googlesource.com/12520
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Reviewed-by: Elad Alon <eladalon@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20359}
This commit is contained in:
Danil Chapovalov 2017-10-18 13:32:57 +02:00 committed by Commit Bot
parent 2d27fb5a33
commit f74d641fc1
5 changed files with 39 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
modules/rtp_rtcp/source/rtcp_sender.cc
View File

@ -154,7 +154,6 @@ RTCPSender::RTCPSender(
transport_(outgoing_transport), transport_(outgoing_transport),
using_nack_(false), using_nack_(false),
sending_(false), sending_(false),
remb_enabled_(false),
next_time_to_send_rtcp_(0), next_time_to_send_rtcp_(0),
timestamp_offset_(0), timestamp_offset_(0),
last_rtp_timestamp_(0), last_rtp_timestamp_(0),
@ -237,33 +236,23 @@ int32_t RTCPSender::SetSendingStatus(const FeedbackState& feedback_state,
return 0; return 0;
} }
bool RTCPSender::REMB() const { void RTCPSender::SetRemb(uint32_t bitrate, const std::vector<uint32_t>& ssrcs) {
rtc::CritScope lock(&critical_section_rtcp_sender_);
return remb_enabled_;
}
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,
const std::vector<uint32_t>& ssrcs) {
rtc::CritScope lock(&critical_section_rtcp_sender_); rtc::CritScope lock(&critical_section_rtcp_sender_);
remb_bitrate_ = bitrate; remb_bitrate_ = bitrate;
remb_ssrcs_ = ssrcs; remb_ssrcs_ = ssrcs;
if (remb_enabled_) SetFlag(kRtcpRemb, /*is_volatile=*/false);
SetFlag(kRtcpRemb, false);
// Send a REMB immediately if we have a new REMB. The frequency of REMBs is // Send a REMB immediately if we have a new REMB. The frequency of REMBs is
// throttled by the caller. // throttled by the caller.
next_time_to_send_rtcp_ = clock_->TimeInMilliseconds(); next_time_to_send_rtcp_ = clock_->TimeInMilliseconds();
} }
void RTCPSender::UnsetRemb() {
rtc::CritScope lock(&critical_section_rtcp_sender_);
// Stop sending REMB each report until it is reenabled and REMB data set.
ConsumeFlag(kRtcpRemb, /*forced=*/true);
}
bool RTCPSender::TMMBR() const { bool RTCPSender::TMMBR() const {
rtc::CritScope lock(&critical_section_rtcp_sender_); rtc::CritScope lock(&critical_section_rtcp_sender_);
return IsFlagPresent(RTCPPacketType::kRtcpTmmbr); return IsFlagPresent(RTCPPacketType::kRtcpTmmbr);

7
modules/rtp_rtcp/source/rtcp_sender.h
View File

@ -120,11 +120,9 @@ class RTCPSender {
int32_t nackSize = 0, int32_t nackSize = 0,
const uint16_t* nackList = 0); const uint16_t* nackList = 0);
bool REMB() const; void SetRemb(uint32_t bitrate, const std::vector<uint32_t>& ssrcs);
void SetREMBStatus(bool enable); void UnsetRemb();
void SetREMBData(uint32_t bitrate, const std::vector<uint32_t>& ssrcs);
bool TMMBR() const; bool TMMBR() const;
@ -199,7 +197,6 @@ class RTCPSender {
rtc::CriticalSection critical_section_rtcp_sender_; rtc::CriticalSection critical_section_rtcp_sender_;
bool using_nack_ RTC_GUARDED_BY(critical_section_rtcp_sender_); bool using_nack_ RTC_GUARDED_BY(critical_section_rtcp_sender_);
bool sending_ RTC_GUARDED_BY(critical_section_rtcp_sender_); bool sending_ RTC_GUARDED_BY(critical_section_rtcp_sender_);
bool remb_enabled_ RTC_GUARDED_BY(critical_section_rtcp_sender_);
int64_t next_time_to_send_rtcp_ RTC_GUARDED_BY(critical_section_rtcp_sender_); int64_t next_time_to_send_rtcp_ RTC_GUARDED_BY(critical_section_rtcp_sender_);

71
modules/rtp_rtcp/source/rtcp_sender_unittest.cc
View File

@ -497,44 +497,39 @@ TEST_F(RtcpSenderTest, SendNack) {
EXPECT_THAT(parser()->nack()->packet_ids(), ElementsAre(0, 1, 16)); EXPECT_THAT(parser()->nack()->packet_ids(), ElementsAre(0, 1, 16));
} }
TEST_F(RtcpSenderTest, RembStatus) { TEST_F(RtcpSenderTest, RembNotIncludedBeforeSet) {
rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
rtcp_sender_->SendRTCP(feedback_state(), kRtcpRr);
ASSERT_EQ(1, parser()->receiver_report()->num_packets());
EXPECT_EQ(0, parser()->remb()->num_packets());
}
TEST_F(RtcpSenderTest, RembNotIncludedAfterUnset) {
const uint64_t kBitrate = 261011; const uint64_t kBitrate = 261011;
const std::vector<uint32_t> kSsrcs = {kRemoteSsrc, kRemoteSsrc + 1}; const std::vector<uint32_t> kSsrcs = {kRemoteSsrc, kRemoteSsrc + 1};
rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize); rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
rtcp_sender_->SetRemb(kBitrate, kSsrcs);
EXPECT_FALSE(rtcp_sender_->REMB());
rtcp_sender_->SendRTCP(feedback_state(), kRtcpRr); rtcp_sender_->SendRTCP(feedback_state(), kRtcpRr);
ASSERT_EQ(1, parser()->receiver_report()->num_packets()); ASSERT_EQ(1, parser()->receiver_report()->num_packets());
EXPECT_EQ(0, parser()->remb()->num_packets()); EXPECT_EQ(1, parser()->remb()->num_packets());
rtcp_sender_->SetREMBStatus(true); // Turn off REMB. rtcp_sender no longer should send it.
EXPECT_TRUE(rtcp_sender_->REMB()); rtcp_sender_->UnsetRemb();
rtcp_sender_->SetREMBData(kBitrate, kSsrcs);
rtcp_sender_->SendRTCP(feedback_state(), kRtcpRr); rtcp_sender_->SendRTCP(feedback_state(), kRtcpRr);
ASSERT_EQ(2, parser()->receiver_report()->num_packets()); ASSERT_EQ(2, parser()->receiver_report()->num_packets());
EXPECT_EQ(1, parser()->remb()->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) { TEST_F(RtcpSenderTest, SendRemb) {
const uint64_t kBitrate = 261011; const uint64_t kBitrate = 261011;
std::vector<uint32_t> ssrcs; const std::vector<uint32_t> kSsrcs = {kRemoteSsrc, kRemoteSsrc + 1};
ssrcs.push_back(kRemoteSsrc);
ssrcs.push_back(kRemoteSsrc + 1);
rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize); rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
rtcp_sender_->SetREMBData(kBitrate, ssrcs); rtcp_sender_->SetRemb(kBitrate, kSsrcs);
EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpRemb));
rtcp_sender_->SendRTCP(feedback_state(), kRtcpRemb);
EXPECT_EQ(1, parser()->remb()->num_packets()); EXPECT_EQ(1, parser()->remb()->num_packets());
EXPECT_EQ(kSenderSsrc, parser()->remb()->sender_ssrc()); EXPECT_EQ(kSenderSsrc, parser()->remb()->sender_ssrc());
EXPECT_EQ(kBitrate, parser()->remb()->bitrate_bps()); EXPECT_EQ(kBitrate, parser()->remb()->bitrate_bps());
@ -542,32 +537,19 @@ TEST_F(RtcpSenderTest, SendRemb) {
ElementsAre(kRemoteSsrc, kRemoteSsrc + 1)); ElementsAre(kRemoteSsrc, kRemoteSsrc + 1));
} }
TEST_F(RtcpSenderTest, RembIncludedInCompoundPacketIfEnabled) { TEST_F(RtcpSenderTest, RembIncludedInEachCompoundPacketAfterSet) {
const int kBitrate = 261011; const int kBitrate = 261011;
std::vector<uint32_t> ssrcs; const std::vector<uint32_t> kSsrcs = {kRemoteSsrc, kRemoteSsrc + 1};
ssrcs.push_back(kRemoteSsrc);
rtcp_sender_->SetRTCPStatus(RtcpMode::kCompound); rtcp_sender_->SetRTCPStatus(RtcpMode::kCompound);
rtcp_sender_->SetREMBStatus(true); rtcp_sender_->SetRemb(kBitrate, kSsrcs);
EXPECT_TRUE(rtcp_sender_->REMB());
rtcp_sender_->SetREMBData(kBitrate, ssrcs); rtcp_sender_->SendRTCP(feedback_state(), kRtcpReport);
EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpReport));
EXPECT_EQ(1, parser()->remb()->num_packets()); EXPECT_EQ(1, parser()->remb()->num_packets());
// REMB should be included in each compound packet. // REMB should be included in each compound packet.
EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpReport)); rtcp_sender_->SendRTCP(feedback_state(), kRtcpReport);
EXPECT_EQ(2, parser()->remb()->num_packets()); EXPECT_EQ(2, parser()->remb()->num_packets());
} }
TEST_F(RtcpSenderTest, RembNotIncludedInCompoundPacketIfNotEnabled) {
const int kBitrate = 261011;
std::vector<uint32_t> ssrcs;
ssrcs.push_back(kRemoteSsrc);
rtcp_sender_->SetRTCPStatus(RtcpMode::kCompound);
rtcp_sender_->SetREMBData(kBitrate, ssrcs);
EXPECT_FALSE(rtcp_sender_->REMB());
EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpReport));
EXPECT_EQ(0, parser()->remb()->num_packets());
}
TEST_F(RtcpSenderTest, SendXrWithVoipMetric) { TEST_F(RtcpSenderTest, SendXrWithVoipMetric) {
rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize); rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
RTCPVoIPMetric metric; RTCPVoIPMetric metric;
@ -757,7 +739,7 @@ TEST_F(RtcpSenderTest, SendCompoundPliRemb) {
std::vector<uint32_t> ssrcs; std::vector<uint32_t> ssrcs;
ssrcs.push_back(kRemoteSsrc); ssrcs.push_back(kRemoteSsrc);
rtcp_sender_->SetRTCPStatus(RtcpMode::kCompound); rtcp_sender_->SetRTCPStatus(RtcpMode::kCompound);
rtcp_sender_->SetREMBData(kBitrate, ssrcs); rtcp_sender_->SetRemb(kBitrate, ssrcs);
std::set<RTCPPacketType> packet_types; std::set<RTCPPacketType> packet_types;
packet_types.insert(kRtcpRemb); packet_types.insert(kRtcpRemb);
packet_types.insert(kRtcpPli); packet_types.insert(kRtcpPli);
@ -766,7 +748,6 @@ TEST_F(RtcpSenderTest, SendCompoundPliRemb) {
EXPECT_EQ(1, parser()->pli()->num_packets()); EXPECT_EQ(1, parser()->pli()->num_packets());
} }
// This test is written to verify that BYE is always the last packet // This test is written to verify that BYE is always the last packet
// type in a RTCP compoud packet. The rtcp_sender_ is recreated with // type in a RTCP compoud packet. The rtcp_sender_ is recreated with
// mock_transport, which is used to check for whether BYE at the end // mock_transport, which is used to check for whether BYE at the end

5
modules/rtp_rtcp/source/rtp_rtcp_impl.cc
View File

@ -631,12 +631,11 @@ int32_t ModuleRtpRtcpImpl::RemoteRTCPStat(
// (REMB) Receiver Estimated Max Bitrate. // (REMB) Receiver Estimated Max Bitrate.
void ModuleRtpRtcpImpl::SetRemb(uint32_t bitrate_bps, void ModuleRtpRtcpImpl::SetRemb(uint32_t bitrate_bps,
const std::vector<uint32_t>& ssrcs) { const std::vector<uint32_t>& ssrcs) {
rtcp_sender_.SetREMBStatus(true); rtcp_sender_.SetRemb(bitrate_bps, ssrcs);
rtcp_sender_.SetREMBData(bitrate_bps, ssrcs);
} }
void ModuleRtpRtcpImpl::UnsetRemb() { void ModuleRtpRtcpImpl::UnsetRemb() {
rtcp_sender_.SetREMBStatus(false); rtcp_sender_.UnsetRemb();
} }
int32_t ModuleRtpRtcpImpl::RegisterSendRtpHeaderExtension( int32_t ModuleRtpRtcpImpl::RegisterSendRtpHeaderExtension(

3
video/video_send_stream_tests.cc
View File

@ -1232,8 +1232,7 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) {
rtcp_sender.SetRTCPStatus(RtcpMode::kReducedSize); rtcp_sender.SetRTCPStatus(RtcpMode::kReducedSize);
rtcp_sender.SetRemoteSSRC(kVideoSendSsrcs[0]); rtcp_sender.SetRemoteSSRC(kVideoSendSsrcs[0]);
if (remb_value > 0) { if (remb_value > 0) {
rtcp_sender.SetREMBStatus(true); rtcp_sender.SetRemb(remb_value, std::vector<uint32_t>());
rtcp_sender.SetREMBData(remb_value, std::vector<uint32_t>());
} }
RTCPSender::FeedbackState feedback_state; RTCPSender::FeedbackState feedback_state;
EXPECT_EQ(0, rtcp_sender.SendRTCP(feedback_state, kRtcpRr)); EXPECT_EQ(0, rtcp_sender.SendRTCP(feedback_state, kRtcpRr));