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}
diff --git a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc
index 41c5386..606ee01 100644
--- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc
+++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc
@@ -302,7 +302,15 @@
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()) {
diff --git a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h
index e4437fb..b8e312a 100644
--- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h
+++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h
@@ -526,13 +526,17 @@
// 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.
diff --git a/test/pc/e2e/analyzer/video/default_video_quality_analyzer_test.cc b/test/pc/e2e/analyzer/video/default_video_quality_analyzer_test.cc
index 8d8a1af..5a99f97 100644
--- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer_test.cc
+++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer_test.cc
@@ -902,6 +902,67 @@
}
}
+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
