From 28efb5acb4ef69bc043a07a698e3c7c9af174e2e Mon Sep 17 00:00:00 2001 From: Jeremy Leconte Date: Thu, 21 Mar 2024 17:25:39 +0100 Subject: [PATCH] DoesUtilizeUlpfecForVp9WithNackEnabled is flaky. Try to increase the timeout to see if it solves the flakiness issue. If it doesn't work we should temporary disable this test. Change-Id: I8ecf3721cb5f7f4c647c8cbf247740c89c72ab82 Bug: webrtc:15885 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343982 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Jeremy Leconte Cr-Commit-Position: refs/heads/main@{#41946} --- video/video_send_stream_tests.cc | 35 ++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index c46529d8da..671b89d1c2 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -108,6 +108,10 @@ enum class WaitUntil : bool { kZero = false, kNonZero = true }; constexpr int64_t kRtcpIntervalMs = 1000; +// Some of the test cases are expected to time out. +// Use a shorter timeout window than the default one for those. +constexpr TimeDelta kReducedTimeout = TimeDelta::Seconds(10); + enum VideoFormat { kGeneric, kVP8, @@ -519,18 +523,15 @@ class FakeReceiveStatistics : public ReceiveStatisticsProvider { class UlpfecObserver : public test::EndToEndTest { public: - // Some of the test cases are expected to time out. - // Use a shorter timeout window than the default one for those. - static constexpr TimeDelta kReducedTimeout = TimeDelta::Seconds(10); - - UlpfecObserver(bool header_extensions_enabled, - bool use_nack, - bool expect_red, - bool expect_ulpfec, - const std::string& codec, - VideoEncoderFactory* encoder_factory) - : EndToEndTest(expect_ulpfec ? test::VideoTestConstants::kDefaultTimeout - : kReducedTimeout), + UlpfecObserver( + bool header_extensions_enabled, + bool use_nack, + bool expect_red, + bool expect_ulpfec, + const std::string& codec, + VideoEncoderFactory* encoder_factory, + const TimeDelta& timeout = test::VideoTestConstants::kDefaultTimeout) + : EndToEndTest(timeout), encoder_factory_(encoder_factory), payload_name_(codec), use_nack_(use_nack), @@ -692,7 +693,8 @@ class VideoSendStreamWithoutUlpfecTest : public test::CallTest { TEST_F(VideoSendStreamWithoutUlpfecTest, NoUlpfecIfDisabledThroughFieldTrial) { test::FunctionVideoEncoderFactory encoder_factory( []() { return VP8Encoder::Create(); }); - UlpfecObserver test(false, false, false, false, "VP8", &encoder_factory); + UlpfecObserver test(false, false, false, false, "VP8", &encoder_factory, + kReducedTimeout); RunBaseTest(&test); } @@ -704,7 +706,8 @@ TEST_F(VideoSendStreamTest, DoesNotUtilizeUlpfecForH264WithNackEnabled) { test::FunctionVideoEncoderFactory encoder_factory([]() { return std::make_unique(Clock::GetRealTimeClock()); }); - UlpfecObserver test(false, true, false, false, "H264", &encoder_factory); + UlpfecObserver test(false, true, false, false, "H264", &encoder_factory, + kReducedTimeout); RunBaseTest(&test); } @@ -728,7 +731,9 @@ TEST_F(VideoSendStreamTest, DoesUtilizeUlpfecForVp8WithNackEnabled) { TEST_F(VideoSendStreamTest, DoesUtilizeUlpfecForVp9WithNackEnabled) { test::FunctionVideoEncoderFactory encoder_factory( []() { return VP9Encoder::Create(); }); - UlpfecObserver test(false, true, true, true, "VP9", &encoder_factory); + // Use kLongTimeout timeout because the test is flaky with kDefaultTimeout. + UlpfecObserver test(false, true, true, true, "VP9", &encoder_factory, + test::VideoTestConstants::kLongTimeout); RunBaseTest(&test); } #endif // defined(RTC_ENABLE_VP9)