From 10814873c584df17e560462adcc2b72e488ada91 Mon Sep 17 00:00:00 2001 From: Jakob Ivarsson Date: Thu, 10 Jun 2021 14:45:21 +0200 Subject: [PATCH] Avoid video stream allocation on configuration change after timeout. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is to prevent the video stream to get in a state where it is allocated but there is no activity. Bug: b/189842675 Change-Id: I0793bd4cbf2a4faed92cf811550437ae75742102 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221618 Reviewed-by: Erik Språng Commit-Queue: Jakob Ivarsson Cr-Commit-Position: refs/heads/master@{#34276} --- video/video_send_stream_impl.cc | 2 +- video/video_send_stream_impl_unittest.cc | 53 ++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc index 3fc6b676dc..316037afa7 100644 --- a/video/video_send_stream_impl.cc +++ b/video/video_send_stream_impl.cc @@ -529,7 +529,7 @@ void VideoSendStreamImpl::OnEncoderConfigurationChanged( rtp_video_sender_->SetEncodingData(streams[0].width, streams[0].height, num_temporal_layers); - if (rtp_video_sender_->IsActive()) { + if (rtp_video_sender_->IsActive() && !timed_out_) { // The send stream is started already. Update the allocator with new bitrate // limits. bitrate_allocator_->AddObserver(this, GetAllocationConfig()); diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc index 71cec7c981..2ba5ab2c9a 100644 --- a/video/video_send_stream_impl_unittest.cc +++ b/video/video_send_stream_impl_unittest.cc @@ -1038,5 +1038,58 @@ TEST_F(VideoSendStreamImplTest, ConfiguresBitratesForSvc) { RTC_FROM_HERE); } } + +// Check that a timed out stream is not allocated by doing the following: +// 1. Allocate a video stream. +// 2. Wait for the stream to time out which de-allocates the stream. +// 3. Reconfigure the stream and make sure that this does not allocate the +// stream again. +TEST_F(VideoSendStreamImplTest, AvoidAllocationAfterTimeout) { + std::unique_ptr vss_impl = CreateVideoSendStreamImpl( + kDefaultInitialBitrateBps, kDefaultBitratePriority, + VideoEncoderConfig::ContentType::kRealtimeVideo); + + test_queue_.SendTask( + [&] { + EXPECT_CALL(bitrate_allocator_, AddObserver(vss_impl.get(), _)); + vss_impl->Start(); + const uint32_t kBitrateBps = 100000; + EXPECT_CALL(rtp_video_sender_, GetPayloadBitrateBps()) + .WillOnce(Return(kBitrateBps)); + static_cast(vss_impl.get()) + ->OnBitrateUpdated(CreateAllocation(kBitrateBps)); + + // The stream should be de-allocated after a timeout. + EXPECT_CALL(bitrate_allocator_, RemoveObserver(vss_impl.get())); + }, + RTC_FROM_HERE); + + rtc::Event done; + test_queue_.PostDelayedTask( + [&] { + VideoStream qvga_stream; + qvga_stream.width = 320; + qvga_stream.height = 180; + qvga_stream.max_framerate = 30; + qvga_stream.min_bitrate_bps = 30000; + qvga_stream.target_bitrate_bps = 150000; + qvga_stream.max_bitrate_bps = 200000; + qvga_stream.max_qp = 56; + qvga_stream.bitrate_priority = 1; + + int min_transmit_bitrate_bps = 30000; + // This should not allocate the stream since it has timed out. + static_cast(vss_impl.get()) + ->OnEncoderConfigurationChanged( + std::vector{qvga_stream}, false, + VideoEncoderConfig::ContentType::kRealtimeVideo, + min_transmit_bitrate_bps); + testing::Mock::VerifyAndClearExpectations(&bitrate_allocator_); + vss_impl->Stop(); + done.Set(); + }, + 2000); + ASSERT_TRUE(done.Wait(5000)); +} } // namespace internal } // namespace webrtc