Allow parsing empty RTCP TargetBitrate messages, but stop sending them.
Also, add ToString() convenience method to the target bitrate struct. Super useful when doing printf debugging :)
BUG=webrtc:7858
Review-Url: https://codereview.webrtc.org/2947633003
Cr-Original-Commit-Position: refs/heads/master@{#18717}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: d0fc37a8841ed70b5338ce80ac9c3e8ac139a310
diff --git a/common_types.cc b/common_types.cc
index 0511f0d..70e5d2e 100644
--- a/common_types.cc
+++ b/common_types.cc
@@ -209,4 +209,53 @@
return sum;
}
+std::string BitrateAllocation::ToString() const {
+ if (sum_ == 0)
+ return "BitrateAllocation [ [] ]";
+
+ // TODO(sprang): Replace this stringstream with something cheaper.
+ std::ostringstream oss;
+ oss << "BitrateAllocation [";
+ uint32_t spatial_cumulator = 0;
+ for (int si = 0; si < kMaxSpatialLayers; ++si) {
+ RTC_DCHECK_LE(spatial_cumulator, sum_);
+ if (spatial_cumulator == sum_)
+ break;
+
+ const uint32_t layer_sum = GetSpatialLayerSum(si);
+ if (layer_sum == sum_) {
+ oss << " [";
+ } else {
+ if (si > 0)
+ oss << ",";
+ oss << std::endl << " [";
+ }
+ spatial_cumulator += layer_sum;
+
+ uint32_t temporal_cumulator = 0;
+ for (int ti = 0; ti < kMaxTemporalStreams; ++ti) {
+ RTC_DCHECK_LE(temporal_cumulator, layer_sum);
+ if (temporal_cumulator == layer_sum)
+ break;
+
+ if (ti > 0)
+ oss << ", ";
+
+ uint32_t bitrate = bitrates_[si][ti];
+ oss << bitrate;
+ temporal_cumulator += bitrate;
+ }
+ oss << "]";
+ }
+
+ RTC_DCHECK_EQ(spatial_cumulator, sum_);
+ oss << " ]";
+ return oss.str();
+}
+
+std::ostream& BitrateAllocation::operator<<(std::ostream& os) const {
+ os << ToString();
+ return os;
+}
+
} // namespace webrtc
diff --git a/common_types.h b/common_types.h
index b0a21d6..7b62dbc 100644
--- a/common_types.h
+++ b/common_types.h
@@ -646,6 +646,10 @@
return !(*this == other);
}
+ // Expensive, please use only in tests.
+ std::string ToString() const;
+ std::ostream& operator<<(std::ostream& os) const;
+
private:
uint32_t sum_;
uint32_t bitrates_[kMaxSpatialLayers][kMaxTemporalStreams];
diff --git a/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc b/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc
index 583083b..d559eef 100644
--- a/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc
+++ b/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc
@@ -204,9 +204,8 @@
void ExtendedReports::ParseTargetBitrateBlock(const uint8_t* block,
uint16_t block_length) {
- target_bitrate_ = rtc::Optional<TargetBitrate>(TargetBitrate());
- if (!target_bitrate_->Parse(block, block_length))
- target_bitrate_ = rtc::Optional<TargetBitrate>();
+ target_bitrate_.emplace();
+ target_bitrate_->Parse(block, block_length);
}
} // namespace rtcp
} // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtcp_packet/target_bitrate.cc b/modules/rtp_rtcp/source/rtcp_packet/target_bitrate.cc
index 9132642..1a00006 100644
--- a/modules/rtp_rtcp/source/rtcp_packet/target_bitrate.cc
+++ b/modules/rtp_rtcp/source/rtcp_packet/target_bitrate.cc
@@ -11,7 +11,6 @@
#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/target_bitrate.h"
#include "webrtc/base/checks.h"
-#include "webrtc/base/logging.h"
#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
namespace webrtc {
@@ -65,15 +64,7 @@
TargetBitrate::TargetBitrate() {}
TargetBitrate::~TargetBitrate() {}
-bool TargetBitrate::Parse(const uint8_t* block, uint16_t block_length) {
- if (block_length < 1) {
- LOG(LS_WARNING)
- << "Cannot parse TargetBitrate RTCP packet: Too little payload data ("
- << kTargetBitrateHeaderSizeBytes << " bytes needed, got "
- << block_length * 4 << ").";
- return false;
- }
-
+void TargetBitrate::Parse(const uint8_t* block, uint16_t block_length) {
// Validate block header (should already have been parsed and checked).
RTC_DCHECK_EQ(block[0], kBlockType);
RTC_DCHECK_EQ(block_length, ByteReader<uint16_t>::ReadBigEndian(&block[2]));
@@ -91,8 +82,6 @@
index += kBitrateItemSizeBytes;
AddTargetBitrate((layers >> 4) & 0x0F, layers & 0x0F, bitrate_kbps);
}
-
- return true;
}
void TargetBitrate::AddTargetBitrate(uint8_t spatial_layer,
diff --git a/modules/rtp_rtcp/source/rtcp_packet/target_bitrate.h b/modules/rtp_rtcp/source/rtcp_packet/target_bitrate.h
index d3f672a..d7c49a9 100644
--- a/modules/rtp_rtcp/source/rtcp_packet/target_bitrate.h
+++ b/modules/rtp_rtcp/source/rtcp_packet/target_bitrate.h
@@ -45,7 +45,7 @@
const std::vector<BitrateItem>& GetTargetBitrates() const;
- bool Parse(const uint8_t* block, uint16_t block_length);
+ void Parse(const uint8_t* block, uint16_t block_length);
size_t BlockLength() const;
diff --git a/modules/rtp_rtcp/source/rtcp_packet/target_bitrate_unittest.cc b/modules/rtp_rtcp/source/rtcp_packet/target_bitrate_unittest.cc
index 1c8f39d..2c5098d 100644
--- a/modules/rtp_rtcp/source/rtcp_packet/target_bitrate_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_packet/target_bitrate_unittest.cc
@@ -54,7 +54,7 @@
TEST(TargetBitrateTest, Parse) {
TargetBitrate target_bitrate;
- EXPECT_TRUE(target_bitrate.Parse(kPacket, kPacketLengthBlocks));
+ target_bitrate.Parse(kPacket, kPacketLengthBlocks);
CheckBitrateItems(target_bitrate.GetTargetBitrates());
}
@@ -86,4 +86,11 @@
EXPECT_EQ(0, memcmp(kPacket, buffer, sizeof(kPacket)));
}
+TEST(TargetBitrateTest, ParseNullBitratePacket) {
+ const uint8_t kNullPacket[] = {TargetBitrate::kBlockType, 0x00, 0x00, 0x00};
+ TargetBitrate target_bitrate;
+ target_bitrate.Parse(kNullPacket, 0);
+ EXPECT_TRUE(target_bitrate.GetTargetBitrates().empty());
+}
+
} // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc
index b5e0d9a..4a3235f 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.cc
+++ b/modules/rtp_rtcp/source/rtcp_receiver.cc
@@ -121,6 +121,7 @@
remote_ssrc_(0),
xr_rrtr_status_(false),
xr_rr_rtt_ms_(0),
+ oldest_tmmbr_info_ms_(0),
last_received_rr_ms_(0),
last_increased_sequence_number_ms_(0),
stats_callback_(nullptr),
diff --git a/video/payload_router.cc b/video/payload_router.cc
index 12ced89..e0e95ae 100644
--- a/video/payload_router.cc
+++ b/video/payload_router.cc
@@ -171,6 +171,10 @@
// Simulcast is in use, split the BitrateAllocation into one struct per
// rtp stream, moving over the temporal layer allocation.
for (size_t si = 0; si < rtp_modules_.size(); ++si) {
+ // Don't send empty TargetBitrate messages on streams not being relayed.
+ if (bitrate.GetSpatialLayerSum(si) == 0)
+ break;
+
BitrateAllocation layer_bitrate;
for (int tl = 0; tl < kMaxTemporalStreams; ++tl)
layer_bitrate.SetBitrate(0, tl, bitrate.GetBitrate(si, tl));
diff --git a/video/payload_router_unittest.cc b/video/payload_router_unittest.cc
index e316695..9f58ceb 100644
--- a/video/payload_router_unittest.cc
+++ b/video/payload_router_unittest.cc
@@ -178,6 +178,29 @@
payload_router.OnBitrateAllocationUpdated(bitrate);
}
+TEST(PayloadRouterTest, SimulcastTargetBitrateWithInactiveStream) {
+ // Set up two active rtp modules.
+ NiceMock<MockRtpRtcp> rtp_1;
+ NiceMock<MockRtpRtcp> rtp_2;
+ std::vector<RtpRtcp*> modules;
+ modules.push_back(&rtp_1);
+ modules.push_back(&rtp_2);
+ PayloadRouter payload_router(modules, 42);
+ payload_router.SetActive(true);
+
+ // Create bitrate allocation with bitrate only for the first stream.
+ BitrateAllocation bitrate;
+ bitrate.SetBitrate(0, 0, 10000);
+ bitrate.SetBitrate(0, 1, 20000);
+
+ // Expect only the first rtp module to be asked to send a TargetBitrate
+ // message. (No target bitrate with 0bps sent from the second one.)
+ EXPECT_CALL(rtp_1, SetVideoBitrateAllocation(bitrate)).Times(1);
+ EXPECT_CALL(rtp_2, SetVideoBitrateAllocation(_)).Times(0);
+
+ payload_router.OnBitrateAllocationUpdated(bitrate);
+}
+
TEST(PayloadRouterTest, SvcTargetBitrate) {
NiceMock<MockRtpRtcp> rtp_1;
std::vector<RtpRtcp*> modules;
