From 2671dac29c27d033d14a79708e9a2b9b2b66b8d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Tue, 19 May 2020 16:29:09 +0200 Subject: [PATCH] Fix all known VideoStreamEncoderTest flakes. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some VideoStreamEncoderTests such as AdaptsFramerateForLowQuality_MaintainResolutionMode had been observed to be flaky. In a local run, this test failed 1/12000 times, but on bots it may have failed more often. All tests could sometimes fail due to expecting something to be the case without waiting for that thing to happen, which was made evident when adding SleepMs() at strategic points in the code and running the tests locally. With this CL, the tests should no longer be flaky after having added waiting for adaptation to be applied or EXPECT_TRUE_WAIT for sink wants to have the appropriate values. Bug: webrtc:11586 Change-Id: I60e76c742d9ccc8305feb14a834a4f61a60a62a3 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175654 Commit-Queue: Henrik Boström Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#31327} --- video/BUILD.gn | 1 + video/video_stream_encoder_unittest.cc | 21 +++++++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/video/BUILD.gn b/video/BUILD.gn index 1b88380d95..25df5085f3 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -615,6 +615,7 @@ if (rtc_include_tests) { "../modules/video_coding:webrtc_vp9", "../rtc_base", "../rtc_base:checks", + "../rtc_base:gunit_helpers", "../rtc_base:rate_limiter", "../rtc_base:rtc_base_approved", "../rtc_base:rtc_base_tests_utils", diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index a76820367e..9292933d89 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -34,6 +34,7 @@ #include "modules/video_coding/utility/quality_scaler.h" #include "modules/video_coding/utility/simulcast_rate_allocator.h" #include "rtc_base/fake_clock.h" +#include "rtc_base/gunit.h" #include "rtc_base/logging.h" #include "rtc_base/ref_counted_object.h" #include "system_wrappers/include/field_trial.h" @@ -1998,7 +1999,7 @@ TEST_F(VideoStreamEncoderTest, sink_.WaitForEncodedFrame(ntp_time); ntp_time += 100; - video_stream_encoder_->SetSource( + video_stream_encoder_->SetSourceAndWaitForRestrictionsUpdated( &video_source_, webrtc::DegradationPreference::MAINTAIN_FRAMERATE); // Give the encoder queue time to process the change in degradation preference // by waiting for an encoded frame. @@ -2031,7 +2032,7 @@ TEST_F(VideoStreamEncoderTest, EXPECT_EQ(video_source_.sink_wants().max_framerate_fps, kInputFps); // Change the degradation preference back. CPU underuse should now adapt. - video_stream_encoder_->SetSource( + video_stream_encoder_->SetSourceAndWaitForRestrictionsUpdated( &video_source_, webrtc::DegradationPreference::MAINTAIN_RESOLUTION); video_source_.IncomingCapturedFrame( CreateFrame(ntp_time, kFrameWidth, kFrameHeight)); @@ -2949,7 +2950,7 @@ TEST_F(VideoStreamEncoderTest, // Enable MAINTAIN_RESOLUTION preference. test::FrameForwarder new_video_source; - video_stream_encoder_->SetSource( + video_stream_encoder_->SetSourceAndWaitForRestrictionsUpdated( &new_video_source, webrtc::DegradationPreference::MAINTAIN_RESOLUTION); // Give the encoder queue time to process the change in degradation preference // by waiting for an encoded frame. @@ -3206,7 +3207,7 @@ TEST_F(VideoStreamEncoderTest, DropFirstFramesIfBwEstimateIsTooLow) { int64_t timestamp_ms = kFrameIntervalMs; source.IncomingCapturedFrame(CreateFrame(timestamp_ms, 1280, 720)); ExpectDroppedFrame(); - VerifyFpsMaxResolutionLt(source.sink_wants(), 1280 * 720); + EXPECT_TRUE_WAIT(source.sink_wants().max_pixel_count < 1280 * 720, 5000); // Insert 720p frame. It should be downscaled and encoded. timestamp_ms += kFrameIntervalMs; @@ -4023,7 +4024,8 @@ TEST_F(VideoStreamEncoderTest, DropsFramesAndScalesWhenBitrateIsTooLow) { ExpectDroppedFrame(); // Expect the sink_wants to specify a scaled frame. - EXPECT_LT(video_source_.sink_wants().max_pixel_count, kWidth * kHeight); + EXPECT_TRUE_WAIT( + video_source_.sink_wants().max_pixel_count < kWidth * kHeight, 5000); int last_pixel_count = video_source_.sink_wants().max_pixel_count; @@ -4034,7 +4036,8 @@ TEST_F(VideoStreamEncoderTest, DropsFramesAndScalesWhenBitrateIsTooLow) { // Expect to drop this frame, the wait should time out. ExpectDroppedFrame(); - EXPECT_LT(video_source_.sink_wants().max_pixel_count, last_pixel_count); + EXPECT_TRUE_WAIT( + video_source_.sink_wants().max_pixel_count < last_pixel_count, 5000); video_stream_encoder_->Stop(); } @@ -4149,7 +4152,8 @@ TEST_F(VideoStreamEncoderTest, InitialFrameDropActivatesWhenBweDrops) { ExpectDroppedFrame(); // Expect the sink_wants to specify a scaled frame. - EXPECT_LT(video_source_.sink_wants().max_pixel_count, kWidth * kHeight); + EXPECT_TRUE_WAIT( + video_source_.sink_wants().max_pixel_count < kWidth * kHeight, 5000); video_stream_encoder_->Stop(); } @@ -4184,7 +4188,8 @@ TEST_F(VideoStreamEncoderTest, RampsUpInQualityWhenBwIsHigh) { int64_t timestamp_ms = kFrameIntervalMs; source.IncomingCapturedFrame(CreateFrame(timestamp_ms, kWidth, kHeight)); ExpectDroppedFrame(); - EXPECT_LT(source.sink_wants().max_pixel_count, kWidth * kHeight); + EXPECT_TRUE_WAIT(source.sink_wants().max_pixel_count < kWidth * kHeight, + 5000); // Increase bitrate to encoder max. video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources(