[DVQA] Fix reporting of time_between_freezes_ms

Fix reporting of time_between_freezes_ms when all peers were
unregistered before end of the call.

Bug: b/243501613
Change-Id: Ie2b77d8399e4f4bf672e80f62ed27609336af171
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272650
Reviewed-by: Andrey Logvin <landrey@google.com>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37885}

test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc

@@ -614,6 +614,10 @@ void DefaultVideoQualityAnalyzer::UnregisterParticipantInCall(
         peer_index == stream_state.sender()) {
       continue;
     }
+
+    AddExistingFramesInFlightForStreamToComparator(stream_index, stream_state,
+                                                   *peer_index);
+
     stream_state.RemovePeer(*peer_index);
   }
 
@@ -660,7 +664,10 @@ void DefaultVideoQualityAnalyzer::Stop() {
     for (auto& state_entry : stream_states_) {
       const size_t stream_index = state_entry.first;
       StreamState& stream_state = state_entry.second;
-      for (size_t peer_index : peers_->GetPresentIndexes()) {
+
+      // Populate `last_rendered_frame_times` map for all peers that were met in
+      // call, not only for the currently presented ones.
+      for (size_t peer_index : peers_->GetAllIndexes()) {
         if (peer_index == stream_state.sender() &&
             !options_.enable_receive_own_stream) {
           continue;
@@ -679,25 +686,18 @@ void DefaultVideoQualityAnalyzer::Stop() {
               stats_key,
               stream_state.last_rendered_frame_time(peer_index).value());
         }
-
-        // Add frames in flight for this stream into frames comparator.
-        // Frames in flight were not rendered, so they won't affect stream's
-        // last rendered frame time.
-        while (!stream_state.IsEmpty(peer_index)) {
-          uint16_t frame_id = stream_state.PopFront(peer_index);
-          auto it = captured_frames_in_flight_.find(frame_id);
-          RTC_DCHECK(it != captured_frames_in_flight_.end());
-          FrameInFlight& frame = it->second;
-
-          frames_comparator_.AddComparison(
-              stats_key, /*captured=*/absl::nullopt,
-              /*rendered=*/absl::nullopt, FrameComparisonType::kFrameInFlight,
-              frame.GetStatsForPeer(peer_index));
-
-          if (frame.HaveAllPeersReceived()) {
-            captured_frames_in_flight_.erase(it);
       }
+
+      // Push left frame in flight for analysis for the peers that are still in
+      // the call.
+      for (size_t peer_index : peers_->GetPresentIndexes()) {
+        if (peer_index == stream_state.sender() &&
+            !options_.enable_receive_own_stream) {
+          continue;
         }
+
+        AddExistingFramesInFlightForStreamToComparator(
+            stream_index, stream_state, peer_index);
       }
     }
   }
@@ -824,6 +824,28 @@ uint16_t DefaultVideoQualityAnalyzer::GetNextFrameId() {
   return frame_id;
 }
 
+void DefaultVideoQualityAnalyzer::
+    AddExistingFramesInFlightForStreamToComparator(size_t stream_index,
+                                                   StreamState& stream_state,
+                                                   size_t peer_index) {
+  InternalStatsKey stats_key(stream_index, stream_state.sender(), peer_index);
+
+  // Add frames in flight for this stream into frames comparator.
+  // Frames in flight were not rendered, so they won't affect stream's
+  // last rendered frame time.
+  while (!stream_state.IsEmpty(peer_index)) {
+    uint16_t frame_id = stream_state.PopFront(peer_index);
+    auto it = captured_frames_in_flight_.find(frame_id);
+    RTC_DCHECK(it != captured_frames_in_flight_.end());
+    FrameInFlight& frame = it->second;
+
+    frames_comparator_.AddComparison(stats_key, /*captured=*/absl::nullopt,
+                                     /*rendered=*/absl::nullopt,
+                                     FrameComparisonType::kFrameInFlight,
+                                     frame.GetStatsForPeer(peer_index));
+  }
+}
+
 void DefaultVideoQualityAnalyzer::ReportResults() {
   using ::webrtc::test::ImproveDirection;
 

test/pc/e2e/analyzer/video/default_video_quality_analyzer.h

@@ -115,6 +115,11 @@ class DefaultVideoQualityAnalyzer : public VideoQualityAnalyzerInterface {
   // because this value is reserved by `VideoFrame` as "ID not set".
   uint16_t GetNextFrameId() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
 
+  void AddExistingFramesInFlightForStreamToComparator(size_t stream_index,
+                                                      StreamState& stream_state,
+                                                      size_t peer_index)
+      RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
+
   // Report results for all metrics for all streams.
   void ReportResults();
   void ReportResults(const std::string& test_case_name,

test/pc/e2e/analyzer/video/default_video_quality_analyzer_test.cc

@@ -31,6 +31,9 @@
 namespace webrtc {
 namespace {
 
+using ::testing::TestWithParam;
+using ::testing::ValuesIn;
+
 using StatsSample = ::webrtc::SamplesStatsCounter::StatsSample;
 
 constexpr int kAnalyzerMaxThreadsCount = 1;
@@ -115,7 +118,8 @@ void PassFramesThroughAnalyzer(DefaultVideoQualityAnalyzer& analyzer,
                                absl::string_view stream_label,
                                std::vector<absl::string_view> receivers,
                                int frames_count,
-                               test::FrameGeneratorInterface& frame_generator) {
+                               test::FrameGeneratorInterface& frame_generator,
+                               int interframe_delay_ms = 0) {
   for (int i = 0; i < frames_count; ++i) {
     VideoFrame frame = NextFrame(&frame_generator, /*timestamp_us=*/1);
     uint16_t frame_id =
@@ -132,6 +136,9 @@ void PassFramesThroughAnalyzer(DefaultVideoQualityAnalyzer& analyzer,
                               VideoQualityAnalyzerInterface::DecoderStats());
       analyzer.OnFrameRendered(receiver, received_frame);
     }
+    if (i < frames_count - 1 && interframe_delay_ms > 0) {
+      SleepMs(interframe_delay_ms);
+    }
   }
 }
 
@@ -1976,5 +1983,76 @@ TEST(DefaultVideoQualityAnalyzerTest,
   EXPECT_EQ(alice_charlie_stream_conters.rendered, 12);
 }
 
+TEST(DefaultVideoQualityAnalyzerTest,
+     FramesInFlightAreAccountedForUnregisterPeers) {
+  std::unique_ptr<test::FrameGeneratorInterface> frame_generator =
+      test::CreateSquareFrameGenerator(kFrameWidth, kFrameHeight,
+                                       /*type=*/absl::nullopt,
+                                       /*num_squares=*/absl::nullopt);
+
+  DefaultVideoQualityAnalyzerOptions options = AnalyzerOptionsForTest();
+  DefaultVideoQualityAnalyzer analyzer(Clock::GetRealTimeClock(), options);
+  analyzer.Start("test_case", std::vector<std::string>{"alice", "bob"},
+                 kAnalyzerMaxThreadsCount);
+
+  // Add one frame in flight which has encode time >= 10ms.
+  VideoFrame frame = NextFrame(frame_generator.get(), /*timestamp_us=*/1);
+  uint16_t frame_id = analyzer.OnFrameCaptured("alice", "alice_video", frame);
+  frame.set_id(frame_id);
+  analyzer.OnFramePreEncode("alice", frame);
+  SleepMs(10);
+  analyzer.OnFrameEncoded("alice", frame.id(), FakeEncode(frame),
+                          VideoQualityAnalyzerInterface::EncoderStats());
+
+  analyzer.UnregisterParticipantInCall("bob");
+
+  // Give analyzer some time to process frames on async thread. The computations
+  // have to be fast (heavy metrics are disabled!), so if doesn't fit 100ms it
+  // means we have an issue!
+  SleepMs(100);
+  analyzer.Stop();
+
+  StreamStats stats = analyzer.GetStats().at(StatsKey("alice_video", "bob"));
+  ASSERT_EQ(stats.encode_time_ms.NumSamples(), 1);
+  EXPECT_GE(stats.encode_time_ms.GetAverage(), 10);
+}
+
+class DefaultVideoQualityAnalyzerTimeBetweenFreezesTest
+    : public TestWithParam<bool> {};
+
+TEST_P(DefaultVideoQualityAnalyzerTimeBetweenFreezesTest,
+       TimeBetweenFreezesIsEqualToStreamDurationWhenThereAreNoFeeezes) {
+  std::unique_ptr<test::FrameGeneratorInterface> frame_generator =
+      test::CreateSquareFrameGenerator(kFrameWidth, kFrameHeight,
+                                       /*type=*/absl::nullopt,
+                                       /*num_squares=*/absl::nullopt);
+
+  DefaultVideoQualityAnalyzerOptions options = AnalyzerOptionsForTest();
+  DefaultVideoQualityAnalyzer analyzer(Clock::GetRealTimeClock(), options);
+  analyzer.Start("test_case", std::vector<std::string>{"alice", "bob"},
+                 kAnalyzerMaxThreadsCount);
+
+  PassFramesThroughAnalyzer(analyzer, "alice", "alice_video", {"bob"},
+                            /*frames_count=*/5, *frame_generator,
+                            /*interframe_delay_ms=*/50);
+  if (GetParam()) {
+    analyzer.UnregisterParticipantInCall("bob");
+  }
+
+  // Give analyzer some time to process frames on async thread. The computations
+  // have to be fast (heavy metrics are disabled!), so if doesn't fit 100ms it
+  // means we have an issue!
+  SleepMs(50);
+  analyzer.Stop();
+
+  StreamStats stats = analyzer.GetStats().at(StatsKey("alice_video", "bob"));
+  ASSERT_EQ(stats.time_between_freezes_ms.NumSamples(), 1);
+  EXPECT_GE(stats.time_between_freezes_ms.GetAverage(), 200);
+}
+
+INSTANTIATE_TEST_SUITE_P(WithRegisteredAndUnregisteredPeerAtTheEndOfTheCall,
+                         DefaultVideoQualityAnalyzerTimeBetweenFreezesTest,
+                         ValuesIn({true, false}));
+
 }  // namespace
 }  // namespace webrtc