Add test for logging of large compound RTCP packets.
Bug: chromium:1134107
Change-Id: Ic6ce50d33700c05733747584ce45480660cf64c9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/188583
Reviewed-by: Elad Alon <eladalon@webrtc.org>
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32445}
diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder_unittest.cc b/logging/rtc_event_log/encoder/rtc_event_log_encoder_unittest.cc
index 009acbd..76740a4 100644
--- a/logging/rtc_event_log/encoder/rtc_event_log_encoder_unittest.cc
+++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder_unittest.cc
@@ -36,6 +36,7 @@
#include "logging/rtc_event_log/rtc_event_log_unittest_helper.h"
#include "modules/audio_coding/audio_network_adaptor/include/audio_network_adaptor_config.h"
#include "modules/remote_bitrate_estimator/include/bwe_defines.h"
+#include "modules/rtp_rtcp/source/rtcp_packet/bye.h"
#include "modules/rtp_rtcp/source/rtp_header_extensions.h"
#include "rtc_base/fake_clock.h"
#include "rtc_base/random.h"
@@ -1242,4 +1243,63 @@
/* Event count: */ ::testing::Values(1, 2, 10, 100),
/* Repeated fields: */ ::testing::Bool()));
+class RtcEventLogEncoderSimpleTest
+ : public ::testing::TestWithParam<RtcEventLog::EncodingType> {
+ protected:
+ RtcEventLogEncoderSimpleTest() : encoding_(GetParam()) {
+ switch (encoding_) {
+ case RtcEventLog::EncodingType::Legacy:
+ encoder_ = std::make_unique<RtcEventLogEncoderLegacy>();
+ break;
+ case RtcEventLog::EncodingType::NewFormat:
+ encoder_ = std::make_unique<RtcEventLogEncoderNewFormat>();
+ break;
+ }
+ }
+ ~RtcEventLogEncoderSimpleTest() override = default;
+
+ std::deque<std::unique_ptr<RtcEvent>> history_;
+ std::unique_ptr<RtcEventLogEncoder> encoder_;
+ ParsedRtcEventLog parsed_log_;
+ const RtcEventLog::EncodingType encoding_;
+};
+
+TEST_P(RtcEventLogEncoderSimpleTest, RtcEventLargeCompoundRtcpPacketIncoming) {
+ // Create a compound packet containing multiple Bye messages.
+ rtc::Buffer packet;
+ size_t index = 0;
+ for (int i = 0; i < 8; i++) {
+ rtcp::Bye bye;
+ std::string reason(255, 'a'); // Add some arbitrary data.
+ bye.SetReason(reason);
+ bye.SetSenderSsrc(0x12345678);
+ packet.SetSize(packet.size() + bye.BlockLength());
+ bool created =
+ bye.Create(packet.data(), &index, packet.capacity(), nullptr);
+ ASSERT_TRUE(created);
+ ASSERT_EQ(index, packet.size());
+ }
+
+ EXPECT_GT(packet.size(), static_cast<size_t>(IP_PACKET_SIZE));
+ auto event = std::make_unique<RtcEventRtcpPacketIncoming>(packet);
+ history_.push_back(event->Copy());
+ std::string encoded = encoder_->EncodeBatch(history_.begin(), history_.end());
+
+ ParsedRtcEventLog::ParseStatus status = parsed_log_.ParseString(encoded);
+ ASSERT_TRUE(status.ok()) << status.message();
+
+ const auto& incoming_rtcp_packets = parsed_log_.incoming_rtcp_packets();
+ ASSERT_EQ(incoming_rtcp_packets.size(), 1u);
+ ASSERT_EQ(incoming_rtcp_packets[0].rtcp.raw_data.size(), packet.size());
+ EXPECT_EQ(memcmp(incoming_rtcp_packets[0].rtcp.raw_data.data(), packet.data(),
+ packet.size()),
+ 0);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ LargeCompoundRtcp,
+ RtcEventLogEncoderSimpleTest,
+ ::testing::Values(RtcEventLog::EncodingType::Legacy,
+ RtcEventLog::EncodingType::NewFormat));
+
} // namespace webrtc
diff --git a/logging/rtc_event_log/logged_events.cc b/logging/rtc_event_log/logged_events.cc
index 3cba8ba..dd0a8aa 100644
--- a/logging/rtc_event_log/logged_events.cc
+++ b/logging/rtc_event_log/logged_events.cc
@@ -41,15 +41,17 @@
LoggedPacketInfo::~LoggedPacketInfo() {}
LoggedRtcpPacket::LoggedRtcpPacket(int64_t timestamp_us,
- const uint8_t* packet,
- size_t total_length)
- : timestamp_us(timestamp_us), raw_data(packet, packet + total_length) {}
+ const std::vector<uint8_t>& packet)
+ : timestamp_us(timestamp_us), raw_data(packet) {}
+
LoggedRtcpPacket::LoggedRtcpPacket(int64_t timestamp_us,
const std::string& packet)
: timestamp_us(timestamp_us), raw_data(packet.size()) {
memcpy(raw_data.data(), packet.data(), packet.size());
}
+
LoggedRtcpPacket::LoggedRtcpPacket(const LoggedRtcpPacket& rhs) = default;
+
LoggedRtcpPacket::~LoggedRtcpPacket() = default;
} // namespace webrtc
diff --git a/logging/rtc_event_log/logged_events.h b/logging/rtc_event_log/logged_events.h
index 4bd33f6..192f7cf 100644
--- a/logging/rtc_event_log/logged_events.h
+++ b/logging/rtc_event_log/logged_events.h
@@ -309,9 +309,7 @@
};
struct LoggedRtcpPacket {
- LoggedRtcpPacket(int64_t timestamp_us,
- const uint8_t* packet,
- size_t total_length);
+ LoggedRtcpPacket(int64_t timestamp_us, const std::vector<uint8_t>& packet);
LoggedRtcpPacket(int64_t timestamp_us, const std::string& packet);
LoggedRtcpPacket(const LoggedRtcpPacket&);
~LoggedRtcpPacket();
@@ -325,9 +323,8 @@
struct LoggedRtcpPacketIncoming {
LoggedRtcpPacketIncoming(int64_t timestamp_us,
- const uint8_t* packet,
- size_t total_length)
- : rtcp(timestamp_us, packet, total_length) {}
+ const std::vector<uint8_t>& packet)
+ : rtcp(timestamp_us, packet) {}
LoggedRtcpPacketIncoming(uint64_t timestamp_us, const std::string& packet)
: rtcp(timestamp_us, packet) {}
@@ -339,9 +336,8 @@
struct LoggedRtcpPacketOutgoing {
LoggedRtcpPacketOutgoing(int64_t timestamp_us,
- const uint8_t* packet,
- size_t total_length)
- : rtcp(timestamp_us, packet, total_length) {}
+ const std::vector<uint8_t>& packet)
+ : rtcp(timestamp_us, packet) {}
LoggedRtcpPacketOutgoing(uint64_t timestamp_us, const std::string& packet)
: rtcp(timestamp_us, packet) {}
diff --git a/logging/rtc_event_log/rtc_event_log_parser.cc b/logging/rtc_event_log/rtc_event_log_parser.cc
index 67b1c0d..a49af87 100644
--- a/logging/rtc_event_log/rtc_event_log_parser.cc
+++ b/logging/rtc_event_log/rtc_event_log_parser.cc
@@ -678,10 +678,8 @@
raw_packet_values[i])) {
continue;
}
- const size_t data_size = raw_packet_values[i].size();
- const uint8_t* data =
- reinterpret_cast<const uint8_t*>(raw_packet_values[i].data());
- rtcp_packets->emplace_back(1000 * timestamp_ms, data, data_size);
+ std::string data(raw_packet_values[i]);
+ rtcp_packets->emplace_back(1000 * timestamp_ms, data);
}
return ParsedRtcEventLog::ParseStatus::Success();
}
@@ -1093,8 +1091,7 @@
video_recv_configs_.clear();
video_send_configs_.clear();
- memset(last_incoming_rtcp_packet_, 0, IP_PACKET_SIZE);
- last_incoming_rtcp_packet_length_ = 0;
+ last_incoming_rtcp_packet_.clear();
first_timestamp_ = std::numeric_limits<int64_t>::max();
last_timestamp_ = std::numeric_limits<int64_t>::min();
@@ -1476,27 +1473,23 @@
}
case rtclog::Event::RTCP_EVENT: {
PacketDirection direction;
- uint8_t packet[IP_PACKET_SIZE];
- size_t total_length;
- auto status = GetRtcpPacket(event, &direction, packet, &total_length);
+ std::vector<uint8_t> packet;
+ auto status = GetRtcpPacket(event, &direction, &packet);
RTC_RETURN_IF_ERROR(status);
RTC_PARSE_CHECK_OR_RETURN(event.has_timestamp_us());
int64_t timestamp_us = event.timestamp_us();
- RTC_PARSE_CHECK_OR_RETURN_LE(total_length, IP_PACKET_SIZE);
if (direction == kIncomingPacket) {
// Currently incoming RTCP packets are logged twice, both for audio and
// video. Only act on one of them. Compare against the previous parsed
// incoming RTCP packet.
- if (total_length == last_incoming_rtcp_packet_length_ &&
- memcmp(last_incoming_rtcp_packet_, packet, total_length) == 0)
+ if (packet == last_incoming_rtcp_packet_)
break;
incoming_rtcp_packets_.push_back(
- LoggedRtcpPacketIncoming(timestamp_us, packet, total_length));
- last_incoming_rtcp_packet_length_ = total_length;
- memcpy(last_incoming_rtcp_packet_, packet, total_length);
+ LoggedRtcpPacketIncoming(timestamp_us, packet));
+ last_incoming_rtcp_packet_ = packet;
} else {
outgoing_rtcp_packets_.push_back(
- LoggedRtcpPacketOutgoing(timestamp_us, packet, total_length));
+ LoggedRtcpPacketOutgoing(timestamp_us, packet));
}
break;
}
@@ -1655,12 +1648,10 @@
return nullptr;
}
-// The packet must have space for at least IP_PACKET_SIZE bytes.
ParsedRtcEventLog::ParseStatus ParsedRtcEventLog::GetRtcpPacket(
const rtclog::Event& event,
PacketDirection* incoming,
- uint8_t* packet,
- size_t* length) const {
+ std::vector<uint8_t>* packet) const {
RTC_PARSE_CHECK_OR_RETURN(event.has_type());
RTC_PARSE_CHECK_OR_RETURN_EQ(event.type(), rtclog::Event::RTCP_EVENT);
RTC_PARSE_CHECK_OR_RETURN(event.has_rtcp_packet());
@@ -1670,16 +1661,11 @@
if (incoming != nullptr) {
*incoming = rtcp_packet.incoming() ? kIncomingPacket : kOutgoingPacket;
}
- // Get packet length.
- RTC_PARSE_CHECK_OR_RETURN(rtcp_packet.has_packet_data());
- if (length != nullptr) {
- *length = rtcp_packet.packet_data().size();
- }
// Get packet contents.
+ RTC_PARSE_CHECK_OR_RETURN(rtcp_packet.has_packet_data());
if (packet != nullptr) {
- RTC_PARSE_CHECK_OR_RETURN_LE(rtcp_packet.packet_data().size(),
- static_cast<unsigned>(IP_PACKET_SIZE));
- memcpy(packet, rtcp_packet.packet_data().data(),
+ packet->resize(rtcp_packet.packet_data().size());
+ memcpy(packet->data(), rtcp_packet.packet_data().data(),
rtcp_packet.packet_data().size());
}
return ParseStatus::Success();
diff --git a/logging/rtc_event_log/rtc_event_log_parser.h b/logging/rtc_event_log/rtc_event_log_parser.h
index 712510b..dce075a 100644
--- a/logging/rtc_event_log/rtc_event_log_parser.h
+++ b/logging/rtc_event_log/rtc_event_log_parser.h
@@ -670,8 +670,7 @@
// NB: The packet must have space for at least IP_PACKET_SIZE bytes.
ParseStatus GetRtcpPacket(const rtclog::Event& event,
PacketDirection* incoming,
- uint8_t* packet,
- size_t* length) const;
+ std::vector<uint8_t>* packet) const;
ParseStatusOr<rtclog::StreamConfig> GetVideoReceiveConfig(
const rtclog::Event& event) const;
@@ -873,8 +872,7 @@
std::vector<LoggedRouteChangeEvent> route_change_events_;
std::vector<LoggedRemoteEstimateEvent> remote_estimate_events_;
- uint8_t last_incoming_rtcp_packet_[IP_PACKET_SIZE];
- uint8_t last_incoming_rtcp_packet_length_;
+ std::vector<uint8_t> last_incoming_rtcp_packet_;
int64_t first_timestamp_;
int64_t last_timestamp_;
