Revert of Periodically update codec bit/frame rate settings. (patchset #2 id:160001 of https://codereview.webrtc.org/2924023002/ )

Reason for revert:
Looks like there's still one failing perf test:
RampUpTest.UpDownUpTransportSequenceNumberPacketLoss

Original issue's description:
> Reland of Periodically update codec bit/frame rate settings. (patchset #1 id:1 of https://codereview.webrtc.org/2923993002/ )
>
> Reason for revert:
> Create reland cl that we can patch with fix.
>
> Original issue's description:
> > Revert of Periodically update codec bit/frame rate settings. (patchset #8 id:140001 of https://codereview.webrtc.org/2883963002/ )
> >
> > Reason for revert:
> > Breaks some Call perf tests that are not run by the try bots....
> >
> > Original issue's description:
> > > Fix bug in vie_encoder.cc which caused channel parameters not to be updated at regular intervals, as it was intended.
> > >
> > > That however exposes a bunch of failed test, so this CL also fixed a few other things:
> > > * FakeEncoder should trust the configured FPS value rather than guesstimating itself based on the realtime clock, so as not to completely undershoot targets in offline mode. Also, compensate for key-frame overshoots when outputting delta frames.
> > > * FrameDropper should not assuming incoming frame rate is 0 if no frames have been seen.
> > > * Fix a bunch of test cases that started failing because they were relying on the fake encoder undershooting.
> > > * Fix test
> > >
> > > BUG=7664
> > >
> > > Review-Url: https://codereview.webrtc.org/2883963002
> > > Cr-Commit-Position: refs/heads/master@{#18473}
> > > Committed: https://chromium.googlesource.com/external/webrtc/+/6431e21da672a5f3bbf166d3d4d98b171d015706
> >
> > [email protected],[email protected]
> > # Skipping CQ checks because original CL landed less than 1 days ago.
> > NOPRESUBMIT=true
> > NOTREECHECKS=true
> > NOTRY=true
> > BUG=7664
> >
> > Review-Url: https://codereview.webrtc.org/2923993002
> > Cr-Commit-Position: refs/heads/master@{#18475}
> > Committed: https://chromium.googlesource.com/external/webrtc/+/5390c4814d7880ea79edcd55596ea25e0d9b97ad
>
> [email protected],[email protected]
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=7664
>
> Review-Url: https://codereview.webrtc.org/2924023002
> Cr-Commit-Position: refs/heads/master@{#18497}
> Committed: https://chromium.googlesource.com/external/webrtc/+/cdafeda1cb95ac49104b2cc5112bab8def863818

[email protected]
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=7664

Review-Url: https://codereview.webrtc.org/2926283002
Cr-Commit-Position: refs/heads/master@{#18500}

Author: sprang

webrtc/media/engine/simulcast.cc

@@ -49,7 +49,7 @@ const SimulcastFormat kSimulcastFormats[] = {
   {0, 0, 1, 200, 150, 30}
 };
 
-const int kDefaultScreenshareSimulcastStreams = 2;
+const int kMaxScreenshareSimulcastStreams = 2;
 
 // Multiway: Number of temporal layers for each simulcast stream, for maximum
 // possible number of simulcast streams |kMaxSimulcastStreams|. The array
@@ -176,8 +176,12 @@ std::vector<webrtc::VideoStream> GetSimulcastConfig(size_t max_streams,
                                                     bool is_screencast) {
   size_t num_simulcast_layers;
   if (is_screencast) {
-    num_simulcast_layers =
-        UseSimulcastScreenshare() ? kDefaultScreenshareSimulcastStreams : 1;
+    if (UseSimulcastScreenshare()) {
+      num_simulcast_layers =
+          std::min<int>(max_streams, kMaxScreenshareSimulcastStreams);
+    } else {
+      num_simulcast_layers = 1;
+    }
   } else {
     num_simulcast_layers = FindSimulcastMaxLayers(width, height);
   }
@@ -194,54 +198,80 @@ std::vector<webrtc::VideoStream> GetSimulcastConfig(size_t max_streams,
   std::vector<webrtc::VideoStream> streams;
   streams.resize(num_simulcast_layers);
 
-  if (!is_screencast) {
+  if (is_screencast) {
+    ScreenshareLayerConfig config = ScreenshareLayerConfig::GetDefault();
+    // For legacy screenshare in conference mode, tl0 and tl1 bitrates are
+    // piggybacked on the VideoCodec struct as target and max bitrates,
+    // respectively. See eg. webrtc::VP8EncoderImpl::SetRates().
+    streams[0].width = width;
+    streams[0].height = height;
+    streams[0].max_qp = max_qp;
+    streams[0].max_framerate = 5;
+    streams[0].min_bitrate_bps = kMinVideoBitrateKbps * 1000;
+    streams[0].target_bitrate_bps = config.tl0_bitrate_kbps * 1000;
+    streams[0].max_bitrate_bps = config.tl1_bitrate_kbps * 1000;
+    streams[0].temporal_layer_thresholds_bps.clear();
+    streams[0].temporal_layer_thresholds_bps.push_back(config.tl0_bitrate_kbps *
+                                                       1000);
+
+    // With simulcast enabled, add another spatial layer. This one will have a
+    // more normal layout, with the regular 3 temporal layer pattern and no fps
+    // restrictions. The base simulcast stream will still use legacy setup.
+    if (num_simulcast_layers == kMaxScreenshareSimulcastStreams) {
+      // Add optional upper simulcast layer.
+      // Lowest temporal layers of a 3 layer setup will have 40% of the total
+      // bitrate allocation for that stream. Make sure the gap between the
+      // target of the lower stream and first temporal layer of the higher one
+      // is at most 2x the bitrate, so that upswitching is not hampered by
+      // stalled bitrate estimates.
+      int max_bitrate_bps = 2 * ((streams[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));
+
+      streams[1].width = width;
+      streams[1].height = height;
+      streams[1].max_qp = max_qp;
+      streams[1].max_framerate = max_framerate;
+      // Three temporal layers means two thresholds.
+      streams[1].temporal_layer_thresholds_bps.resize(2);
+      streams[1].min_bitrate_bps = streams[0].target_bitrate_bps * 2;
+      streams[1].target_bitrate_bps = max_bitrate_bps;
+      streams[1].max_bitrate_bps = max_bitrate_bps;
+    }
+  } else {
     // Format width and height has to be divisible by |2 ^ number_streams - 1|.
     width = NormalizeSimulcastSize(width, num_simulcast_layers);
     height = NormalizeSimulcastSize(height, num_simulcast_layers);
-  }
 
-  // Add simulcast sub-streams from lower resolution to higher resolutions.
-  // Add simulcast streams, from highest resolution (|s| = number_streams -1)
-  // to lowest resolution at |s| = 0.
-  for (size_t s = num_simulcast_layers - 1;; --s) {
-    streams[s].width = width;
-    streams[s].height = height;
-    // TODO(pbos): Fill actual temporal-layer bitrate thresholds.
-    streams[s].max_qp = max_qp;
-    if (is_screencast && s == 0) {
-      ScreenshareLayerConfig config = ScreenshareLayerConfig::GetDefault();
-      // For legacy screenshare in conference mode, tl0 and tl1 bitrates are
-      // piggybacked on the VideoCodec struct as target and max bitrates,
-      // respectively. See eg. webrtc::VP8EncoderImpl::SetRates().
-      streams[s].min_bitrate_bps = kMinVideoBitrateKbps * 1000;
-      streams[s].target_bitrate_bps = config.tl0_bitrate_kbps * 1000;
-      streams[s].max_bitrate_bps = config.tl1_bitrate_kbps * 1000;
-      streams[s].temporal_layer_thresholds_bps.clear();
-      streams[s].temporal_layer_thresholds_bps.push_back(
-          config.tl0_bitrate_kbps * 1000);
-      streams[s].max_framerate = 5;
-    } else {
+    // Add simulcast sub-streams from lower resolution to higher resolutions.
+    // Add simulcast streams, from highest resolution (|s| = number_streams -1)
+    // to lowest resolution at |s| = 0.
+    for (size_t s = num_simulcast_layers - 1;; --s) {
+      streams[s].width = width;
+      streams[s].height = height;
+      // TODO(pbos): Fill actual temporal-layer bitrate thresholds.
+      streams[s].max_qp = max_qp;
       streams[s].temporal_layer_thresholds_bps.resize(
           kDefaultConferenceNumberOfTemporalLayers[s] - 1);
       streams[s].max_bitrate_bps = FindSimulcastMaxBitrateBps(width, height);
       streams[s].target_bitrate_bps =
           FindSimulcastTargetBitrateBps(width, height);
       streams[s].min_bitrate_bps = FindSimulcastMinBitrateBps(width, height);
       streams[s].max_framerate = max_framerate;
-    }
 
-    if (!is_screencast) {
       width /= 2;
       height /= 2;
+
+      if (s == 0)
+        break;
     }
-    if (s == 0)
-      break;
-  }
 
-  // Spend additional bits to boost the max stream.
-  int bitrate_left_bps = max_bitrate_bps - GetTotalMaxBitrateBps(streams);
-  if (bitrate_left_bps > 0) {
-    streams.back().max_bitrate_bps += bitrate_left_bps;
+    // Spend additional bits to boost the max stream.
+    int bitrate_left_bps = max_bitrate_bps - GetTotalMaxBitrateBps(streams);
+    if (bitrate_left_bps > 0) {
+      streams.back().max_bitrate_bps += bitrate_left_bps;
+    }
   }
 
   return streams;

webrtc/modules/video_coding/media_optimization.cc

@@ -118,13 +118,7 @@ uint32_t MediaOptimization::SetTargetRates(uint32_t target_bitrate) {
   // Update encoding rates following protection settings.
   float target_video_bitrate_kbps =
       static_cast<float>(video_target_bitrate_) / 1000.0f;
-  float framerate = incoming_frame_rate_;
-  if (framerate == 0.0) {
-    // No framerate estimate available, use configured max framerate instead.
-    framerate = user_frame_rate_;
-  }
-
-  frame_dropper_->SetRates(target_video_bitrate_kbps, framerate);
+  frame_dropper_->SetRates(target_video_bitrate_kbps, incoming_frame_rate_);
 
   return video_target_bitrate_;
 }

webrtc/modules/video_coding/video_sender.cc

@@ -103,11 +103,6 @@ int32_t VideoSender::RegisterSendCodec(const VideoCodec* sendCodec,
     numLayers = sendCodec->VP8().numberOfTemporalLayers;
   } else if (sendCodec->codecType == kVideoCodecVP9) {
     numLayers = sendCodec->VP9().numberOfTemporalLayers;
-  } else if (sendCodec->codecType == kVideoCodecGeneric &&
-             sendCodec->numberOfSimulcastStreams > 0) {
-    // This is mainly for unit testing, disabling frame dropping.
-    // TODO(sprang): Add a better way to disable frame dropping.
-    numLayers = sendCodec->simulcastStream[0].numberOfTemporalLayers;
   } else {
     numLayers = 1;
   }

webrtc/test/fake_encoder.cc

@@ -24,15 +24,11 @@
 namespace webrtc {
 namespace test {
 
-const int kKeyframeSizeFactor = 10;
-
 FakeEncoder::FakeEncoder(Clock* clock)
     : clock_(clock),
       callback_(nullptr),
-      configured_input_framerate_(-1),
       max_target_bitrate_kbps_(-1),
-      pending_keyframe_(true),
-      debt_bytes_(0) {
+      last_encode_time_ms_(0) {
   // Generate some arbitrary not-all-zero data
   for (size_t i = 0; i < sizeof(encoded_buffer_); ++i) {
     encoded_buffer_[i] = static_cast<uint8_t>(i);
@@ -51,8 +47,6 @@ int32_t FakeEncoder::InitEncode(const VideoCodec* config,
   rtc::CritScope cs(&crit_sect_);
   config_ = *config;
   target_bitrate_.SetBitrate(0, 0, config_.startBitrate * 1000);
-  configured_input_framerate_ = config_.maxFramerate;
-  pending_keyframe_ = true;
   return 0;
 }
 
@@ -65,10 +59,9 @@ int32_t FakeEncoder::Encode(const VideoFrame& input_image,
   EncodedImageCallback* callback;
   uint32_t target_bitrate_sum_kbps;
   int max_target_bitrate_kbps;
+  int64_t last_encode_time_ms;
   size_t num_encoded_bytes;
-  int framerate;
   VideoCodecMode mode;
-  bool keyframe;
   {
     rtc::CritScope cs(&crit_sect_);
     max_framerate = config_.maxFramerate;
@@ -79,32 +72,42 @@ int32_t FakeEncoder::Encode(const VideoFrame& input_image,
     callback = callback_;
     target_bitrate_sum_kbps = target_bitrate_.get_sum_kbps();
     max_target_bitrate_kbps = max_target_bitrate_kbps_;
+    last_encode_time_ms = last_encode_time_ms_;
     num_encoded_bytes = sizeof(encoded_buffer_);
     mode = config_.mode;
-    if (configured_input_framerate_ > 0) {
-      framerate = configured_input_framerate_;
-    } else {
-      framerate = max_framerate;
-    }
-    keyframe = pending_keyframe_;
-    pending_keyframe_ = false;
   }
 
-  for (FrameType frame_type : *frame_types) {
-    if (frame_type == kVideoFrameKey) {
-      keyframe = true;
-      break;
-    }
+  int64_t time_now_ms = clock_->TimeInMilliseconds();
+  const bool first_encode = (last_encode_time_ms == 0);
+  RTC_DCHECK_GT(max_framerate, 0);
+  int64_t time_since_last_encode_ms = 1000 / max_framerate;
+  if (!first_encode) {
+    // For all frames but the first we can estimate the display time by looking
+    // at the display time of the previous frame.
+    time_since_last_encode_ms = time_now_ms - last_encode_time_ms;
+  }
+  if (time_since_last_encode_ms > 3 * 1000 / max_framerate) {
+    // Rudimentary check to make sure we don't widely overshoot bitrate target
+    // when resuming encoding after a suspension.
+    time_since_last_encode_ms = 3 * 1000 / max_framerate;
   }
 
-  RTC_DCHECK_GT(max_framerate, 0);
+  size_t bits_available =
+      static_cast<size_t>(target_bitrate_sum_kbps * time_since_last_encode_ms);
+  size_t min_bits = static_cast<size_t>(simulcast_streams[0].minBitrate *
+                                        time_since_last_encode_ms);
 
-  size_t bitrate =
-      std::max(target_bitrate_sum_kbps, simulcast_streams[0].minBitrate);
-  if (max_target_bitrate_kbps > 0)
-    bitrate = std::min(bitrate, static_cast<size_t>(max_target_bitrate_kbps));
+  if (bits_available < min_bits)
+    bits_available = min_bits;
+  size_t max_bits =
+      static_cast<size_t>(max_target_bitrate_kbps * time_since_last_encode_ms);
+  if (max_bits > 0 && max_bits < bits_available)
+    bits_available = max_bits;
 
-  size_t bits_available = bitrate * 1000 / framerate;
+  {
+    rtc::CritScope cs(&crit_sect_);
+    last_encode_time_ms_ = time_now_ms;
+  }
 
   RTC_DCHECK_GT(num_simulcast_streams, 0);
   for (unsigned char i = 0; i < num_simulcast_streams; ++i) {
@@ -113,27 +116,18 @@ int32_t FakeEncoder::Encode(const VideoFrame& input_image,
     specifics.codecType = kVideoCodecGeneric;
     specifics.codecSpecific.generic.simulcast_idx = i;
     size_t min_stream_bits = static_cast<size_t>(
-        (simulcast_streams[i].minBitrate * 1000) / framerate);
+        simulcast_streams[i].minBitrate * time_since_last_encode_ms);
     size_t max_stream_bits = static_cast<size_t>(
-        (simulcast_streams[i].maxBitrate * 1000) / framerate);
+        simulcast_streams[i].maxBitrate * time_since_last_encode_ms);
     size_t stream_bits = (bits_available > max_stream_bits) ? max_stream_bits :
         bits_available;
     size_t stream_bytes = (stream_bits + 7) / 8;
-    if (keyframe) {
+    if (first_encode) {
       // The first frame is a key frame and should be larger.
-      // Store the overshoot bytes and distribute them over the coming frames,
-      // so that we on average meet the bitrate target.
-      debt_bytes_ += (kKeyframeSizeFactor - 1) * stream_bytes;
-      stream_bytes *= kKeyframeSizeFactor;
-    } else {
-      if (debt_bytes_ > 0) {
-        // Pay at most half of the frame size for old debts.
-        size_t payment_size = std::min(stream_bytes / 2, debt_bytes_);
-        debt_bytes_ -= payment_size;
-        stream_bytes -= payment_size;
-      }
+      // TODO(holmer): The FakeEncoder should store the bits_available between
+      // encodes so that it can compensate for oversized frames.
+      stream_bytes *= 10;
     }
-
     if (stream_bytes > num_encoded_bytes)
       stream_bytes = num_encoded_bytes;
 
@@ -181,7 +175,6 @@ int32_t FakeEncoder::SetRateAllocation(const BitrateAllocation& rate_allocation,
                                        uint32_t framerate) {
   rtc::CritScope cs(&crit_sect_);
   target_bitrate_ = rate_allocation;
-  configured_input_framerate_ = framerate;
   return 0;
 }
 
@@ -190,11 +183,6 @@ const char* FakeEncoder::ImplementationName() const {
   return kImplementationName;
 }
 
-int FakeEncoder::GetConfiguredInputFramerate() const {
-  rtc::CritScope cs(&crit_sect_);
-  return configured_input_framerate_;
-}
-
 FakeH264Encoder::FakeH264Encoder(Clock* clock)
     : FakeEncoder(clock), callback_(nullptr), idr_counter_(0) {
   FakeEncoder::RegisterEncodeCompleteCallback(this);

webrtc/test/fake_encoder.h

@@ -45,7 +45,6 @@ class FakeEncoder : public VideoEncoder {
   int32_t SetRateAllocation(const BitrateAllocation& rate_allocation,
                             uint32_t framerate) override;
   const char* ImplementationName() const override;
-  int GetConfiguredInputFramerate() const;
 
   static const char* kImplementationName;
 
@@ -54,16 +53,11 @@ class FakeEncoder : public VideoEncoder {
   VideoCodec config_ GUARDED_BY(crit_sect_);
   EncodedImageCallback* callback_ GUARDED_BY(crit_sect_);
   BitrateAllocation target_bitrate_ GUARDED_BY(crit_sect_);
-  int configured_input_framerate_ GUARDED_BY(crit_sect_);
   int max_target_bitrate_kbps_ GUARDED_BY(crit_sect_);
-  bool pending_keyframe_ GUARDED_BY(crit_sect_);
+  int64_t last_encode_time_ms_ GUARDED_BY(crit_sect_);
   rtc::CriticalSection crit_sect_;
 
   uint8_t encoded_buffer_[100000];
-
-  // Current byte debt to be payed over a number of frames.
-  // The debt is acquired by keyframes overshooting the bitrate target.
-  size_t debt_bytes_;
 };
 
 class FakeH264Encoder : public FakeEncoder, public EncodedImageCallback {

webrtc/video/video_send_stream_tests.cc

@@ -939,12 +939,10 @@ void VideoSendStreamTest::TestPacketFragmentationSize(VideoFormat format,
 
     void TriggerLossReport(const RTPHeader& header) {
       // Send lossy receive reports to trigger FEC enabling.
-      const int kLossPercent = 5;
-      if (packet_count_++ % (100 / kLossPercent) != 0) {
+      if (packet_count_++ % 2 != 0) {
+        // Receive statistics reporting having lost 50% of the packets.
         FakeReceiveStatistics lossy_receive_stats(
-            kVideoSendSsrcs[0], header.sequenceNumber,
-            (packet_count_ * (100 - kLossPercent)) / 100,  // Cumulative lost.
-            static_cast<uint8_t>((255 * kLossPercent) / 100));  // Loss percent.
+            kVideoSendSsrcs[0], header.sequenceNumber, packet_count_ / 2, 127);
         RTCPSender rtcp_sender(false, Clock::GetRealTimeClock(),
                                &lossy_receive_stats, nullptr, nullptr,
                                transport_adapter_.get());
@@ -997,35 +995,6 @@ void VideoSendStreamTest::TestPacketFragmentationSize(VideoFormat format,
       // Make sure there is at least one extension header, to make the RTP
       // header larger than the base length of 12 bytes.
       EXPECT_FALSE(send_config->rtp.extensions.empty());
-
-      // Setup screen content disables frame dropping which makes this easier.
-      class VideoStreamFactory
-          : public VideoEncoderConfig::VideoStreamFactoryInterface {
-       public:
-        explicit VideoStreamFactory(size_t num_temporal_layers)
-            : num_temporal_layers_(num_temporal_layers) {
-          EXPECT_GT(num_temporal_layers, 0u);
-        }
-
-       private:
-        std::vector<VideoStream> CreateEncoderStreams(
-            int width,
-            int height,
-            const VideoEncoderConfig& encoder_config) override {
-          std::vector<VideoStream> streams =
-              test::CreateVideoStreams(width, height, encoder_config);
-          for (VideoStream& stream : streams) {
-            stream.temporal_layer_thresholds_bps.resize(num_temporal_layers_ -
-                                                        1);
-          }
-          return streams;
-        }
-        const size_t num_temporal_layers_;
-      };
-
-      encoder_config->video_stream_factory =
-          new rtc::RefCountedObject<VideoStreamFactory>(2);
-      encoder_config->content_type = VideoEncoderConfig::ContentType::kScreen;
     }
 
     void PerformTest() override {