Disable padding for paused encoders even on reconfiguration
Bug: None
Change-Id: If5bdcd5197f82abc9d39ecacc24e58d5b92d6780
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132324
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27561}diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc
index 81d269d..0b3d081 100644
--- a/video/video_send_stream_impl.cc
+++ b/video/video_send_stream_impl.cc
@@ -222,6 +222,7 @@
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 @@
SignalEncoderTimedOut();
}
timed_out_ = true;
+ disable_padding_ = true;
} else if (timed_out_) {
SignalEncoderActive();
timed_out_ = false;
@@ -490,15 +492,17 @@
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<uint32_t>(encoder_min_bitrate_bps_),
encoder_max_bitrate_bps_,
- static_cast<uint32_t>(max_padding_bitrate_),
+ static_cast<uint32_t>(disable_padding_ ? 0 : max_padding_bitrate_),
/* priority_bitrate */ 0,
!config_->suspend_below_min_bitrate,
config_->track_id,
@@ -584,6 +588,20 @@
// 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 f9393d8..c451576 100644
--- a/video/video_send_stream_impl.h
+++ b/video/video_send_stream_impl.h
@@ -163,6 +163,7 @@
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 cae3b86..8126835 100644
--- a/video/video_send_stream_impl_unittest.cc
+++ b/video/video_send_stream_impl_unittest.cc
@@ -213,16 +213,18 @@
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<uint32_t>(min_transmit_bitrate_bps));
EXPECT_EQ(config.max_bitrate_bps,
static_cast<uint32_t>(qvga_stream.max_bitrate_bps +
vga_stream.max_bitrate_bps));
- EXPECT_EQ(config.pad_up_bitrate_bps,
- static_cast<uint32_t>(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<uint32_t>(qvga_stream.target_bitrate_bps +
+ vga_stream.min_bitrate_bps));
+ }
EXPECT_EQ(config.enforce_min_bitrate, !kSuspend);
}));
@@ -279,15 +281,17 @@
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<uint32_t>(low_stream.min_bitrate_bps));
EXPECT_EQ(config.max_bitrate_bps,
static_cast<uint32_t>(low_stream.max_bitrate_bps +
high_stream.max_bitrate_bps));
- EXPECT_EQ(config.pad_up_bitrate_bps,
- static_cast<uint32_t>(min_transmit_bitrate_bps));
+ if (config.pad_up_bitrate_bps != 0) {
+ EXPECT_EQ(config.pad_up_bitrate_bps,
+ static_cast<uint32_t>(min_transmit_bitrate_bps));
+ }
EXPECT_EQ(config.enforce_min_bitrate, !kSuspend);
}));
@@ -335,16 +339,19 @@
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<uint32_t>(low_stream.min_bitrate_bps));
EXPECT_EQ(config.max_bitrate_bps,
static_cast<uint32_t>(low_stream.max_bitrate_bps +
high_stream.max_bitrate_bps));
- EXPECT_EQ(config.pad_up_bitrate_bps,
- static_cast<uint32_t>(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<uint32_t>(low_stream.target_bitrate_bps +
+ 1.25 * high_stream.min_bitrate_bps));
+ }
}));
static_cast<VideoStreamEncoderInterface::EncoderSink*>(vss_impl.get())
@@ -726,5 +733,96 @@
});
}
+TEST_F(VideoSendStreamImplTest, DisablesPaddingOnPausedEncoder) {
+ int padding_bitrate = 0;
+ std::unique_ptr<VideoSendStreamImpl> 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<VideoStreamEncoderInterface::EncoderSink*>(vss_impl.get())
+ ->OnEncoderConfigurationChanged(
+ std::vector<VideoStream>{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<BitrateAllocatorObserver*>(vss_impl.get())
+ ->OnBitrateUpdated(CreateAllocation(kBitrateBps));
+
+ // A frame is encoded.
+ EncodedImage encoded_image;
+ CodecSpecificInfo codec_specific;
+ static_cast<EncodedImageCallback*>(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
