Use intersection of app and encoder bitrate limits.

Before this change, if both app and encoder provided bitrate limits,
WebRTC ignored the limits provided by encoder. Now intersection of
these sets is used.

Also changed DCHECKs in GetEncoderBitrateLimits to allow zero values
of min_bitrate_bps and min_start_bitrate_bps.

Bug: none
Change-Id: Ib8be965ea43f51013b0a0f82fd4256a372432dda
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166600
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Sergey Silkin <ssilkin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30338}
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index cf38b3b..65e81c1 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -203,8 +203,8 @@
        });
 
   for (size_t i = 0; i < bitrate_limits.size(); ++i) {
-    RTC_DCHECK_GT(bitrate_limits[i].min_bitrate_bps, 0);
-    RTC_DCHECK_GT(bitrate_limits[i].min_start_bitrate_bps, 0);
+    RTC_DCHECK_GE(bitrate_limits[i].min_bitrate_bps, 0);
+    RTC_DCHECK_GE(bitrate_limits[i].min_start_bitrate_bps, 0);
     RTC_DCHECK_GE(bitrate_limits[i].max_bitrate_bps,
                   bitrate_limits[i].min_bitrate_bps);
     if (i > 0) {
@@ -525,16 +525,45 @@
       last_frame_info_->width * last_frame_info_->height);
 
   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;
+    // Bitrate limits can be set by app (in SDP or RtpEncodingParameters) or/and
+    // can be provided by encoder. In presence of both set of limits, the final
+    // set is derived as their intersection.
+    int min_bitrate_bps;
+    if (encoder_config_.simulcast_layers.empty() ||
+        encoder_config_.simulcast_layers[0].min_bitrate_bps <= 0) {
+      min_bitrate_bps = encoder_bitrate_limits_->min_bitrate_bps;
+    } else {
+      min_bitrate_bps = std::max(encoder_bitrate_limits_->min_bitrate_bps,
+                                 streams.back().min_bitrate_bps);
+    }
+
+    int max_bitrate_bps;
+    // We don't check encoder_config_.simulcast_layers[0].max_bitrate_bps
+    // here since encoder_config_.max_bitrate_bps is derived from it (as
+    // well as from other inputs).
+    if (encoder_config_.max_bitrate_bps <= 0) {
+      max_bitrate_bps = encoder_bitrate_limits_->max_bitrate_bps;
+    } else {
+      max_bitrate_bps = std::min(encoder_bitrate_limits_->max_bitrate_bps,
+                                 streams.back().max_bitrate_bps);
+    }
+
+    if (min_bitrate_bps < max_bitrate_bps) {
+      streams.back().min_bitrate_bps = min_bitrate_bps;
+      streams.back().max_bitrate_bps = max_bitrate_bps;
       streams.back().target_bitrate_bps =
           std::min(streams.back().target_bitrate_bps,
                    encoder_bitrate_limits_->max_bitrate_bps);
+    } else {
+      RTC_LOG(LS_WARNING) << "Bitrate limits provided by encoder"
+                          << " (min="
+                          << encoder_bitrate_limits_->min_bitrate_bps
+                          << ", max="
+                          << encoder_bitrate_limits_->min_bitrate_bps
+                          << ") do not intersect with limits set by app"
+                          << " (min=" << streams.back().min_bitrate_bps
+                          << ", max=" << encoder_config_.max_bitrate_bps
+                          << "). The app bitrate limits will be used.";
     }
   }
 
diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index e5439f3..78840a5 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -1426,93 +1426,82 @@
 }
 
 TEST_F(VideoStreamEncoderTest,
-       EncoderRecommendedBitrateLimitsDoNotOverrideAppBitrateLimits) {
+       IntersectionOfEncoderAndAppBitrateLimitsUsedWhenBothProvided) {
   video_stream_encoder_->OnBitrateUpdated(
       DataRate::bps(kTargetBitrateBps), 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_encoder_config.simulcast_layers[0].min_bitrate_bps = 0;
-  video_stream_encoder_->ConfigureEncoder(video_encoder_config.Copy(),
-                                          kMaxPayloadLength);
-
-  video_source_.IncomingCapturedFrame(CreateFrame(1, 360, 180));
-  WaitForEncodedFrame(1);
-
-  // 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 uint32_t kMinEncBitrateKbps = 100;
+  const uint32_t kMaxEncBitrateKbps = 1000;
   const VideoEncoder::ResolutionBitrateLimits encoder_bitrate_limits(
-      codec_width_ * codec_height_, kEncMinBitrateKbps * 1000,
-      kEncMinBitrateKbps * 1000, kEncMaxBitrateKbps * 1000);
+      /*frame_size_pixels=*/codec_width_ * codec_height_,
+      /*min_start_bitrate_bps=*/0,
+      /*min_bitrate_bps=*/kMinEncBitrateKbps * 1000,
+      /*max_bitrate_bps=*/kMaxEncBitrateKbps * 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));
+  VideoEncoderConfig video_encoder_config;
+  test::FillEncoderConfiguration(kVideoCodecVP8, 1, &video_encoder_config);
+  video_encoder_config.max_bitrate_bps = (kMaxEncBitrateKbps + 1) * 1000;
+  video_encoder_config.simulcast_layers[0].min_bitrate_bps =
+      (kMinEncBitrateKbps + 1) * 1000;
+  video_stream_encoder_->ConfigureEncoder(video_encoder_config.Copy(),
+                                          kMaxPayloadLength);
+
+  // When both encoder and app provide bitrate limits, the intersection of
+  // provided sets should be used.
+  video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr));
+  WaitForEncodedFrame(1);
+  EXPECT_EQ(kMaxEncBitrateKbps,
+            bitrate_allocator_factory_.codec_config().maxBitrate);
+  EXPECT_EQ(kMinEncBitrateKbps + 1,
+            bitrate_allocator_factory_.codec_config().minBitrate);
+
+  video_encoder_config.max_bitrate_bps = (kMaxEncBitrateKbps - 1) * 1000;
+  video_encoder_config.simulcast_layers[0].min_bitrate_bps =
+      (kMinEncBitrateKbps - 1) * 1000;
+  video_stream_encoder_->ConfigureEncoder(video_encoder_config.Copy(),
+                                          kMaxPayloadLength);
+  video_source_.IncomingCapturedFrame(CreateFrame(2, nullptr));
   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,
+  EXPECT_EQ(kMaxEncBitrateKbps - 1,
             bitrate_allocator_factory_.codec_config().maxBitrate);
-  EXPECT_EQ(kEncMinBitrateKbps,
+  EXPECT_EQ(kMinEncBitrateKbps,
             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_->Stop();
+}
+
+TEST_F(VideoStreamEncoderTest,
+       EncoderAndAppLimitsDontIntersectEncoderLimitsIgnored) {
+  video_stream_encoder_->OnBitrateUpdated(
+      DataRate::bps(kTargetBitrateBps), DataRate::bps(kTargetBitrateBps),
+      DataRate::bps(kTargetBitrateBps), 0, 0);
+
+  const uint32_t kMinAppBitrateKbps = 100;
+  const uint32_t kMaxAppBitrateKbps = 200;
+  const uint32_t kMinEncBitrateKbps = kMaxAppBitrateKbps + 1;
+  const uint32_t kMaxEncBitrateKbps = kMaxAppBitrateKbps * 2;
+  const VideoEncoder::ResolutionBitrateLimits encoder_bitrate_limits(
+      /*frame_size_pixels=*/codec_width_ * codec_height_,
+      /*min_start_bitrate_bps=*/0,
+      /*min_bitrate_bps=*/kMinEncBitrateKbps * 1000,
+      /*max_bitrate_bps=*/kMaxEncBitrateKbps * 1000);
+  fake_encoder_.SetResolutionBitrateLimits({encoder_bitrate_limits});
+
+  VideoEncoderConfig video_encoder_config;
+  test::FillEncoderConfiguration(kVideoCodecVP8, 1, &video_encoder_config);
+  video_encoder_config.max_bitrate_bps = kMaxAppBitrateKbps * 1000;
+  video_encoder_config.simulcast_layers[0].min_bitrate_bps =
+      kMinAppBitrateKbps * 1000;
   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,
+  video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr));
+  WaitForEncodedFrame(1);
+  EXPECT_EQ(kMaxAppBitrateKbps,
             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(6, nullptr));
-  WaitForEncodedFrame(6);
-
-  // 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,
+  EXPECT_EQ(kMinAppBitrateKbps,
             bitrate_allocator_factory_.codec_config().minBitrate);
 
   video_stream_encoder_->Stop();