Track padding and header size in log event.
Padding size and header size are not part of the header, but we still
want to log them. Add the values as separate fields to the log events.
Bug: webrtc:8111
Change-Id: I8dfa2ccafe679f96b8911b538a8512b0170bc642
Reviewed-on: https://webrtc-review.googlesource.com/c/106321
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Elad Alon <eladalon@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25237}diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc
index 7ef8d67..1db2bf8 100644
--- a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc
+++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc
@@ -571,14 +571,14 @@
std::string RtcEventLogEncoderLegacy::EncodeRtpPacketIncoming(
const RtcEventRtpPacketIncoming& event) {
return EncodeRtpPacket(event.timestamp_us_, event.header_,
- event.packet_length_, PacedPacketInfo::kNotAProbe,
+ event.packet_length(), PacedPacketInfo::kNotAProbe,
true);
}
std::string RtcEventLogEncoderLegacy::EncodeRtpPacketOutgoing(
const RtcEventRtpPacketOutgoing& event) {
return EncodeRtpPacket(event.timestamp_us_, event.header_,
- event.packet_length_, event.probe_cluster_id_, false);
+ event.packet_length(), event.probe_cluster_id_, false);
}
std::string RtcEventLogEncoderLegacy::EncodeVideoReceiveStreamConfig(
diff --git a/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.cc b/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.cc
index 8e15d02..d0e5118 100644
--- a/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.cc
+++ b/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.cc
@@ -17,13 +17,20 @@
RtcEventRtpPacketIncoming::RtcEventRtpPacketIncoming(
const RtpPacketReceived& packet)
- : packet_length_(packet.size()) {
+ : payload_length_(packet.payload_size()),
+ header_length_(packet.headers_size()),
+ padding_length_(packet.padding_size()) {
header_.CopyHeaderFrom(packet);
+ RTC_DCHECK_EQ(packet.size(),
+ payload_length_ + header_length_ + padding_length_);
}
RtcEventRtpPacketIncoming::RtcEventRtpPacketIncoming(
const RtcEventRtpPacketIncoming& other)
- : RtcEvent(other.timestamp_us_), packet_length_(other.packet_length_) {
+ : RtcEvent(other.timestamp_us_),
+ payload_length_(other.payload_length_),
+ header_length_(other.header_length_),
+ padding_length_(other.padding_length_) {
header_.CopyHeaderFrom(other.header_);
}
diff --git a/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.h b/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.h
index 6b141b6..edc506b 100644
--- a/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.h
+++ b/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.h
@@ -31,8 +31,14 @@
std::unique_ptr<RtcEvent> Copy() const override;
+ size_t packet_length() const {
+ return payload_length_ + header_length_ + padding_length_;
+ }
+
RtpPacket header_; // Only the packet's header will be stored here.
- const size_t packet_length_; // Length before stripping away all but header.
+ const size_t payload_length_; // Media payload, excluding header and padding.
+ const size_t header_length_; // RTP header.
+ const size_t padding_length_; // RTP padding.
private:
RtcEventRtpPacketIncoming(const RtcEventRtpPacketIncoming& other);
diff --git a/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.cc b/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.cc
index 103440a..3f86c3f 100644
--- a/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.cc
+++ b/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.cc
@@ -18,14 +18,21 @@
RtcEventRtpPacketOutgoing::RtcEventRtpPacketOutgoing(
const RtpPacketToSend& packet,
int probe_cluster_id)
- : packet_length_(packet.size()), probe_cluster_id_(probe_cluster_id) {
+ : payload_length_(packet.payload_size()),
+ header_length_(packet.headers_size()),
+ padding_length_(packet.padding_size()),
+ probe_cluster_id_(probe_cluster_id) {
header_.CopyHeaderFrom(packet);
+ RTC_DCHECK_EQ(packet.size(),
+ payload_length_ + header_length_ + padding_length_);
}
RtcEventRtpPacketOutgoing::RtcEventRtpPacketOutgoing(
const RtcEventRtpPacketOutgoing& other)
: RtcEvent(other.timestamp_us_),
- packet_length_(other.packet_length_),
+ payload_length_(other.payload_length_),
+ header_length_(other.header_length_),
+ padding_length_(other.padding_length_),
probe_cluster_id_(other.probe_cluster_id_) {
header_.CopyHeaderFrom(other.header_);
}
diff --git a/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.h b/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.h
index 171a21c..c21e613 100644
--- a/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.h
+++ b/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.h
@@ -32,8 +32,14 @@
std::unique_ptr<RtcEvent> Copy() const override;
+ size_t packet_length() const {
+ return payload_length_ + header_length_ + padding_length_;
+ }
+
RtpPacket header_; // Only the packet's header will be stored here.
- const size_t packet_length_; // Length before stripping away all but header.
+ const size_t payload_length_; // Media payload, excluding header and padding.
+ const size_t header_length_; // RTP header.
+ const size_t padding_length_; // RTP padding.
const int probe_cluster_id_;
private:
diff --git a/logging/rtc_event_log/rtc_event_log2.proto b/logging/rtc_event_log/rtc_event_log2.proto
index d1e2f30..213aee7 100644
--- a/logging/rtc_event_log/rtc_event_log2.proto
+++ b/logging/rtc_event_log/rtc_event_log2.proto
@@ -63,14 +63,15 @@
// TODO(terelius/dinor): Add CSRCs. Field number 7 reserved for this purpose.
- // required - The size (in bytes) of the packet including header, payload
- // and padding.
- optional uint32 packet_size = 8;
+ // required - The size (in bytes) of the media payload, not including
+ // RTP header or padding. The packet size is the sum of payload, header and
+ // padding.
+ optional uint32 payload_size = 8;
// required - The size (in bytes) of the RTP header.
optional uint32 header_size = 9;
- // required - The size (in bytes) of the RTP header.
+ // required - The size (in bytes) of the padding.
optional uint32 padding_size = 10;
// optional - required if the batch contains delta encoded events.
@@ -94,7 +95,7 @@
optional bytes rtp_timestamp_deltas = 105;
// Field number 107 reserved for CSRC.
optional bytes ssrc_deltas = 106;
- optional bytes packet_size_deltas = 108;
+ optional bytes payload_size_deltas = 108;
optional bytes header_size_deltas = 109;
optional bytes padding_size_deltas = 110;
// Field number 111-114 reserved for future use.
@@ -126,14 +127,15 @@
// TODO(terelius/dinor): Add CSRCs. Field number 7 reserved for this purpose.
- // required - The size (in bytes) of the packet including header, payload
- // and padding.
- optional uint32 packet_size = 8;
+ // required - The size (in bytes) of the media payload, not including
+ // RTP header or padding. The packet size is the sum of payload, header and
+ // padding.
+ optional uint32 payload_size = 8;
// required - The size (in bytes) of the RTP header.
optional uint32 header_size = 9;
- // required - The size (in bytes) of the RTP header.
+ // required - The size (in bytes) of the padding.
optional uint32 padding_size = 10;
// optional - required if the batch contains delta encoded events.
@@ -157,7 +159,7 @@
optional bytes rtp_timestamp_deltas = 105;
optional bytes ssrc_deltas = 106;
// Field number 107 reserved for CSRC.
- optional bytes packet_size_deltas = 108;
+ optional bytes payload_size_deltas = 108;
optional bytes header_size_deltas = 109;
optional bytes padding_size_deltas = 110;
// Field number 111-114 reserved for future use.
diff --git a/logging/rtc_event_log/rtc_event_log_unittest_helper.cc b/logging/rtc_event_log/rtc_event_log_unittest_helper.cc
index db51d7d..581e791 100644
--- a/logging/rtc_event_log/rtc_event_log_unittest_helper.cc
+++ b/logging/rtc_event_log/rtc_event_log_unittest_helper.cc
@@ -655,17 +655,12 @@
EXPECT_EQ(original_event.header_.headers_size(),
logged_event.rtp.header_length);
- EXPECT_EQ(original_event.packet_length_, logged_event.rtp.total_length);
+ EXPECT_EQ(original_event.packet_length(), logged_event.rtp.total_length);
- if ((original_event.header_.data()[0] & 0x20) != 0) { // has padding
- // Currently, RTC eventlog encoder-parser can only maintain padding length
- // if packet is full padding.
- // TODO(webrtc:9730): Change the condition to something like
- // original_event.padding_length_ != logged_event.rtp.header.paddingLength.
- EXPECT_EQ(
- original_event.packet_length_ - original_event.header_.headers_size(),
- logged_event.rtp.header.paddingLength);
- }
+ // Currently, RTC eventlog encoder-parser can only maintain padding length
+ // if packet is full padding.
+ EXPECT_EQ(original_event.padding_length_,
+ logged_event.rtp.header.paddingLength);
VerifyLoggedRtpHeader(original_event.header_, logged_event.rtp.header);
}
@@ -678,17 +673,12 @@
EXPECT_EQ(original_event.header_.headers_size(),
logged_event.rtp.header_length);
- EXPECT_EQ(original_event.packet_length_, logged_event.rtp.total_length);
+ EXPECT_EQ(original_event.packet_length(), logged_event.rtp.total_length);
- if ((original_event.header_.data()[0] & 0x20) != 0) { // has padding
- // Currently, RTC eventlog encoder-parser can only maintain padding length
- // if packet is full padding.
- // TODO(webrtc:9730): Change the condition to something like
- // original_event.padding_length_ != logged_event.rtp.header.paddingLength.
- EXPECT_EQ(
- original_event.packet_length_ - original_event.header_.headers_size(),
- logged_event.rtp.header.paddingLength);
- }
+ // Currently, RTC eventlog encoder-parser can only maintain padding length
+ // if packet is full padding.
+ EXPECT_EQ(original_event.padding_length_,
+ logged_event.rtp.header.paddingLength);
// TODO(terelius): Probe cluster ID isn't parsed, used or tested. Unless
// someone has a strong reason to keep it, it'll be removed.
