Change how event_log_visualizer ignore duplicate incoming RTCP packets.

webrtc/tools/event_log_visualizer/analyzer.cc

 /*
  *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 290 matching lines @@
 
   // Maps a stream identifier consisting of ssrc and direction
   // to the header extensions used by that stream,
   std::map<StreamId, RtpHeaderExtensionMap> extension_maps;
 
   PacketDirection direction;
   uint8_t header[IP_PACKET_SIZE];
   size_t header_length;
   size_t total_length;
 
+  uint8_t last_incoming_rtcp_packet[IP_PACKET_SIZE];
+  memset(&last_incoming_rtcp_packet, 0, 1);
+
   // Make a default extension map for streams without configuration information.
   // TODO(ivoc): Once configuration of audio streams is stored in the event log,
   //             this can be removed. Tracking bug: webrtc:6399
   RtpHeaderExtensionMap default_extension_map = GetDefaultHeaderExtensionMap();
 
   for (size_t i = 0; i < parsed_log_.GetNumberOfEvents(); i++) {
     ParsedRtcEventLog::EventType event_type = parsed_log_.GetEventType(i);
     if (event_type != ParsedRtcEventLog::VIDEO_RECEIVER_CONFIG_EVENT &&
         event_type != ParsedRtcEventLog::VIDEO_SENDER_CONFIG_EVENT &&
         event_type != ParsedRtcEventLog::AUDIO_RECEIVER_CONFIG_EVENT &&
@@ skipping 75 matching lines @@
         uint64_t timestamp = parsed_log_.GetTimestamp(i);
         rtp_packets_[stream].push_back(
             LoggedRtpPacket(timestamp, parsed_header, total_length));
         break;
       }
       case ParsedRtcEventLog::RTCP_EVENT: {
         uint8_t packet[IP_PACKET_SIZE];
         MediaType media_type;
         parsed_log_.GetRtcpPacket(i, &direction, &media_type, packet,
                                   &total_length);
-
-        // Currently feedback is logged twice, both for audio and video.
-        // Only act on one of them.
-        if (media_type == MediaType::AUDIO || media_type == MediaType::ANY) {
-          rtcp::CommonHeader header;
-          const uint8_t* packet_end = packet + total_length;
-          for (const uint8_t* block = packet; block < packet_end;
-               block = header.NextPacket()) {
-            RTC_CHECK(header.Parse(block, packet_end - block));
-            if (header.type() == rtcp::TransportFeedback::kPacketType &&
-                header.fmt() == rtcp::TransportFeedback::kFeedbackMessageType) {
-              std::unique_ptr<rtcp::TransportFeedback> rtcp_packet(
-                  new rtcp::TransportFeedback());
-              if (rtcp_packet->Parse(header)) {
-                uint32_t ssrc = rtcp_packet->sender_ssrc();
-                StreamId stream(ssrc, direction);
-                uint64_t timestamp = parsed_log_.GetTimestamp(i);
-                rtcp_packets_[stream].push_back(LoggedRtcpPacket(
-                    timestamp, kRtcpTransportFeedback, std::move(rtcp_packet)));
-              }
-            } else if (header.type() == rtcp::SenderReport::kPacketType) {
-              std::unique_ptr<rtcp::SenderReport> rtcp_packet(
-                  new rtcp::SenderReport());
-              if (rtcp_packet->Parse(header)) {
-                uint32_t ssrc = rtcp_packet->sender_ssrc();
-                StreamId stream(ssrc, direction);
-                uint64_t timestamp = parsed_log_.GetTimestamp(i);
-                rtcp_packets_[stream].push_back(LoggedRtcpPacket(
-                    timestamp, kRtcpSr, std::move(rtcp_packet)));
-              }
-            } else if (header.type() == rtcp::ReceiverReport::kPacketType) {
-              std::unique_ptr<rtcp::ReceiverReport> rtcp_packet(
-                  new rtcp::ReceiverReport());
-              if (rtcp_packet->Parse(header)) {
-                uint32_t ssrc = rtcp_packet->sender_ssrc();
-                StreamId stream(ssrc, direction);
-                uint64_t timestamp = parsed_log_.GetTimestamp(i);
-                rtcp_packets_[stream].push_back(LoggedRtcpPacket(
-                    timestamp, kRtcpRr, std::move(rtcp_packet)));
-              }
+        // Currently incoming RTCP packets are logged twice, both for audio and
+        // video. Only act on one of them.
+        if (direction == webrtc::kIncomingPacket &&
+            memcmp(last_incoming_rtcp_packet, packet, IP_PACKET_SIZE) == 0) {

[terelius, 2017/05/19 12:07:32]

This reads outside the actual packet. Maybe store size_t
last_incoming_rtcp_packet_size too?

[perkj_webrtc, 2017/05/19 12:29:05]

both are of size IP_PACKET_SIZE. Sure I can store a separate variable as well if
you prefer. 

[terelius, 2017/05/19 12:47:02]

The buffers are of size IP_PACKET_SIZE, but the actual packet data is less. As
is, the code would compare the RTCP packet to the beginning of the previous
packet and then compare the uninitialized part to the packet buffer to the
remainder of the previous packet.

+          continue;
+        } else {
+          memcpy(last_incoming_rtcp_packet, packet, IP_PACKET_SIZE);

[terelius, 2017/05/19 12:07:32]

Doesn't this copy outgoing RTCP into last_incoming_rtcp_packet?

[perkj_webrtc, 2017/05/19 12:29:05]

.... eh, good cath.... But it does not matter. I will fix though. 

+        }
+        rtcp::CommonHeader header;
+        const uint8_t* packet_end = packet + total_length;
+        for (const uint8_t* block = packet; block < packet_end;
+             block = header.NextPacket()) {
+          RTC_CHECK(header.Parse(block, packet_end - block));
+          if (header.type() == rtcp::TransportFeedback::kPacketType &&
+              header.fmt() == rtcp::TransportFeedback::kFeedbackMessageType) {
+            std::unique_ptr<rtcp::TransportFeedback> rtcp_packet(
+                new rtcp::TransportFeedback());
+            if (rtcp_packet->Parse(header)) {
+              uint32_t ssrc = rtcp_packet->sender_ssrc();
+              StreamId stream(ssrc, direction);
+              uint64_t timestamp = parsed_log_.GetTimestamp(i);
+              rtcp_packets_[stream].push_back(LoggedRtcpPacket(
+                  timestamp, kRtcpTransportFeedback, std::move(rtcp_packet)));
+            }
+          } else if (header.type() == rtcp::SenderReport::kPacketType) {
+            std::unique_ptr<rtcp::SenderReport> rtcp_packet(
+                new rtcp::SenderReport());
+            if (rtcp_packet->Parse(header)) {
+              uint32_t ssrc = rtcp_packet->sender_ssrc();
+              StreamId stream(ssrc, direction);
+              uint64_t timestamp = parsed_log_.GetTimestamp(i);
+              rtcp_packets_[stream].push_back(
+                  LoggedRtcpPacket(timestamp, kRtcpSr, std::move(rtcp_packet)));
+            }
+          } else if (header.type() == rtcp::ReceiverReport::kPacketType) {
+            std::unique_ptr<rtcp::ReceiverReport> rtcp_packet(
+                new rtcp::ReceiverReport());
+            if (rtcp_packet->Parse(header)) {
+              uint32_t ssrc = rtcp_packet->sender_ssrc();
+              StreamId stream(ssrc, direction);
+              uint64_t timestamp = parsed_log_.GetTimestamp(i);
+              rtcp_packets_[stream].push_back(
+                  LoggedRtcpPacket(timestamp, kRtcpRr, std::move(rtcp_packet)));
             }
           }
         }
         break;
       }
       case ParsedRtcEventLog::LOG_START: {
         break;
       }
       case ParsedRtcEventLog::LOG_END: {
         break;
@@ skipping 932 matching lines @@
       },
       audio_network_adaptation_events_, begin_time_, &time_series);
   plot->AppendTimeSeries(std::move(time_series));
   plot->SetXAxis(0, call_duration_s_, "Time (s)", kLeftMargin, kRightMargin);
   plot->SetSuggestedYAxis(0, 1, "Number of channels (1 (mono)/2 (stereo))",
                           kBottomMargin, kTopMargin);
   plot->SetTitle("Reported audio encoder number of channels");
 }
 }  // namespace plotting
 }  // namespace webrtc