From 94c09169a22a847a6aa8d9d13f1949d1bc497c58 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Terelius?= <terelius@webrtc.org>
Date: Tue, 17 May 2022 16:47:31 +0200
Subject: [PATCH] Improve RTC event log parser.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

(Support parsing TWCC which nack sequnce numbers greater than the last
one received. Don't silently drop unrecognized/malformed RTCP packets.)

Bug: webrtc:14078
Change-Id: I34a0deabfdb5f36b988919cfcc9159197435756c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/262800
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36921}
---
 logging/rtc_event_log/rtc_event_log_parser.cc | 47 +++++++++----------
 1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/logging/rtc_event_log/rtc_event_log_parser.cc b/logging/rtc_event_log/rtc_event_log_parser.cc
index 4033a593a1..a67c95aa58 100644
--- a/logging/rtc_event_log/rtc_event_log_parser.cc
+++ b/logging/rtc_event_log/rtc_event_log_parser.cc
@@ -690,46 +690,40 @@ ParsedRtcEventLog::ParseStatus StoreRtcpBlocks(
         header.fmt() == rtcp::TransportFeedback::kFeedbackMessageType) {
       LoggedRtcpPacketTransportFeedback parsed_block;
       parsed_block.timestamp = timestamp;
-      if (parsed_block.transport_feedback.Parse(header))
-        transport_feedback_list->push_back(std::move(parsed_block));
+      RTC_PARSE_CHECK_OR_RETURN(parsed_block.transport_feedback.Parse(header));
+      transport_feedback_list->push_back(std::move(parsed_block));
     } else if (header.type() == rtcp::SenderReport::kPacketType) {
       LoggedRtcpPacketSenderReport parsed_block;
       parsed_block.timestamp = timestamp;
-      if (parsed_block.sr.Parse(header)) {
-        sr_list->push_back(std::move(parsed_block));
-      }
+      RTC_PARSE_CHECK_OR_RETURN(parsed_block.sr.Parse(header));
+      sr_list->push_back(std::move(parsed_block));
     } else if (header.type() == rtcp::ReceiverReport::kPacketType) {
       LoggedRtcpPacketReceiverReport parsed_block;
       parsed_block.timestamp = timestamp;
-      if (parsed_block.rr.Parse(header)) {
-        rr_list->push_back(std::move(parsed_block));
-      }
+      RTC_PARSE_CHECK_OR_RETURN(parsed_block.rr.Parse(header));
+      rr_list->push_back(std::move(parsed_block));
     } else if (header.type() == rtcp::ExtendedReports::kPacketType) {
       LoggedRtcpPacketExtendedReports parsed_block;
       parsed_block.timestamp = timestamp;
-      if (parsed_block.xr.Parse(header)) {
-        xr_list->push_back(std::move(parsed_block));
-      }
+      RTC_PARSE_CHECK_OR_RETURN(parsed_block.xr.Parse(header));
+      xr_list->push_back(std::move(parsed_block));
     } else if (header.type() == rtcp::Fir::kPacketType &&
                header.fmt() == rtcp::Fir::kFeedbackMessageType) {
       LoggedRtcpPacketFir parsed_block;
       parsed_block.timestamp = timestamp;
-      if (parsed_block.fir.Parse(header)) {
-        fir_list->push_back(std::move(parsed_block));
-      }
+      RTC_PARSE_CHECK_OR_RETURN(parsed_block.fir.Parse(header));
+      fir_list->push_back(std::move(parsed_block));
     } else if (header.type() == rtcp::Pli::kPacketType &&
                header.fmt() == rtcp::Pli::kFeedbackMessageType) {
       LoggedRtcpPacketPli parsed_block;
       parsed_block.timestamp = timestamp;
-      if (parsed_block.pli.Parse(header)) {
-        pli_list->push_back(std::move(parsed_block));
-      }
+      RTC_PARSE_CHECK_OR_RETURN(parsed_block.pli.Parse(header));
+      pli_list->push_back(std::move(parsed_block));
     } else if (header.type() == rtcp::Bye::kPacketType) {
       LoggedRtcpPacketBye parsed_block;
       parsed_block.timestamp = timestamp;
-      if (parsed_block.bye.Parse(header)) {
-        bye_list->push_back(std::move(parsed_block));
-      }
+      RTC_PARSE_CHECK_OR_RETURN(parsed_block.bye.Parse(header));
+      bye_list->push_back(std::move(parsed_block));
     } else if (header.type() == rtcp::Psfb::kPacketType &&
                header.fmt() == rtcp::Psfb::kAfbMessageType) {
       bool type_found = false;
@@ -749,13 +743,13 @@ ParsedRtcEventLog::ParseStatus StoreRtcpBlocks(
           type_found = true;
         }
       }
+      // We ignore other application-layer feedback types.
     } else if (header.type() == rtcp::Nack::kPacketType &&
                header.fmt() == rtcp::Nack::kFeedbackMessageType) {
       LoggedRtcpPacketNack parsed_block;
       parsed_block.timestamp = timestamp;
-      if (parsed_block.nack.Parse(header)) {
-        nack_list->push_back(std::move(parsed_block));
-      }
+      RTC_PARSE_CHECK_OR_RETURN(parsed_block.nack.Parse(header));
+      nack_list->push_back(std::move(parsed_block));
     }
   }
   return ParsedRtcEventLog::ParseStatus::Success();
@@ -2393,8 +2387,11 @@ std::vector<LoggedPacketInfo> ParsedRtcEventLog::GetPacketInfos(
         last->last_in_feedback = true;
         for (LoggedPacketInfo* fb : packet_feedbacks) {
           if (direction == PacketDirection::kOutgoingPacket) {
-            fb->feedback_hold_duration =
-                last->reported_recv_time - fb->reported_recv_time;
+            if (last->reported_recv_time.IsFinite() &&
+                fb->reported_recv_time.IsFinite()) {
+              fb->feedback_hold_duration =
+                  last->reported_recv_time - fb->reported_recv_time;
+            }
           } else {
             fb->feedback_hold_duration =
                 log_feedback_time - fb->log_packet_time;