Changed LogRtpHeader to read the header length from the packet

webrtc/video/rtc_event_log.cc

Index: webrtc/video/rtc_event_log.cc
diff --git a/webrtc/video/rtc_event_log.cc b/webrtc/video/rtc_event_log.cc
index 476ee2afd79c82b9be6b41feb7424939842d942b..d632920ca53d0f5709dad20b22e1584aced4c99a 100644
--- a/webrtc/video/rtc_event_log.cc
+++ b/webrtc/video/rtc_event_log.cc
@@ -16,6 +16,7 @@
 #include "webrtc/base/criticalsection.h"
 #include "webrtc/base/thread_annotations.h"
 #include "webrtc/call.h"
+#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
 #include "webrtc/system_wrappers/interface/clock.h"
 #include "webrtc/system_wrappers/interface/file_wrapper.h"
 
@@ -44,8 +45,7 @@ class RtcEventLogImpl final : public RtcEventLog {
   void LogRtpHeader(bool incoming,
                     MediaType media_type,
                     const uint8_t* header,
-                    size_t header_length,
-                    size_t total_length) override {}
+                    size_t packet_length) override {}
   void LogRtcpPacket(bool incoming,
                      MediaType media_type,
                      const uint8_t* packet,
@@ -67,8 +67,7 @@ class RtcEventLogImpl final : public RtcEventLog {
   void LogRtpHeader(bool incoming,
                     MediaType media_type,
                     const uint8_t* header,
-                    size_t header_length,
-                    size_t total_length) override;
+                    size_t packet_length) override;
   void LogRtcpPacket(bool incoming,
                      MediaType media_type,
                      const uint8_t* packet,
@@ -284,13 +283,26 @@ void RtcEventLogImpl::LogVideoSendStreamConfig(
   HandleEvent(&event);
 }
 
-// TODO(terelius): It is more convenient and less error prone to parse the
-// header length from the packet instead of relying on the caller to provide it.
 void RtcEventLogImpl::LogRtpHeader(bool incoming,
                                    MediaType media_type,
                                    const uint8_t* header,
-                                   size_t header_length,
-                                   size_t total_length) {
+                                   size_t packet_length) {
+  // Read header length (in bytes) from packet data.
+  if (packet_length < 12u) {
+      return;  // Don't read outside the packet.
+  }
+  const uint8_t X  = (header[0] & 0x10) == 0 ? 0 : 1;

[hlundin-webrtc, 2015/08/04 09:43:50]

const bool x = (header[0] & 0x10) != 0;

[terelius, 2015/08/12 13:12:40]

Done.

+  const uint8_t CC = header[0] & 0x0f;

[hlundin-webrtc, 2015/08/04 09:43:50]

cc

[terelius, 2015/08/12 13:12:40]

Done.

+  size_t header_length = 12u + CC*4u;

[hlundin-webrtc, 2015/08/04 09:43:50]

The style guide says it is ok to omit the spaces around factors, but I think we
tend to include them in the WebRTC code base.

[terelius, 2015/08/12 13:12:40]

Done.

+
+  if (X) {
+    if (packet_length < 12u + CC*4u + 4u) {

[ivoc, 2015/08/06 12:29:44]

Why not make this 16u + CC*4u? Another option would be header_length + 4u.

[terelius, 2015/08/12 13:12:40]

I wrote it this way to more clearly match the RTP specification. The fixed
header is 12 bytes, then follows CC 32-bit words and then (if there are header
extensions) 4 bytes containing the number of extensions. All optimizing
compilers will compute it as 16+CC*4, so there is no performance penalty.

[hlundin-webrtc, 2015/08/13 14:09:51]

I like it this way.

[ivoc, 2015/08/14 11:21:53]

Acknowledged.

+      return;  // Don't read outside the packet.
+    }
+    size_t XLen = ByteReader<uint16_t>::ReadBigEndian(header + 14 + CC*4);

[hlundin-webrtc, 2015/08/04 09:43:50]

x_len

[terelius, 2015/08/12 13:12:39]

Done.

+    header_length += (XLen+1)*4;

[hlundin-webrtc, 2015/08/04 09:43:50]

Spaces around binary operators, please.

[terelius, 2015/08/12 13:12:39]

Done.

+  }
+
   rtc::CritScope lock(&crit_);
   rtclog::Event rtp_event;
   const int64_t timestamp = clock_->TimeInMicroseconds();
@@ -298,7 +310,7 @@ void RtcEventLogImpl::LogRtpHeader(bool incoming,
   rtp_event.set_type(rtclog::Event::RTP_EVENT);
   rtp_event.mutable_rtp_packet()->set_incoming(incoming);
   rtp_event.mutable_rtp_packet()->set_type(ConvertMediaType(media_type));
-  rtp_event.mutable_rtp_packet()->set_packet_length(total_length);
+  rtp_event.mutable_rtp_packet()->set_packet_length(packet_length);
   rtp_event.mutable_rtp_packet()->set_header(header, header_length);
   HandleEvent(&rtp_event);
 }