Prevent resolution limited max bitrate from going below min

When simulcast screenshare is enabled, the max bitrate for the high quality stream can be limited based on the resolution. This CL fixes a bug where that limit could get so low that it is below the min bitrate of the top layer, which in turn could cause the encoder to fail initialization. Bug: webrtc:9761 Change-Id: I093bd0ba68fe0165e8982d169daf02cdf912c924 Reviewed-on: https://webrtc-review.googlesource.com/100682 Reviewed-by: Ilya Nikolaevskiy <[email protected]> Commit-Queue: Erik Språng <[email protected]> Cr-Commit-Position: refs/heads/master@{#24767}
JamesTerm · Sep 18, 2018 · cf91942 · cf91942
1 parent 32ce18c
commit cf91942
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 7 deletions.
diff --git a/media/engine/simulcast.cc b/media/engine/simulcast.cc
@@ -329,6 +329,7 @@ std::vector<webrtc::VideoStream> GetScreenshareLayers(
     // Add optional upper simulcast layer.
     const int num_temporal_layers = DefaultNumberOfTemporalLayers(1, true);
     int max_bitrate_bps;
+    bool using_boosted_bitrate = false;
     if (!temporal_layers_supported) {
       // Set the max bitrate to where the base layer would have been if temporal
       // layers were enabled.
@@ -340,6 +341,7 @@ std::vector<webrtc::VideoStream> GetScreenshareLayers(
                webrtc::field_trial::IsEnabled("WebRTC-UseShortVP8TL3Pattern")) {
       // Experimental temporal layer mode used, use increased max bitrate.
       max_bitrate_bps = kScreenshareHighStreamMaxBitrateBps;
+      using_boosted_bitrate = true;
     } else {
       // Keep current bitrates with default 3tl/8 frame settings.
       // Lowest temporal layers of a 3 layer setup will have 40% of the total
@@ -350,19 +352,22 @@ std::vector<webrtc::VideoStream> GetScreenshareLayers(
       max_bitrate_bps = 2 * ((layers[0].target_bitrate_bps * 10) / 4);
     }
 
-    // Cap max bitrate so it isn't overly high for the given resolution.
-    max_bitrate_bps = std::min<int>(max_bitrate_bps,
-                                    FindSimulcastMaxBitrateBps(width, height));
     layers[1].width = width;
     layers[1].height = height;
     layers[1].max_qp = max_qp;
     layers[1].max_framerate = kDefaultVideoMaxFramerate;
     layers[1].num_temporal_layers =
         temporal_layers_supported ? DefaultNumberOfTemporalLayers(1, true) : 0;
-    layers[1].min_bitrate_bps =
-        max_bitrate_bps == kScreenshareHighStreamMaxBitrateBps
-            ? kScreenshareHighStreamMinBitrateBps
-            : layers[0].target_bitrate_bps * 2;
+    layers[1].min_bitrate_bps = using_boosted_bitrate
+                                    ? kScreenshareHighStreamMinBitrateBps
+                                    : layers[0].target_bitrate_bps * 2;
+
+    // Cap max bitrate so it isn't overly high for the given resolution.
+    int resolution_limited_bitrate = std::max(
+        FindSimulcastMaxBitrateBps(width, height), layers[1].min_bitrate_bps);
+    max_bitrate_bps =
+        std::min<int>(max_bitrate_bps, resolution_limited_bitrate);
+
     layers[1].target_bitrate_bps = max_bitrate_bps;
     layers[1].max_bitrate_bps = max_bitrate_bps;
   }

diff --git a/media/engine/simulcast_unittest.cc b/media/engine/simulcast_unittest.cc
@@ -187,4 +187,41 @@ TEST(SimulcastTest, GetConfigForScreenshareSimulcastWithLimitedMaxLayers) {
   EXPECT_EQ(kMaxLayers, streams.size());
 }
 
+TEST(SimulcastTest, SimulcastScreenshareMaxBitrateAdjustedForResolution) {
+  test::ScopedFieldTrials field_trials("WebRTC-SimulcastScreenshare/Enabled/");
+
+  constexpr int kScreenshareHighStreamMinBitrateBps = 600000;
+  constexpr int kScreenshareHighStreamMaxBitrateBps = 1250000;
+  constexpr int kMaxBirate960_540 = 900000;
+
+  // Normal case, max bitrate not limited by resolution.
+  const size_t kMaxLayers = 2;
+  std::vector<VideoStream> streams = cricket::GetSimulcastConfig(
+      kMaxLayers, 1920, 1080, kMaxBitrateBps, kBitratePriority, kQpMax, kMaxFps,
+      kScreenshare);
+  EXPECT_EQ(kMaxLayers, streams.size());
+  EXPECT_EQ(streams[1].max_bitrate_bps, kScreenshareHighStreamMaxBitrateBps);
+  EXPECT_EQ(streams[1].min_bitrate_bps, kScreenshareHighStreamMinBitrateBps);
+  EXPECT_GE(streams[1].max_bitrate_bps, streams[1].min_bitrate_bps);
+
+  // At 960x540, the max bitrate is limited to 900kbps.
+  streams = cricket::GetSimulcastConfig(kMaxLayers, 960, 540, kMaxBitrateBps,
+                                        kBitratePriority, kQpMax, kMaxFps,
+                                        kScreenshare);
+  EXPECT_EQ(kMaxLayers, streams.size());
+  EXPECT_EQ(streams[1].max_bitrate_bps, kMaxBirate960_540);
+  EXPECT_EQ(streams[1].min_bitrate_bps, kScreenshareHighStreamMinBitrateBps);
+  EXPECT_GE(streams[1].max_bitrate_bps, streams[1].min_bitrate_bps);
+
+  // At 480x270, the max bitrate is limited to 450kbps. This is lower than
+  // the min bitrate, so use that as a lower bound.
+  streams = cricket::GetSimulcastConfig(kMaxLayers, 480, 270, kMaxBitrateBps,
+                                        kBitratePriority, kQpMax, kMaxFps,
+                                        kScreenshare);
+  EXPECT_EQ(kMaxLayers, streams.size());
+  EXPECT_EQ(streams[1].max_bitrate_bps, kScreenshareHighStreamMinBitrateBps);
+  EXPECT_EQ(streams[1].min_bitrate_bps, kScreenshareHighStreamMinBitrateBps);
+  EXPECT_GE(streams[1].max_bitrate_bps, streams[1].min_bitrate_bps);
+}
+
 }  // namespace webrtc