Fix DVQA to tolerate the case when frame was fully received before beeing encoded

Bug: b/194482121
Change-Id: Ie9268291242168021860e4e84b62634507a8128d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227701
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34652}

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

@@ -302,7 +302,15 @@ void DefaultVideoQualityAnalyzer::OnFrameEncoded(
     const EncoderStats& stats) {
   MutexLock lock(&lock_);
   auto it = captured_frames_in_flight_.find(frame_id);
-  RTC_DCHECK(it != captured_frames_in_flight_.end());
+  if (it == captured_frames_in_flight_.end()) {
+    RTC_LOG(WARNING)
+        << "The encoding of video frame with id [" << frame_id << "] for peer ["
+        << peer_name << "] finished after all receivers rendered this frame. "
+        << "It can be OK for simulcast/SVC if higher quality stream is not "
+        << "required, but it may indicate an ERROR for singlecast or if it "
+        << "happens often.";
+    return;
+  }
   // For SVC we can receive multiple encoded images for one frame, so to cover
   // all cases we have to pick the last encode time.
   if (!it->second.HasEncodedTime()) {

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

@@ -526,13 +526,17 @@ class DefaultVideoQualityAnalyzer : public VideoQualityAnalyzerInterface {
   // Mapping from stream label to unique size_t value to use in stats and avoid
   // extra string copying.
   NamesCollection streams_ RTC_GUARDED_BY(lock_);
-  // Frames that were captured by all streams and still aren't rendered by any
-  // stream or deemed dropped. Frame with id X can be removed from this map if:
-  // 1. The frame with id X was received in OnFrameRendered
-  // 2. The frame with id Y > X was received in OnFrameRendered
+  // Frames that were captured by all streams and still aren't rendered on
+  // receviers or deemed dropped. Frame with id X can be removed from this map
+  // if:
+  // 1. The frame with id X was received in OnFrameRendered by all expected
+  //    receivers.
+  // 2. The frame with id Y > X was received in OnFrameRendered by all expected
+  //    receivers.
   // 3. Next available frame id for newly captured frame is X
   // 4. There too many frames in flight for current video stream and X is the
-  //    oldest frame id in this stream.
+  //    oldest frame id in this stream. In such case only the frame content
+  //    will be removed, but the map entry will be preserved.
   std::map<uint16_t, FrameInFlight> captured_frames_in_flight_
       RTC_GUARDED_BY(lock_);
   // Global frames count for all video streams.

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

@@ -902,6 +902,67 @@ TEST(DefaultVideoQualityAnalyzerTest, RuntimeParticipantsAdding) {
   }
 }
 
+TEST(DefaultVideoQualityAnalyzerTest,
+     SimulcastFrameWasFullyReceivedByAllPeersBeforeEncodeFinish) {
+  std::unique_ptr<test::FrameGeneratorInterface> frame_generator =
+      test::CreateSquareFrameGenerator(kFrameWidth, kFrameHeight,
+                                       /*type=*/absl::nullopt,
+                                       /*num_squares=*/absl::nullopt);
+
+  DefaultVideoQualityAnalyzer analyzer(Clock::GetRealTimeClock(),
+                                       AnalyzerOptionsForTest());
+  constexpr char kAlice[] = "alice";
+  constexpr char kBob[] = "bob";
+  constexpr char kCharlie[] = "charlie";
+  analyzer.Start("test_case", std::vector<std::string>{kAlice, kBob, kCharlie},
+                 kAnalyzerMaxThreadsCount);
+
+  VideoFrame frame = NextFrame(frame_generator.get(), 1);
+
+  frame.set_id(analyzer.OnFrameCaptured(kAlice, kStreamLabel, frame));
+  analyzer.OnFramePreEncode(kAlice, frame);
+  // Encode 1st simulcast layer
+  analyzer.OnFrameEncoded(kAlice, frame.id(), FakeEncode(frame),
+                          VideoQualityAnalyzerInterface::EncoderStats());
+
+  // Receive by Bob
+  VideoFrame received_frame = DeepCopy(frame);
+  analyzer.OnFramePreDecode(kBob, received_frame.id(),
+                            FakeEncode(received_frame));
+  analyzer.OnFrameDecoded(kBob, received_frame,
+                          VideoQualityAnalyzerInterface::DecoderStats());
+  analyzer.OnFrameRendered(kBob, received_frame);
+  // Receive by Charlie
+  received_frame = DeepCopy(frame);
+  analyzer.OnFramePreDecode(kCharlie, received_frame.id(),
+                            FakeEncode(received_frame));
+  analyzer.OnFrameDecoded(kCharlie, received_frame,
+                          VideoQualityAnalyzerInterface::DecoderStats());
+  analyzer.OnFrameRendered(kCharlie, received_frame);
+
+  // Encode 2nd simulcast layer
+  analyzer.OnFrameEncoded(kAlice, frame.id(), FakeEncode(frame),
+                          VideoQualityAnalyzerInterface::EncoderStats());
+
+  // 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();
+
+  AnalyzerStats stats = analyzer.GetAnalyzerStats();
+  EXPECT_EQ(stats.comparisons_done, 2);
+
+  std::vector<StatsSample> frames_in_flight_sizes =
+      GetSortedSamples(stats.frames_in_flight_left_count);
+  EXPECT_EQ(frames_in_flight_sizes.back().value, 0)
+      << ToString(frames_in_flight_sizes);
+
+  FrameCounters frame_counters = analyzer.GetGlobalCounters();
+  EXPECT_EQ(frame_counters.captured, 1);
+  EXPECT_EQ(frame_counters.rendered, 2);
+}
+
 }  // namespace
 }  // namespace webrtc_pc_e2e
 }  // namespace webrtc