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 64b5f05816..8cf9e7a011 100644 --- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc +++ b/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; 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 5aec7c1e26..44bcd28ef9 100644 --- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h +++ b/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, 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 40662fd31d..3220cc0ae9 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 @@ -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 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 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{"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 {}; + +TEST_P(DefaultVideoQualityAnalyzerTimeBetweenFreezesTest, + TimeBetweenFreezesIsEqualToStreamDurationWhenThereAreNoFeeezes) { + std::unique_ptr 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{"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