From 573f5461970d63a7e5c79541521553012416c415 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Fri, 17 Feb 2023 15:12:22 +0100 Subject: [PATCH] Fix max bitrate not being resptected with some HW codecs. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes the max bitrate configured in RTP parameter to still be respected, instead of being overridden by defaults in the case that an encoder with untrusted QP is reconfigured (e.g. due to some adaptation). Bug: webrtc:14914 Change-Id: I2d3ff645c069b80ec2e36887e6ce0ecd09a7ecbf Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/293944 Reviewed-by: Philip Eliasson Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/main@{#39357} --- video/video_stream_encoder.cc | 18 ++++--- video/video_stream_encoder_unittest.cc | 67 +++++++++++++++++++++----- 2 files changed, 66 insertions(+), 19 deletions(-) diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index e5d6df396d..65aa0760c5 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -1063,19 +1063,21 @@ void VideoStreamEncoder::ReconfigureEncoder() { if (qp_untrusted_bitrate_limit) { // bandwidth_quality_scaler is only used for singlecast. if (streams.size() == 1 && encoder_config_.simulcast_layers.size() == 1) { - streams.back().min_bitrate_bps = - qp_untrusted_bitrate_limit->min_bitrate_bps; - streams.back().max_bitrate_bps = - qp_untrusted_bitrate_limit->max_bitrate_bps; + VideoStream& stream = streams.back(); + stream.max_bitrate_bps = + std::min(stream.max_bitrate_bps, + qp_untrusted_bitrate_limit->max_bitrate_bps); + stream.min_bitrate_bps = + std::min(stream.max_bitrate_bps, + qp_untrusted_bitrate_limit->min_bitrate_bps); // If it is screen share mode, the minimum value of max_bitrate should // be greater than/equal to 1200kbps. if (encoder_config_.content_type == VideoEncoderConfig::ContentType::kScreen) { - streams.back().max_bitrate_bps = std::max( - streams.back().max_bitrate_bps, kDefaultMinScreenSharebps); + stream.max_bitrate_bps = + std::max(stream.max_bitrate_bps, kDefaultMinScreenSharebps); } - streams.back().target_bitrate_bps = - qp_untrusted_bitrate_limit->max_bitrate_bps; + stream.target_bitrate_bps = stream.max_bitrate_bps; } } } else { diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index cdd4c75ab7..908c96fc43 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -8384,7 +8384,7 @@ TEST_F(VideoStreamEncoderTest, EncoderProvideLimitsWhenQPIsNotTrusted) { } TEST_F(VideoStreamEncoderTest, EncoderDoesnotProvideLimitsWhenQPIsNotTrusted) { - // Set QP trusted in encoder info. + // Set QP not trusted in encoder info. fake_encoder_.SetIsQpTrusted(false); absl::optional suitable_bitrate_limit = @@ -8395,28 +8395,73 @@ TEST_F(VideoStreamEncoderTest, EncoderDoesnotProvideLimitsWhenQPIsNotTrusted) { GetDefaultSinglecastBitrateLimitsWhenQpIsUntrusted()); EXPECT_TRUE(suitable_bitrate_limit.has_value()); - const int MaxEncBitrate = suitable_bitrate_limit->max_bitrate_bps; - const int MinEncBitrate = suitable_bitrate_limit->min_bitrate_bps; - const int TargetEncBitrate = MaxEncBitrate; + const int max_encoder_bitrate = suitable_bitrate_limit->max_bitrate_bps; + const int min_encoder_bitrate = suitable_bitrate_limit->min_bitrate_bps; + const int target_encoder_bitrate = max_encoder_bitrate; video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources( - DataRate::BitsPerSec(TargetEncBitrate), - DataRate::BitsPerSec(TargetEncBitrate), - DataRate::BitsPerSec(TargetEncBitrate), 0, 0, 0); + DataRate::BitsPerSec(target_encoder_bitrate), + DataRate::BitsPerSec(target_encoder_bitrate), + DataRate::BitsPerSec(target_encoder_bitrate), 0, 0, 0); VideoEncoderConfig video_encoder_config; test::FillEncoderConfiguration(kVideoCodecH264, 1, &video_encoder_config); - video_encoder_config.max_bitrate_bps = MaxEncBitrate; - video_encoder_config.simulcast_layers[0].min_bitrate_bps = MinEncBitrate; + video_encoder_config.max_bitrate_bps = max_encoder_bitrate; + video_encoder_config.simulcast_layers[0].min_bitrate_bps = + min_encoder_bitrate; video_stream_encoder_->ConfigureEncoder(video_encoder_config.Copy(), kMaxPayloadLength); video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); WaitForEncodedFrame(1); EXPECT_EQ( - MaxEncBitrate / 1000, + max_encoder_bitrate / 1000, static_cast(bitrate_allocator_factory_.codec_config().maxBitrate)); EXPECT_EQ( - MinEncBitrate / 1000, + min_encoder_bitrate / 1000, + static_cast(bitrate_allocator_factory_.codec_config().minBitrate)); + + video_stream_encoder_->Stop(); +} + +TEST_F(VideoStreamEncoderTest, + RespectsLowerThanDefaultMinBitrateWhenQPIsNotTrusted) { + // Set QP not trusted in encoder info. + fake_encoder_.SetIsQpTrusted(false); + + absl::optional suitable_bitrate_limit = + EncoderInfoSettings:: + GetSinglecastBitrateLimitForResolutionWhenQpIsUntrusted( + codec_width_ * codec_height_, + EncoderInfoSettings:: + GetDefaultSinglecastBitrateLimitsWhenQpIsUntrusted()); + EXPECT_TRUE(suitable_bitrate_limit.has_value()); + + const int max_encoder_bitrate = suitable_bitrate_limit->max_bitrate_bps; + const int min_encoder_bitrate = suitable_bitrate_limit->min_bitrate_bps; + const int target_encoder_bitrate = max_encoder_bitrate; + video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources( + DataRate::BitsPerSec(target_encoder_bitrate), + DataRate::BitsPerSec(target_encoder_bitrate), + DataRate::BitsPerSec(target_encoder_bitrate), 0, 0, 0); + + VideoEncoderConfig video_encoder_config; + test::FillEncoderConfiguration(kVideoCodecH264, 1, &video_encoder_config); + // Set the max bitrate config to be lower than what the resolution bitrate + // limits would suggest for the current resolution. + video_encoder_config.max_bitrate_bps = + (max_encoder_bitrate + min_encoder_bitrate) / 2; + video_encoder_config.simulcast_layers[0].min_bitrate_bps = + min_encoder_bitrate; + video_stream_encoder_->ConfigureEncoder(video_encoder_config.Copy(), + kMaxPayloadLength); + + video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); + WaitForEncodedFrame(1); + EXPECT_EQ( + video_encoder_config.max_bitrate_bps / 1000, + static_cast(bitrate_allocator_factory_.codec_config().maxBitrate)); + EXPECT_EQ( + min_encoder_bitrate / 1000, static_cast(bitrate_allocator_factory_.codec_config().minBitrate)); video_stream_encoder_->Stop();