From 6b2cec17c2f1ffbb352f431419c09fbed67955b6 Mon Sep 17 00:00:00 2001 From: Sergey Silkin Date: Fri, 9 Aug 2019 16:04:05 +0200 Subject: [PATCH] Use recommended min bitrate limit provided by encoder. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also: - DCHECK that bitrate limits recommended by encoder are reasonable. - Restrict target bitrate such that it never exceed the max bitrate. Bug: webrtc:10853 Change-Id: Ie43d30a7acfc8fa115deffd94165844248ce7945 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/148442 Commit-Queue: Sergey Silkin Reviewed-by: Åsa Persson Cr-Commit-Position: refs/heads/master@{#28822} --- video/video_stream_encoder.cc | 22 +++- video/video_stream_encoder_unittest.cc | 159 +++++++++++++++++++++---- 2 files changed, 152 insertions(+), 29 deletions(-) diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 2bf2d5b248..5d82434d7e 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -682,6 +682,11 @@ GetEncoderBitrateLimits(const VideoEncoder::EncoderInfo& encoder_info, }); for (size_t i = 0; i < bitrate_limits.size(); ++i) { + RTC_DCHECK_GT(bitrate_limits[i].min_bitrate_bps, 0); + RTC_DCHECK_GE(bitrate_limits[i].min_start_bitrate_bps, + bitrate_limits[i].min_bitrate_bps); + RTC_DCHECK_GT(bitrate_limits[i].max_bitrate_bps, + bitrate_limits[i].min_start_bitrate_bps); if (i > 0) { // The bitrate limits aren't expected to decrease with resolution. RTC_DCHECK_GE(bitrate_limits[i].min_bitrate_bps, @@ -754,11 +759,18 @@ void VideoStreamEncoder::ReconfigureEncoder() { encoder_->GetEncoderInfo(), last_frame_info_->width * last_frame_info_->height); - if (encoder_config_.max_bitrate_bps <= 0 && streams.size() == 1 && - encoder_bitrate_limits_ && encoder_bitrate_limits_->max_bitrate_bps > 0) { - // If max video bitrate is not limited explicitly, set it equal to max - // bitrate recommended by encoder. - streams.back().max_bitrate_bps = encoder_bitrate_limits_->max_bitrate_bps; + if (streams.size() == 1 && encoder_bitrate_limits_) { + // Use bitrate limits recommended by encoder only if app didn't set any of + // them. + if (encoder_config_.max_bitrate_bps <= 0 && + (encoder_config_.simulcast_layers.empty() || + encoder_config_.simulcast_layers[0].min_bitrate_bps <= 0)) { + streams.back().min_bitrate_bps = encoder_bitrate_limits_->min_bitrate_bps; + streams.back().max_bitrate_bps = encoder_bitrate_limits_->max_bitrate_bps; + streams.back().target_bitrate_bps = + std::min(streams.back().target_bitrate_bps, + encoder_bitrate_limits_->max_bitrate_bps); + } } VideoCodec codec; diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index fe059c2d12..04a8802559 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -1342,50 +1342,106 @@ TEST_F(VideoStreamEncoderTest, BitrateLimitsChangeReconfigureRateAllocator) { } TEST_F(VideoStreamEncoderTest, - EncoderConfigMaxBitrateOverridesMaxBitrateRecommendedByEncoder) { + EncoderRecommendedBitrateLimitsDoNotOverrideAppBitrateLimits) { video_stream_encoder_->OnBitrateUpdated( DataRate::bps(kTargetBitrateBps), DataRate::bps(kTargetBitrateBps), 0, 0); - const VideoEncoder::ResolutionBitrateLimits encoder_bitrate_limits( - codec_width_ * codec_height_, 0, 0, kTargetBitrateBps + 123 * 1000); - fake_encoder_.SetResolutionBitrateLimits({encoder_bitrate_limits}); - VideoEncoderConfig video_encoder_config; test::FillEncoderConfiguration(kVideoCodecVP8, 1, &video_encoder_config); video_encoder_config.max_bitrate_bps = 0; + video_encoder_config.simulcast_layers[0].min_bitrate_bps = 0; video_stream_encoder_->ConfigureEncoder(video_encoder_config.Copy(), kMaxPayloadLength); - video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); + video_source_.IncomingCapturedFrame(CreateFrame(1, 360, 180)); WaitForEncodedFrame(1); - // VideoEncoderConfig::max_bitrate_bps is set to 0 - the max bitrate - // recommended by encoder should be used. - EXPECT_EQ(static_cast(encoder_bitrate_limits.max_bitrate_bps), - bitrate_allocator_factory_.codec_config().maxBitrate * 1000); - video_encoder_config.max_bitrate_bps = kTargetBitrateBps; + // Get the default bitrate limits and use them as baseline for custom + // application and encoder recommended limits. + const uint32_t kDefaultMinBitrateKbps = + bitrate_allocator_factory_.codec_config().minBitrate; + const uint32_t kDefaultMaxBitrateKbps = + bitrate_allocator_factory_.codec_config().maxBitrate; + const uint32_t kEncMinBitrateKbps = kDefaultMinBitrateKbps * 2; + const uint32_t kEncMaxBitrateKbps = kDefaultMaxBitrateKbps * 2; + const uint32_t kAppMinBitrateKbps = kDefaultMinBitrateKbps * 3; + const uint32_t kAppMaxBitrateKbps = kDefaultMaxBitrateKbps * 3; + + const VideoEncoder::ResolutionBitrateLimits encoder_bitrate_limits( + codec_width_ * codec_height_, kEncMinBitrateKbps * 1000, + kEncMinBitrateKbps * 1000, kEncMaxBitrateKbps * 1000); + fake_encoder_.SetResolutionBitrateLimits({encoder_bitrate_limits}); + + // Change resolution. This will trigger encoder re-configuration and video + // stream encoder will pick up the bitrate limits recommended by encoder. + video_source_.IncomingCapturedFrame(CreateFrame(2, 640, 360)); + WaitForEncodedFrame(2); + video_source_.IncomingCapturedFrame(CreateFrame(3, 360, 180)); + WaitForEncodedFrame(3); + + // App bitrate limits are not set - bitrate limits recommended by encoder + // should be used. + EXPECT_EQ(kEncMaxBitrateKbps, + bitrate_allocator_factory_.codec_config().maxBitrate); + EXPECT_EQ(kEncMinBitrateKbps, + bitrate_allocator_factory_.codec_config().minBitrate); + + video_encoder_config.max_bitrate_bps = kAppMaxBitrateKbps * 1000; + video_encoder_config.simulcast_layers[0].min_bitrate_bps = 0; + video_stream_encoder_->ConfigureEncoder(video_encoder_config.Copy(), + kMaxPayloadLength); + video_source_.IncomingCapturedFrame(CreateFrame(4, nullptr)); + WaitForEncodedFrame(4); + + // App limited the max bitrate - bitrate limits recommended by encoder should + // not be applied. + EXPECT_EQ(kAppMaxBitrateKbps, + bitrate_allocator_factory_.codec_config().maxBitrate); + EXPECT_EQ(kDefaultMinBitrateKbps, + bitrate_allocator_factory_.codec_config().minBitrate); + + video_encoder_config.max_bitrate_bps = 0; + video_encoder_config.simulcast_layers[0].min_bitrate_bps = + kAppMinBitrateKbps * 1000; + video_stream_encoder_->ConfigureEncoder(video_encoder_config.Copy(), + kMaxPayloadLength); + video_source_.IncomingCapturedFrame(CreateFrame(5, nullptr)); + WaitForEncodedFrame(5); + + // App limited the min bitrate - bitrate limits recommended by encoder should + // not be applied. + EXPECT_EQ(kDefaultMaxBitrateKbps, + bitrate_allocator_factory_.codec_config().maxBitrate); + EXPECT_EQ(kAppMinBitrateKbps, + bitrate_allocator_factory_.codec_config().minBitrate); + + video_encoder_config.max_bitrate_bps = kAppMaxBitrateKbps * 1000; + video_encoder_config.simulcast_layers[0].min_bitrate_bps = + kAppMinBitrateKbps * 1000; video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config), kMaxPayloadLength); - video_source_.IncomingCapturedFrame(CreateFrame(2, nullptr)); - WaitForEncodedFrame(2); + video_source_.IncomingCapturedFrame(CreateFrame(6, nullptr)); + WaitForEncodedFrame(6); - // When VideoEncoderConfig::max_bitrate_bps is set it should override the max - // bitrate limits recommended by encoder. - EXPECT_EQ(kTargetBitrateBps, - bitrate_allocator_factory_.codec_config().maxBitrate * 1000); + // App limited both min and max bitrates - bitrate limits recommended by + // encoder should not be applied. + EXPECT_EQ(kAppMaxBitrateKbps, + bitrate_allocator_factory_.codec_config().maxBitrate); + EXPECT_EQ(kAppMinBitrateKbps, + bitrate_allocator_factory_.codec_config().minBitrate); video_stream_encoder_->Stop(); } TEST_F(VideoStreamEncoderTest, - EncoderRecommendedMaxBitrateUsedForGivenResolution) { + EncoderRecommendedMaxAndMinBitratesUsedForGivenResolution) { video_stream_encoder_->OnBitrateUpdated( DataRate::bps(kTargetBitrateBps), DataRate::bps(kTargetBitrateBps), 0, 0); const VideoEncoder::ResolutionBitrateLimits encoder_bitrate_limits_270p( - 480 * 270, 0, 0, kTargetBitrateBps + 270 * 1000); + 480 * 270, 34 * 1000, 12 * 1000, 1234 * 1000); const VideoEncoder::ResolutionBitrateLimits encoder_bitrate_limits_360p( - 640 * 360, 0, 0, kTargetBitrateBps + 360 * 1000); + 640 * 360, 43 * 1000, 21 * 1000, 2345 * 1000); fake_encoder_.SetResolutionBitrateLimits( {encoder_bitrate_limits_270p, encoder_bitrate_limits_360p}); @@ -1395,32 +1451,42 @@ TEST_F(VideoStreamEncoderTest, video_stream_encoder_->ConfigureEncoder(video_encoder_config.Copy(), kMaxPayloadLength); - // 270p. The max bitrate limit recommended by encoder for 270p should be used. + // 270p. The bitrate limits recommended by encoder for 270p should be used. video_source_.IncomingCapturedFrame(CreateFrame(1, 480, 270)); WaitForEncodedFrame(1); + EXPECT_EQ(static_cast(encoder_bitrate_limits_270p.min_bitrate_bps), + bitrate_allocator_factory_.codec_config().minBitrate * 1000); EXPECT_EQ(static_cast(encoder_bitrate_limits_270p.max_bitrate_bps), bitrate_allocator_factory_.codec_config().maxBitrate * 1000); - // 360p. The max bitrate limit recommended by encoder for 360p should be used. + // 360p. The bitrate limits recommended by encoder for 360p should be used. video_source_.IncomingCapturedFrame(CreateFrame(2, 640, 360)); WaitForEncodedFrame(2); + EXPECT_EQ(static_cast(encoder_bitrate_limits_360p.min_bitrate_bps), + bitrate_allocator_factory_.codec_config().minBitrate * 1000); EXPECT_EQ(static_cast(encoder_bitrate_limits_360p.max_bitrate_bps), bitrate_allocator_factory_.codec_config().maxBitrate * 1000); - // Resolution between 270p and 360p. The max bitrate limit recommended by + // Resolution between 270p and 360p. The bitrate limits recommended by // encoder for 360p should be used. video_source_.IncomingCapturedFrame( CreateFrame(3, (640 + 480) / 2, (360 + 270) / 2)); WaitForEncodedFrame(3); + EXPECT_EQ(static_cast(encoder_bitrate_limits_360p.min_bitrate_bps), + bitrate_allocator_factory_.codec_config().minBitrate * 1000); EXPECT_EQ(static_cast(encoder_bitrate_limits_360p.max_bitrate_bps), bitrate_allocator_factory_.codec_config().maxBitrate * 1000); - // Resolution higher than 360p. The caps recommenended by encoder should be + // Resolution higher than 360p. The caps recommended by encoder should be // ignored. video_source_.IncomingCapturedFrame(CreateFrame(4, 960, 540)); WaitForEncodedFrame(4); + EXPECT_NE(static_cast(encoder_bitrate_limits_270p.min_bitrate_bps), + bitrate_allocator_factory_.codec_config().minBitrate * 1000); EXPECT_NE(static_cast(encoder_bitrate_limits_270p.max_bitrate_bps), bitrate_allocator_factory_.codec_config().maxBitrate * 1000); + EXPECT_NE(static_cast(encoder_bitrate_limits_360p.min_bitrate_bps), + bitrate_allocator_factory_.codec_config().minBitrate * 1000); EXPECT_NE(static_cast(encoder_bitrate_limits_360p.max_bitrate_bps), bitrate_allocator_factory_.codec_config().maxBitrate * 1000); @@ -1428,12 +1494,57 @@ TEST_F(VideoStreamEncoderTest, // for 270p should be used. video_source_.IncomingCapturedFrame(CreateFrame(5, 320, 180)); WaitForEncodedFrame(5); + EXPECT_EQ(static_cast(encoder_bitrate_limits_270p.min_bitrate_bps), + bitrate_allocator_factory_.codec_config().minBitrate * 1000); EXPECT_EQ(static_cast(encoder_bitrate_limits_270p.max_bitrate_bps), bitrate_allocator_factory_.codec_config().maxBitrate * 1000); video_stream_encoder_->Stop(); } +TEST_F(VideoStreamEncoderTest, EncoderRecommendedMaxBitrateCapsTargetBitrate) { + video_stream_encoder_->OnBitrateUpdated( + DataRate::bps(kTargetBitrateBps), DataRate::bps(kTargetBitrateBps), 0, 0); + + VideoEncoderConfig video_encoder_config; + test::FillEncoderConfiguration(kVideoCodecVP8, 1, &video_encoder_config); + video_encoder_config.max_bitrate_bps = 0; + video_stream_encoder_->ConfigureEncoder(video_encoder_config.Copy(), + kMaxPayloadLength); + + // Encode 720p frame to get the default encoder target bitrate. + video_source_.IncomingCapturedFrame(CreateFrame(1, 1280, 720)); + WaitForEncodedFrame(1); + const uint32_t kDefaultTargetBitrateFor720pKbps = + bitrate_allocator_factory_.codec_config() + .simulcastStream[0] + .targetBitrate; + + // Set the max recommended encoder bitrate to something lower than the default + // target bitrate. + const VideoEncoder::ResolutionBitrateLimits encoder_bitrate_limits( + 1280 * 720, 10 * 1000, 10 * 1000, + kDefaultTargetBitrateFor720pKbps / 2 * 1000); + fake_encoder_.SetResolutionBitrateLimits({encoder_bitrate_limits}); + + // Change resolution to trigger encoder reinitialization. + video_source_.IncomingCapturedFrame(CreateFrame(2, 640, 360)); + WaitForEncodedFrame(2); + video_source_.IncomingCapturedFrame(CreateFrame(3, 1280, 720)); + WaitForEncodedFrame(3); + + // Ensure the target bitrate is capped by the max bitrate. + EXPECT_EQ(bitrate_allocator_factory_.codec_config().maxBitrate * 1000, + static_cast(encoder_bitrate_limits.max_bitrate_bps)); + EXPECT_EQ(bitrate_allocator_factory_.codec_config() + .simulcastStream[0] + .targetBitrate * + 1000, + static_cast(encoder_bitrate_limits.max_bitrate_bps)); + + video_stream_encoder_->Stop(); +} + TEST_F(VideoStreamEncoderTest, SwitchSourceDeregisterEncoderAsSink) { EXPECT_TRUE(video_source_.has_sinks()); test::FrameForwarder new_video_source;