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}

logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc

@@ -571,14 +571,14 @@ std::string RtcEventLogEncoderLegacy::EncodeRtcpPacketOutgoing(
 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(

logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.cc

@@ -17,13 +17,20 @@ namespace webrtc {
 
 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_);
 }
 

logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.h

@@ -31,8 +31,14 @@ class RtcEventRtpPacketIncoming final : public RtcEvent {
 
   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);

logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.cc

@@ -18,14 +18,21 @@ namespace webrtc {
 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_);
 }

logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.h

@@ -32,8 +32,14 @@ class RtcEventRtpPacketOutgoing final : public RtcEvent {
 
   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:

logging/rtc_event_log/rtc_event_log2.proto

@@ -63,14 +63,15 @@ message IncomingRtpPackets {
 
   // TODO(terelius/dinor): Add CSRCs. Field number 7 reserved for this purpose.
 
-  // required - The size (in bytes) of the packet including header, payload
+  // required - The size (in bytes) of the media payload, not including
-  // and padding.
+  // RTP header or padding. The packet size is the sum of payload, header and
-  optional uint32 packet_size = 8;
+  // 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 @@ message IncomingRtpPackets {
   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 @@ message OutgoingRtpPackets {
 
   // TODO(terelius/dinor): Add CSRCs. Field number 7 reserved for this purpose.
 
-  // required - The size (in bytes) of the packet including header, payload
+  // required - The size (in bytes) of the media payload, not including
-  // and padding.
+  // RTP header or padding. The packet size is the sum of payload, header and
-  optional uint32 packet_size = 8;
+  // 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 @@ message OutgoingRtpPackets {
   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.

logging/rtc_event_log/rtc_event_log_unittest_helper.cc

@@ -655,17 +655,12 @@ void VerifyLoggedRtpPacketIncoming(
   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
-    // Currently, RTC eventlog encoder-parser can only maintain padding length
+  // if packet is full padding.
-    // if packet is full padding.
+  EXPECT_EQ(original_event.padding_length_,
-    // TODO(webrtc:9730): Change the condition to something like
+            logged_event.rtp.header.paddingLength);
-    // 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);
-  }
 
   VerifyLoggedRtpHeader(original_event.header_, logged_event.rtp.header);
 }
@@ -678,17 +673,12 @@ void VerifyLoggedRtpPacketOutgoing(
   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
-    // Currently, RTC eventlog encoder-parser can only maintain padding length
+  // if packet is full padding.
-    // if packet is full padding.
+  EXPECT_EQ(original_event.padding_length_,
-    // TODO(webrtc:9730): Change the condition to something like
+            logged_event.rtp.header.paddingLength);
-    // 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);
-  }
 
   // TODO(terelius): Probe cluster ID isn't parsed, used or tested. Unless
   // someone has a strong reason to keep it, it'll be removed.