diff --git a/pc/peer_connection_simulcast_unittest.cc b/pc/peer_connection_simulcast_unittest.cc index 2351fe1ab1..af0c8c0f71 100644 --- a/pc/peer_connection_simulcast_unittest.cc +++ b/pc/peer_connection_simulcast_unittest.cc @@ -166,7 +166,7 @@ const webrtc::RTCOutboundRTPStreamStats* FindOutboundRtpByRid( namespace webrtc { constexpr TimeDelta kDefaultTimeout = TimeDelta::Seconds(5); -constexpr TimeDelta kLongTimeoutForRampingUp = TimeDelta::Seconds(20); +constexpr TimeDelta kLongTimeoutForRampingUp = TimeDelta::Seconds(30); class PeerConnectionSimulcastTests : public ::testing::Test { public: @@ -977,7 +977,8 @@ class PeerConnectionSimulcastWithMediaFlowTests bool HasOutboundRtpExpectedResolutions( rtc::scoped_refptr pc_wrapper, - std::vector resolutions) { + std::vector resolutions, + bool log_during_ramp_up) { rtc::scoped_refptr report = GetStats(pc_wrapper); std::vector outbound_rtps = report->GetStatsOfType(); @@ -995,12 +996,14 @@ class PeerConnectionSimulcastWithMediaFlowTests EXPECT_THAT(*outbound_rtp->frame_height, Le(resolution.height)); if (*outbound_rtp->frame_width != resolution.width || *outbound_rtp->frame_height != resolution.height) { - // Useful logging for debugging. - RTC_LOG(LS_ERROR) << "rid=" << resolution.rid << " is " - << *outbound_rtp->frame_width << "x" - << *outbound_rtp->frame_height << " (want " - << resolution.width << "x" << resolution.height - << ")"; + if (log_during_ramp_up) { + // Useful logging for debugging. + RTC_LOG(LS_ERROR) + << "rid=" << resolution.rid << " is " + << *outbound_rtp->frame_width << "x" + << *outbound_rtp->frame_height << " (want " << resolution.width + << "x" << resolution.height << ")"; + } return false; } } @@ -1083,11 +1086,15 @@ TEST_F(PeerConnectionSimulcastWithMediaFlowTests, remote_pc_wrapper->WaitForConnection(); // Wait until media is flowing on all three layers. + // Ramp up time is needed before all three layers are sending. EXPECT_TRUE_WAIT(HasOutboundRtpBytesSent(local_pc_wrapper, 3u), kLongTimeoutForRampingUp.ms()); + // No significant additional ramp up time should be needed so we use + // `kDefaultTimeout` and `log_during_ramp_up`. EXPECT_TRUE_WAIT(HasOutboundRtpExpectedResolutions( local_pc_wrapper, - {{"f", 320, 180}, {"h", 640, 360}, {"q", 1280, 720}}), + {{"f", 320, 180}, {"h", 640, 360}, {"q", 1280, 720}}, + /*log_during_ramp_up=*/true), kDefaultTimeout.ms()); // Verify codec and scalability mode. rtc::scoped_refptr report = GetStats(local_pc_wrapper); @@ -1130,15 +1137,15 @@ TEST_F(PeerConnectionSimulcastWithMediaFlowTests, remote_pc_wrapper->WaitForConnection(); // Wait until media is flowing. We only expect a single RTP stream. + // We expect to see bytes flowing almost immediately on the lowest layer. EXPECT_TRUE_WAIT(HasOutboundRtpBytesSent(local_pc_wrapper, 1u), - kLongTimeoutForRampingUp.ms()); - // It takes a long time for SVC to ramp up. To avoid risk of flakes on slow - // machines this is commented out, but we do expect this to pass. - // TODO(https://crbug.com/webrtc/14884): See if we can enable this EXPECT line - // in a standalone CL in case it gets reverted. - // EXPECT_TRUE_WAIT(HasOutboundRtpExpectedResolutions(local_pc_wrapper, - // {{"f", 1280, 720}}), - // kLongTimeoutForRampingUp.ms()); + kDefaultTimeout.ms()); + // Significant ramp up time is needed until maximum resolution is achieved so + // we disable `log_during_ramp_up` to avoid log spam. + EXPECT_TRUE_WAIT( + HasOutboundRtpExpectedResolutions(local_pc_wrapper, {{"f", 1280, 720}}, + /*log_during_ramp_up=*/false), + kLongTimeoutForRampingUp.ms()); // Verify codec and scalability mode. rtc::scoped_refptr report = GetStats(local_pc_wrapper); std::vector outbound_rtps =