From aa9aa575db0ea5909f1b586cdca3f19045b6754d Mon Sep 17 00:00:00 2001 From: Ilya Nikolaevskiy Date: Thu, 11 Apr 2019 09:20:24 +0200 Subject: [PATCH] Disable padding for paused encoders even on reconfiguration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: None Change-Id: If5bdcd5197f82abc9d39ecacc24e58d5b92d6780 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132324 Commit-Queue: Ilya Nikolaevskiy Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/master@{#27561} --- video/video_send_stream_impl.cc | 24 ++++- video/video_send_stream_impl.h | 1 + video/video_send_stream_impl_unittest.cc | 122 ++++++++++++++++++++--- 3 files changed, 132 insertions(+), 15 deletions(-) diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc index 81d269d896..0b3d08114b 100644 --- a/video/video_send_stream_impl.cc +++ b/video/video_send_stream_impl.cc @@ -222,6 +222,7 @@ VideoSendStreamImpl::VideoSendStreamImpl( call_stats_(call_stats), transport_(transport), bitrate_allocator_(bitrate_allocator), + disable_padding_(true), max_padding_bitrate_(0), encoder_min_bitrate_bps_(0), encoder_target_rate_bps_(0), @@ -398,6 +399,7 @@ void VideoSendStreamImpl::StartupVideoSendStream() { SignalEncoderTimedOut(); } timed_out_ = true; + disable_padding_ = true; } else if (timed_out_) { SignalEncoderActive(); timed_out_ = false; @@ -490,15 +492,17 @@ void VideoSendStreamImpl::OnBitrateAllocationUpdated( void VideoSendStreamImpl::SignalEncoderActive() { RTC_DCHECK_RUN_ON(worker_queue_); - RTC_LOG(LS_INFO) << "SignalEncoderActive, Encoder is active."; - bitrate_allocator_->AddObserver(this, GetAllocationConfig()); + if (rtp_video_sender_->IsActive()) { + RTC_LOG(LS_INFO) << "SignalEncoderActive, Encoder is active."; + bitrate_allocator_->AddObserver(this, GetAllocationConfig()); + } } MediaStreamAllocationConfig VideoSendStreamImpl::GetAllocationConfig() const { return MediaStreamAllocationConfig{ static_cast(encoder_min_bitrate_bps_), encoder_max_bitrate_bps_, - static_cast(max_padding_bitrate_), + static_cast(disable_padding_ ? 0 : max_padding_bitrate_), /* priority_bitrate */ 0, !config_->suspend_below_min_bitrate, config_->track_id, @@ -584,6 +588,20 @@ EncodedImageCallback::Result VideoSendStreamImpl::OnEncodedImage( // Indicate that there still is activity going on. activity_ = true; + auto enable_padding_task = [this]() { + if (disable_padding_) { + RTC_DCHECK_RUN_ON(worker_queue_); + disable_padding_ = false; + // To ensure that padding bitrate is propagated to the bitrate allocator. + SignalEncoderActive(); + } + }; + if (!worker_queue_->IsCurrent()) { + worker_queue_->PostTask(enable_padding_task); + } else { + enable_padding_task(); + } + EncodedImageCallback::Result result(EncodedImageCallback::Result::OK); if (media_transport_) { int64_t frame_id; diff --git a/video/video_send_stream_impl.h b/video/video_send_stream_impl.h index f9393d8802..c45157697b 100644 --- a/video/video_send_stream_impl.h +++ b/video/video_send_stream_impl.h @@ -163,6 +163,7 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, rtc::CriticalSection ivf_writers_crit_; + bool disable_padding_; int max_padding_bitrate_; int encoder_min_bitrate_bps_; uint32_t encoder_max_bitrate_bps_; diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc index cae3b86cc0..8126835000 100644 --- a/video/video_send_stream_impl_unittest.cc +++ b/video/video_send_stream_impl_unittest.cc @@ -213,16 +213,18 @@ TEST_F(VideoSendStreamImplTest, UpdatesObserverOnConfigurationChange) { config_.rtp.ssrcs.emplace_back(2); EXPECT_CALL(bitrate_allocator_, AddObserver(vss_impl.get(), _)) - .WillOnce(Invoke( + .WillRepeatedly(Invoke( [&](BitrateAllocatorObserver*, MediaStreamAllocationConfig config) { EXPECT_EQ(config.min_bitrate_bps, static_cast(min_transmit_bitrate_bps)); EXPECT_EQ(config.max_bitrate_bps, static_cast(qvga_stream.max_bitrate_bps + vga_stream.max_bitrate_bps)); - EXPECT_EQ(config.pad_up_bitrate_bps, - static_cast(qvga_stream.target_bitrate_bps + - vga_stream.min_bitrate_bps)); + if (config.pad_up_bitrate_bps != 0) { + EXPECT_EQ(config.pad_up_bitrate_bps, + static_cast(qvga_stream.target_bitrate_bps + + vga_stream.min_bitrate_bps)); + } EXPECT_EQ(config.enforce_min_bitrate, !kSuspend); })); @@ -279,15 +281,17 @@ TEST_F(VideoSendStreamImplTest, UpdatesObserverOnConfigurationChangeWithAlr) { config_.rtp.ssrcs.emplace_back(2); EXPECT_CALL(bitrate_allocator_, AddObserver(vss_impl.get(), _)) - .WillOnce(Invoke( + .WillRepeatedly(Invoke( [&](BitrateAllocatorObserver*, MediaStreamAllocationConfig config) { EXPECT_EQ(config.min_bitrate_bps, static_cast(low_stream.min_bitrate_bps)); EXPECT_EQ(config.max_bitrate_bps, static_cast(low_stream.max_bitrate_bps + high_stream.max_bitrate_bps)); - EXPECT_EQ(config.pad_up_bitrate_bps, - static_cast(min_transmit_bitrate_bps)); + if (config.pad_up_bitrate_bps != 0) { + EXPECT_EQ(config.pad_up_bitrate_bps, + static_cast(min_transmit_bitrate_bps)); + } EXPECT_EQ(config.enforce_min_bitrate, !kSuspend); })); @@ -335,16 +339,19 @@ TEST_F(VideoSendStreamImplTest, config_.rtp.ssrcs.emplace_back(2); EXPECT_CALL(bitrate_allocator_, AddObserver(vss_impl.get(), _)) - .WillOnce(Invoke([&](BitrateAllocatorObserver*, - MediaStreamAllocationConfig config) { + .WillRepeatedly(Invoke([&](BitrateAllocatorObserver*, + MediaStreamAllocationConfig config) { EXPECT_EQ(config.min_bitrate_bps, static_cast(low_stream.min_bitrate_bps)); EXPECT_EQ(config.max_bitrate_bps, static_cast(low_stream.max_bitrate_bps + high_stream.max_bitrate_bps)); - EXPECT_EQ(config.pad_up_bitrate_bps, - static_cast(low_stream.target_bitrate_bps + - 1.25 * high_stream.min_bitrate_bps)); + if (config.pad_up_bitrate_bps != 0) { + EXPECT_EQ( + config.pad_up_bitrate_bps, + static_cast(low_stream.target_bitrate_bps + + 1.25 * high_stream.min_bitrate_bps)); + } })); static_cast(vss_impl.get()) @@ -726,5 +733,96 @@ TEST_F(VideoSendStreamImplTest, CallsVideoStreamEncoderOnBitrateUpdate) { }); } +TEST_F(VideoSendStreamImplTest, DisablesPaddingOnPausedEncoder) { + int padding_bitrate = 0; + std::unique_ptr vss_impl; + + test_queue_.SendTask([&] { + vss_impl = CreateVideoSendStreamImpl( + kDefaultInitialBitrateBps, kDefaultBitratePriority, + VideoEncoderConfig::ContentType::kRealtimeVideo); + + // Capture padding bitrate for testing. + EXPECT_CALL(bitrate_allocator_, AddObserver(vss_impl.get(), _)) + .WillRepeatedly(Invoke( + [&](BitrateAllocatorObserver*, MediaStreamAllocationConfig config) { + padding_bitrate = config.pad_up_bitrate_bps; + })); + // If observer is removed, no padding will be sent. + EXPECT_CALL(bitrate_allocator_, RemoveObserver(vss_impl.get())) + .WillRepeatedly( + Invoke([&](BitrateAllocatorObserver*) { padding_bitrate = 0; })); + + EXPECT_CALL(rtp_video_sender_, OnEncodedImage(_, _, _)) + .WillRepeatedly(Return( + EncodedImageCallback::Result(EncodedImageCallback::Result::OK))); + + config_.track_id = "test"; + const bool kSuspend = false; + config_.suspend_below_min_bitrate = kSuspend; + config_.rtp.extensions.emplace_back( + RtpExtension::kTransportSequenceNumberUri, 1); + 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; + + config_.rtp.ssrcs.emplace_back(1); + + vss_impl->Start(); + + // Starts without padding. + EXPECT_EQ(0, padding_bitrate); + + // Reconfigure e.g. due to a fake frame. + static_cast(vss_impl.get()) + ->OnEncoderConfigurationChanged( + std::vector{qvga_stream}, + VideoEncoderConfig::ContentType::kRealtimeVideo, + min_transmit_bitrate_bps); + // Still no padding because no actual frames were passed, only + // reconfiguration happened. + EXPECT_EQ(0, padding_bitrate); + + // Unpause encoder. + const uint32_t kBitrateBps = 100000; + EXPECT_CALL(rtp_video_sender_, GetPayloadBitrateBps()) + .Times(1) + .WillOnce(Return(kBitrateBps)); + static_cast(vss_impl.get()) + ->OnBitrateUpdated(CreateAllocation(kBitrateBps)); + + // A frame is encoded. + EncodedImage encoded_image; + CodecSpecificInfo codec_specific; + static_cast(vss_impl.get()) + ->OnEncodedImage(encoded_image, &codec_specific, nullptr); + // Only after actual frame is encoded are we enabling the padding. + EXPECT_GT(padding_bitrate, 0); + }); + + rtc::Event done; + test_queue_.PostDelayedTask( + [&] { + // No padding supposed to be sent for paused observer + EXPECT_EQ(0, padding_bitrate); + testing::Mock::VerifyAndClearExpectations(&bitrate_allocator_); + vss_impl->Stop(); + vss_impl.reset(); + done.Set(); + }, + 5000); + + // Pause the test suite so that the last delayed task executes. + ASSERT_TRUE(done.Wait(10000)); +} + } // namespace internal } // namespace webrtc