Use max bitrate limit recommended by encoder.

If VideoEncoderConfig::max_bitrate_bps is unset then max bitrate of
video stream is set equal to max bitrate value recommended by encoder
for given resolution via encoder capabilities (if available).

Bug: webrtc:10796
Change-Id: I7fce9afc476b794a16956e694e891faee110048e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144526
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Sergey Silkin <ssilkin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28515}

test/encoder_settings.cc

@@ -36,7 +36,17 @@ std::vector<VideoStream> CreateVideoStreams(
              DefaultVideoStreamFactory::kMaxNumberOfStreams);
 
   std::vector<VideoStream> stream_settings(encoder_config.number_of_streams);
-  int bitrate_left_bps = encoder_config.max_bitrate_bps;
+
+  int bitrate_left_bps = 0;
+  if (encoder_config.max_bitrate_bps > 0) {
+    bitrate_left_bps = encoder_config.max_bitrate_bps;
+  } else {
+    for (size_t stream_num = 0; stream_num < encoder_config.number_of_streams;
+         ++stream_num) {
+      bitrate_left_bps +=
+          DefaultVideoStreamFactory::kMaxBitratePerStream[stream_num];
+    }
+  }
 
   for (size_t i = 0; i < encoder_config.number_of_streams; ++i) {
     stream_settings[i].width =

video/video_stream_encoder.cc

@@ -662,6 +662,39 @@ void VideoStreamEncoder::ConfigureEncoderOnTaskQueue(
   }
 }
 
+static absl::optional<VideoEncoder::ResolutionBitrateLimits>
+GetEncoderBitrateLimits(const VideoEncoder::EncoderInfo& encoder_info,
+                        int frame_size_pixels) {
+  std::vector<VideoEncoder::ResolutionBitrateLimits> bitrate_limits =
+      encoder_info.resolution_bitrate_limits;
+
+  // Sort the list of bitrate limits by resolution.
+  sort(bitrate_limits.begin(), bitrate_limits.end(),
+       [](const VideoEncoder::ResolutionBitrateLimits& lhs,
+          const VideoEncoder::ResolutionBitrateLimits& rhs) {
+         return lhs.frame_size_pixels < rhs.frame_size_pixels;
+       });
+
+  for (size_t i = 0; i < bitrate_limits.size(); ++i) {
+    if (i > 0) {
+      // The bitrate limits aren't expected to decrease with resolution.
+      RTC_DCHECK_GE(bitrate_limits[i].min_bitrate_bps,
+                    bitrate_limits[i - 1].min_bitrate_bps);
+      RTC_DCHECK_GE(bitrate_limits[i].min_start_bitrate_bps,
+                    bitrate_limits[i - 1].min_start_bitrate_bps);
+      RTC_DCHECK_GE(bitrate_limits[i].max_bitrate_bps,
+                    bitrate_limits[i - 1].max_bitrate_bps);
+    }
+
+    if (bitrate_limits[i].frame_size_pixels >= frame_size_pixels) {
+      return absl::optional<VideoEncoder::ResolutionBitrateLimits>(
+          bitrate_limits[i]);
+    }
+  }
+
+  return absl::nullopt;
+}
+
 // TODO(bugs.webrtc.org/8807): Currently this always does a hard
 // reconfiguration, but this isn't always necessary. Add in logic to only update
 // the VideoBitrateAllocator and call OnEncoderConfigurationChanged with a
@@ -690,6 +723,38 @@ void VideoStreamEncoder::ReconfigureEncoder() {
   crop_width_ = last_frame_info_->width - highest_stream_width;
   crop_height_ = last_frame_info_->height - highest_stream_height;
 
+  bool encoder_reset_required = false;
+  if (pending_encoder_creation_) {
+    // Destroy existing encoder instance before creating a new one. Otherwise
+    // attempt to create another instance will fail if encoder factory
+    // supports only single instance of encoder of given type.
+    encoder_.reset();
+
+    encoder_ = settings_.encoder_factory->CreateVideoEncoder(
+        encoder_config_.video_format);
+    // TODO(nisse): What to do if creating the encoder fails? Crash,
+    // or just discard incoming frames?
+    RTC_CHECK(encoder_);
+
+    encoder_->SetFecControllerOverride(fec_controller_override_);
+
+    codec_info_ = settings_.encoder_factory->QueryVideoEncoder(
+        encoder_config_.video_format);
+
+    encoder_reset_required = true;
+  }
+
+  encoder_bitrate_limits_ = GetEncoderBitrateLimits(
+      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;
+  }
+
   VideoCodec codec;
   if (!VideoCodecInitializer::SetupCodec(encoder_config_, streams, &codec)) {
     RTC_LOG(LS_ERROR) << "Failed to create encoder configuration.";
@@ -743,8 +808,10 @@ void VideoStreamEncoder::ReconfigureEncoder() {
 
   // Reset (release existing encoder) if one exists and anything except
   // start bitrate or max framerate has changed.
-  const bool reset_required = RequiresEncoderReset(
+  if (!encoder_reset_required) {
-      codec, send_codec_, was_encode_called_since_last_initialization_);
+    encoder_reset_required = RequiresEncoderReset(
+        codec, send_codec_, was_encode_called_since_last_initialization_);
+  }
   send_codec_ = codec;
 
   // Keep the same encoder, as long as the video_format is unchanged.
@@ -752,26 +819,8 @@ void VideoStreamEncoder::ReconfigureEncoder() {
   // CPU adaptation with the correct settings should be polled after
   // encoder_->InitEncode().
   bool success = true;
-  if (pending_encoder_creation_ || reset_required) {
+  if (encoder_reset_required) {
     ReleaseEncoder();
-    if (pending_encoder_creation_) {
-      // Destroy existing encoder instance before creating a new one. Otherwise
-      // attempt to create another instance will fail if encoder factory
-      // supports only single encoder instance.
-      encoder_.reset();
-
-      encoder_ = settings_.encoder_factory->CreateVideoEncoder(
-          encoder_config_.video_format);
-      // TODO(nisse): What to do if creating the encoder fails? Crash,
-      // or just discard incoming frames?
-      RTC_CHECK(encoder_);
-
-      encoder_->SetFecControllerOverride(fec_controller_override_);
-
-      codec_info_ = settings_.encoder_factory->QueryVideoEncoder(
-          encoder_config_.video_format);
-    }
-
     const size_t max_data_payload_length = max_data_payload_length_ > 0
                                                ? max_data_payload_length_
                                                : kDefaultPayloadSize;

video/video_stream_encoder.h

@@ -335,6 +335,8 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface,
   absl::optional<int64_t> last_encode_info_ms_ RTC_GUARDED_BY(&encoder_queue_);
 
   VideoEncoder::EncoderInfo encoder_info_ RTC_GUARDED_BY(&encoder_queue_);
+  absl::optional<VideoEncoder::ResolutionBitrateLimits> encoder_bitrate_limits_
+      RTC_GUARDED_BY(&encoder_queue_);
   VideoEncoderFactory::CodecInfo codec_info_ RTC_GUARDED_BY(&encoder_queue_);
   VideoCodec send_codec_ RTC_GUARDED_BY(&encoder_queue_);
 

video/video_stream_encoder_unittest.cc

@@ -666,6 +666,8 @@ class VideoStreamEncoderTest : public ::testing::Test {
           }
         }
       }
+
+      info.resolution_bitrate_limits = resolution_bitrate_limits_;
       return info;
     }
 
@@ -701,6 +703,12 @@ class VideoStreamEncoderTest : public ::testing::Test {
       temporal_layers_supported_[spatial_idx] = supported;
     }
 
+    void SetResolutionBitrateLimits(
+        std::vector<ResolutionBitrateLimits> thresholds) {
+      rtc::CritScope cs(&local_crit_sect_);
+      resolution_bitrate_limits_ = thresholds;
+    }
+
     void ForceInitEncodeFailure(bool force_failure) {
       rtc::CritScope lock(&local_crit_sect_);
       force_init_encode_failed_ = force_failure;
@@ -882,6 +890,8 @@ class VideoStreamEncoderTest : public ::testing::Test {
         RTC_GUARDED_BY(local_crit_sect_) = nullptr;
     MockFecControllerOverride fec_controller_override_;
     int num_encoder_initializations_ RTC_GUARDED_BY(local_crit_sect_) = 0;
+    std::vector<ResolutionBitrateLimits> resolution_bitrate_limits_
+        RTC_GUARDED_BY(local_crit_sect_);
   };
 
   class TestSink : public VideoStreamEncoder::EncoderSink {
@@ -1309,8 +1319,8 @@ TEST_F(VideoStreamEncoderTest, BitrateLimitsChangeReconfigureRateAllocator) {
   EXPECT_EQ(kStartBitrateBps,
             bitrate_allocator_factory_.codec_config().startBitrate * 1000);
 
-  test::FillEncoderConfiguration(kVideoCodecVP8, 1, &video_encoder_config);
+  test::FillEncoderConfiguration(kVideoCodecVP8, 1,
-
+                                 &video_encoder_config);  //???
   video_encoder_config.max_bitrate_bps = kTargetBitrateBps * 2;
   video_stream_encoder_->SetStartBitrate(kStartBitrateBps * 2);
   video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config),
@@ -1331,6 +1341,99 @@ TEST_F(VideoStreamEncoderTest, BitrateLimitsChangeReconfigureRateAllocator) {
   video_stream_encoder_->Stop();
 }
 
+TEST_F(VideoStreamEncoderTest,
+       EncoderConfigMaxBitrateOverridesMaxBitrateRecommendedByEncoder) {
+  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_stream_encoder_->ConfigureEncoder(video_encoder_config.Copy(),
+                                          kMaxPayloadLength);
+
+  video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr));
+  WaitForEncodedFrame(1);
+  // VideoEncoderConfig::max_bitrate_bps is set to 0 - the max bitrate
+  // recommended by encoder should be used.
+  EXPECT_EQ(static_cast<uint32_t>(encoder_bitrate_limits.max_bitrate_bps),
+            bitrate_allocator_factory_.codec_config().maxBitrate * 1000);
+
+  video_encoder_config.max_bitrate_bps = kTargetBitrateBps;
+  video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config),
+                                          kMaxPayloadLength);
+  video_source_.IncomingCapturedFrame(CreateFrame(2, nullptr));
+  WaitForEncodedFrame(2);
+
+  // 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);
+
+  video_stream_encoder_->Stop();
+}
+
+TEST_F(VideoStreamEncoderTest,
+       EncoderRecommendedMaxBitrateUsedForGivenResolution) {
+  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);
+  const VideoEncoder::ResolutionBitrateLimits encoder_bitrate_limits_360p(
+      640 * 360, 0, 0, kTargetBitrateBps + 360 * 1000);
+  fake_encoder_.SetResolutionBitrateLimits(
+      {encoder_bitrate_limits_270p, encoder_bitrate_limits_360p});
+
+  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);
+
+  // 270p. The max bitrate limit recommended by encoder for 270p should be used.
+  video_source_.IncomingCapturedFrame(CreateFrame(1, 480, 270));
+  WaitForEncodedFrame(1);
+  EXPECT_EQ(static_cast<uint32_t>(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.
+  video_source_.IncomingCapturedFrame(CreateFrame(2, 640, 360));
+  WaitForEncodedFrame(2);
+  EXPECT_EQ(static_cast<uint32_t>(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
+  // encoder for 360p should be used.
+  video_source_.IncomingCapturedFrame(
+      CreateFrame(3, (640 + 480) / 2, (360 + 270) / 2));
+  WaitForEncodedFrame(3);
+  EXPECT_EQ(static_cast<uint32_t>(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
+  // ignored.
+  video_source_.IncomingCapturedFrame(CreateFrame(4, 960, 540));
+  WaitForEncodedFrame(4);
+  EXPECT_NE(static_cast<uint32_t>(encoder_bitrate_limits_270p.max_bitrate_bps),
+            bitrate_allocator_factory_.codec_config().maxBitrate * 1000);
+  EXPECT_NE(static_cast<uint32_t>(encoder_bitrate_limits_360p.max_bitrate_bps),
+            bitrate_allocator_factory_.codec_config().maxBitrate * 1000);
+
+  // Resolution lower than 270p. The max bitrate limit recommended by encoder
+  // for 270p should be used.
+  video_source_.IncomingCapturedFrame(CreateFrame(5, 320, 180));
+  WaitForEncodedFrame(5);
+  EXPECT_EQ(static_cast<uint32_t>(encoder_bitrate_limits_270p.max_bitrate_bps),
+            bitrate_allocator_factory_.codec_config().maxBitrate * 1000);
+
+  video_stream_encoder_->Stop();
+}
+
 TEST_F(VideoStreamEncoderTest, SwitchSourceDeregisterEncoderAsSink) {
   EXPECT_TRUE(video_source_.has_sinks());
   test::FrameForwarder new_video_source;