Bitrate controller for VideoToolbox encoder.

Also fixes a crash on encoder Release.

BUG=webrtc:4081

Review URL: https://codereview.webrtc.org/1660963002

Cr-Commit-Position: refs/heads/master@{#11729}

webrtc/base/BUILD.gn

@@ -138,6 +138,8 @@ static_library("rtc_base_approved") {
     "platform_thread_types.h",
     "random.cc",
     "random.h",
+    "rate_statistics.cc",
+    "rate_statistics.h",
     "refcount.h",
     "safe_conversions.h",
     "safe_conversions_impl.h",

webrtc/base/base.gyp

@@ -106,6 +106,8 @@
         'platform_thread_types.h',
         'random.cc',
         'random.h',
+        'rate_statistics.cc',
+        'rate_statistics.h',
         'ratetracker.cc',
         'ratetracker.h',
         'refcount.h',

webrtc/base/base_tests.gyp

@@ -85,6 +85,7 @@
           'proxy_unittest.cc',
           'proxydetect_unittest.cc',
           'random_unittest.cc',
+          'rate_statistics_unittest.cc',
           'ratelimiter_unittest.cc',
           'ratetracker_unittest.cc',
           'referencecountedsingletonfactory_unittest.cc',

webrtc/base/rate_statistics.cc

@@ -8,7 +8,7 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#include "webrtc/modules/remote_bitrate_estimator/rate_statistics.h"
+#include "webrtc/base/rate_statistics.h"
 
 #include <assert.h>
 
@@ -20,11 +20,9 @@ RateStatistics::RateStatistics(uint32_t window_size_ms, float scale)
       accumulated_count_(0),
       oldest_time_(0),
       oldest_index_(0),
-      scale_(scale / (num_buckets_ - 1)) {
+      scale_(scale / (num_buckets_ - 1)) {}
-}
 
-RateStatistics::~RateStatistics() {
+RateStatistics::~RateStatistics() {}
-}
 
 void RateStatistics::Reset() {
   accumulated_count_ = 0;

webrtc/base/rate_statistics.h

@@ -8,8 +8,8 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#ifndef WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_RATE_STATISTICS_H_
+#ifndef WEBRTC_BASE_RATE_STATISTICS_H_
-#define WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_RATE_STATISTICS_H_
+#define WEBRTC_BASE_RATE_STATISTICS_H_
 
 #include "webrtc/base/scoped_ptr.h"
 #include "webrtc/typedefs.h"
@@ -50,4 +50,4 @@ class RateStatistics {
 };
 }  // namespace webrtc
 
-#endif  // WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_RATE_STATISTICS_H_
+#endif  // WEBRTC_BASE_RATE_STATISTICS_H_

webrtc/base/rate_statistics_unittest.cc

@@ -9,7 +9,7 @@
  */
 
 #include "testing/gtest/include/gtest/gtest.h"
-#include "webrtc/modules/remote_bitrate_estimator/rate_statistics.h"
+#include "webrtc/base/rate_statistics.h"
 
 namespace {
 

webrtc/modules/modules.gyp

@@ -277,7 +277,6 @@
             'remote_bitrate_estimator/include/mock/mock_remote_bitrate_estimator.h',
             'remote_bitrate_estimator/inter_arrival_unittest.cc',
             'remote_bitrate_estimator/overuse_detector_unittest.cc',
-            'remote_bitrate_estimator/rate_statistics_unittest.cc',
             'remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc',
             'remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc',
             'remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc',
@@ -360,6 +359,7 @@
             'video_coding/codecs/vp8/simulcast_unittest.h',
             'video_coding/codecs/vp9/screenshare_layers_unittest.cc',
             'video_coding/include/mock/mock_vcm_callbacks.h',
+            'video_coding/bitrate_adjuster_unittest.cc',
             'video_coding/decoding_state_unittest.cc',
             'video_coding/jitter_buffer_unittest.cc',
             'video_coding/jitter_estimator_tests.cc',

webrtc/modules/remote_bitrate_estimator/BUILD.gn

@@ -10,8 +10,6 @@ source_set("remote_bitrate_estimator") {
   sources = [
     "include/bwe_defines.h",
     "include/remote_bitrate_estimator.h",
-    "rate_statistics.cc",
-    "rate_statistics.h",
   ]
 
   configs += [ "../../:common_inherited_config" ]

webrtc/modules/remote_bitrate_estimator/overuse_detector_unittest.cc

@@ -17,12 +17,12 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 #include "webrtc/base/random.h"
+#include "webrtc/base/rate_statistics.h"
 #include "webrtc/base/scoped_ptr.h"
 #include "webrtc/common_types.h"
 #include "webrtc/modules/remote_bitrate_estimator/inter_arrival.h"
 #include "webrtc/modules/remote_bitrate_estimator/overuse_detector.h"
 #include "webrtc/modules/remote_bitrate_estimator/overuse_estimator.h"
-#include "webrtc/modules/remote_bitrate_estimator/rate_statistics.h"
 #include "webrtc/test/field_trial.h"
 
 namespace webrtc {

webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi

@@ -30,8 +30,6 @@
         'overuse_detector.h',
         'overuse_estimator.cc',
         'overuse_estimator.h',
-        'rate_statistics.cc',
-        'rate_statistics.h',
         'remote_bitrate_estimator_abs_send_time.cc',
         'remote_bitrate_estimator_abs_send_time.h',
         'remote_bitrate_estimator_single_stream.cc',

webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h

@@ -17,6 +17,7 @@
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/criticalsection.h"
+#include "webrtc/base/rate_statistics.h"
 #include "webrtc/base/scoped_ptr.h"
 #include "webrtc/base/thread_checker.h"
 #include "webrtc/modules/remote_bitrate_estimator/aimd_rate_control.h"
@@ -24,7 +25,6 @@
 #include "webrtc/modules/remote_bitrate_estimator/inter_arrival.h"
 #include "webrtc/modules/remote_bitrate_estimator/overuse_detector.h"
 #include "webrtc/modules/remote_bitrate_estimator/overuse_estimator.h"
-#include "webrtc/modules/remote_bitrate_estimator/rate_statistics.h"
 #include "webrtc/system_wrappers/include/critical_section_wrapper.h"
 
 namespace webrtc {

webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h

@@ -14,9 +14,9 @@
 #include <map>
 #include <vector>
 
+#include "webrtc/base/rate_statistics.h"
 #include "webrtc/modules/remote_bitrate_estimator/aimd_rate_control.h"
 #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
-#include "webrtc/modules/remote_bitrate_estimator/rate_statistics.h"
 #include "webrtc/system_wrappers/include/critical_section_wrapper.h"
 
 namespace webrtc {

webrtc/modules/video_coding/BUILD.gn

@@ -10,6 +10,8 @@ import("../../build/webrtc.gni")
 
 source_set("video_coding") {
   sources = [
+    "bitrate_adjuster.cc",
+    "bitrate_adjuster.h",
     "codec_database.cc",
     "codec_database.h",
     "codec_timer.cc",

webrtc/modules/video_coding/bitrate_adjuster.cc

@@ -0,0 +1,160 @@
+/*
+ *  Copyright 2016 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "webrtc/modules/video_coding/include/bitrate_adjuster.h"
+
+#include <cmath>
+
+#include "webrtc/base/checks.h"
+#include "webrtc/base/logging.h"
+#include "webrtc/system_wrappers/include/clock.h"
+
+namespace webrtc {
+
+// Update bitrate at most once every second.
+const uint32_t BitrateAdjuster::kBitrateUpdateIntervalMs = 1000;
+
+// Update bitrate at most once every 30 frames.
+const uint32_t BitrateAdjuster::kBitrateUpdateFrameInterval = 30;
+
+// 10 percent of original.
+const float BitrateAdjuster::kBitrateTolerancePct = .1f;
+
+const float BitrateAdjuster::kBytesPerMsToBitsPerSecond = 8 * 1000;
+
+BitrateAdjuster::BitrateAdjuster(Clock* clock,
+                                 float min_adjusted_bitrate_pct,
+                                 float max_adjusted_bitrate_pct)
+    : clock_(clock),
+      min_adjusted_bitrate_pct_(min_adjusted_bitrate_pct),
+      max_adjusted_bitrate_pct_(max_adjusted_bitrate_pct),
+      bitrate_tracker_(1.5 * kBitrateUpdateIntervalMs,
+                       kBytesPerMsToBitsPerSecond) {
+  Reset();
+}
+
+void BitrateAdjuster::SetTargetBitrateBps(uint32_t bitrate_bps) {
+  rtc::CritScope cs(&crit_);
+  // If the change in target bitrate is large, update the adjusted bitrate
+  // immediately since it's likely we have gained or lost a sizeable amount of
+  // bandwidth and we'll want to respond quickly.
+  // If the change in target bitrate fits within the existing tolerance of
+  // encoder output, wait for the next adjustment time to preserve
+  // existing penalties and not forcibly reset the adjusted bitrate to target.
+  // However, if we received many small deltas within an update time
+  // window and one of them exceeds the tolerance when compared to the last
+  // target we updated against, treat it as a large change in target bitrate.
+  if (!IsWithinTolerance(bitrate_bps, target_bitrate_bps_) ||
+      !IsWithinTolerance(bitrate_bps, last_adjusted_target_bitrate_bps_)) {
+    adjusted_bitrate_bps_ = bitrate_bps;
+    last_adjusted_target_bitrate_bps_ = bitrate_bps;
+  }
+  target_bitrate_bps_ = bitrate_bps;
+}
+
+uint32_t BitrateAdjuster::GetTargetBitrateBps() const {
+  rtc::CritScope cs(&crit_);
+  return target_bitrate_bps_;
+}
+
+uint32_t BitrateAdjuster::GetAdjustedBitrateBps() const {
+  rtc::CritScope cs(&crit_);
+  return adjusted_bitrate_bps_;
+}
+
+uint32_t BitrateAdjuster::GetEstimatedBitrateBps() {
+  rtc::CritScope cs(&crit_);
+  return bitrate_tracker_.Rate(clock_->TimeInMilliseconds());
+}
+
+void BitrateAdjuster::Update(size_t frame_size) {
+  rtc::CritScope cs(&crit_);
+  uint32_t current_time_ms = clock_->TimeInMilliseconds();
+  bitrate_tracker_.Update(frame_size, current_time_ms);
+  UpdateBitrate(current_time_ms);
+}
+
+bool BitrateAdjuster::IsWithinTolerance(uint32_t bitrate_bps,
+                                        uint32_t target_bitrate_bps) {
+  if (target_bitrate_bps == 0) {
+    return false;
+  }
+  float delta = std::abs(static_cast<float>(bitrate_bps) -
+                         static_cast<float>(target_bitrate_bps));
+  float delta_pct = delta / target_bitrate_bps;
+  return delta_pct < kBitrateTolerancePct;
+}
+
+uint32_t BitrateAdjuster::GetMinAdjustedBitrateBps() const {
+  return min_adjusted_bitrate_pct_ * target_bitrate_bps_;
+}
+
+uint32_t BitrateAdjuster::GetMaxAdjustedBitrateBps() const {
+  return max_adjusted_bitrate_pct_ * target_bitrate_bps_;
+}
+
+// Only safe to call this after Update calls have stopped
+void BitrateAdjuster::Reset() {
+  rtc::CritScope cs(&crit_);
+  target_bitrate_bps_ = 0;
+  adjusted_bitrate_bps_ = 0;
+  last_adjusted_target_bitrate_bps_ = 0;
+  last_bitrate_update_time_ms_ = 0;
+  frames_since_last_update_ = 0;
+  bitrate_tracker_.Reset();
+}
+
+void BitrateAdjuster::UpdateBitrate(uint32_t current_time_ms) {
+  uint32_t time_since_last_update_ms =
+      current_time_ms - last_bitrate_update_time_ms_;
+  // Don't attempt to update bitrate unless enough time and frames have passed.
+  ++frames_since_last_update_;
+  if (time_since_last_update_ms < kBitrateUpdateIntervalMs ||
+      frames_since_last_update_ < kBitrateUpdateFrameInterval) {
+    return;
+  }
+  float estimated_bitrate_bps = bitrate_tracker_.Rate(current_time_ms);
+  float target_bitrate_bps = target_bitrate_bps_;
+  float error = target_bitrate_bps - estimated_bitrate_bps;
+
+  // Adjust if we've overshot by any amount or if we've undershot too much.
+  if (estimated_bitrate_bps > target_bitrate_bps ||
+      error > kBitrateTolerancePct * target_bitrate_bps) {
+    // Adjust the bitrate by a fraction of the error.
+    float adjustment = .5 * error;
+    float adjusted_bitrate_bps = target_bitrate_bps + adjustment;
+
+    // Clamp the adjustment.
+    float min_bitrate_bps = GetMinAdjustedBitrateBps();
+    float max_bitrate_bps = GetMaxAdjustedBitrateBps();
+    adjusted_bitrate_bps = std::max(adjusted_bitrate_bps, min_bitrate_bps);
+    adjusted_bitrate_bps = std::min(adjusted_bitrate_bps, max_bitrate_bps);
+
+    // Set the adjustment if it's not already set.
+    float last_adjusted_bitrate_bps = adjusted_bitrate_bps_;
+    if (adjusted_bitrate_bps != last_adjusted_bitrate_bps) {
+      LOG(LS_VERBOSE) << "Adjusting encoder bitrate:"
+                      << "\n  target_bitrate:"
+                      << static_cast<uint32_t>(target_bitrate_bps)
+                      << "\n  estimated_bitrate:"
+                      << static_cast<uint32_t>(estimated_bitrate_bps)
+                      << "\n  last_adjusted_bitrate:"
+                      << static_cast<uint32_t>(last_adjusted_bitrate_bps)
+                      << "\n  adjusted_bitrate:"
+                      << static_cast<uint32_t>(adjusted_bitrate_bps);
+      adjusted_bitrate_bps_ = adjusted_bitrate_bps;
+    }
+  }
+  last_bitrate_update_time_ms_ = current_time_ms;
+  frames_since_last_update_ = 0;
+  last_adjusted_target_bitrate_bps_ = target_bitrate_bps_;
+}
+
+}  // namespace webrtc

webrtc/modules/video_coding/bitrate_adjuster_unittest.cc

@@ -0,0 +1,168 @@
+/*
+ *  Copyright 2016 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+#include "webrtc/modules/video_coding/include/bitrate_adjuster.h"
+#include "webrtc/system_wrappers/include/clock.h"
+
+namespace webrtc {
+
+class BitrateAdjusterTest : public ::testing::Test {
+ public:
+  BitrateAdjusterTest()
+      : clock_(0),
+        adjuster_(&clock_, kMinAdjustedBitratePct, kMaxAdjustedBitratePct) {}
+
+  // Simulate an output bitrate for one update cycle of BitrateAdjuster.
+  void SimulateBitrateBps(uint32_t bitrate_bps) {
+    const uint32_t update_interval_ms =
+        BitrateAdjuster::kBitrateUpdateIntervalMs;
+    const uint32_t update_frame_interval =
+        BitrateAdjuster::kBitrateUpdateFrameInterval;
+    // Round up frame interval so we get one cycle passes.
+    const uint32_t frame_interval_ms =
+        (update_interval_ms + update_frame_interval - 1) /
+        update_frame_interval;
+    const size_t frame_size_bytes =
+        (bitrate_bps * frame_interval_ms) / (8 * 1000);
+    for (size_t i = 0; i < update_frame_interval; ++i) {
+      clock_.AdvanceTimeMilliseconds(frame_interval_ms);
+      adjuster_.Update(frame_size_bytes);
+    }
+  }
+
+  uint32_t GetTargetBitrateBpsPct(float pct) {
+    return pct * adjuster_.GetTargetBitrateBps();
+  }
+
+  void VerifyAdjustment() {
+    // The adjusted bitrate should be between the estimated bitrate and the
+    // target bitrate within clamp.
+    uint32_t target_bitrate_bps = adjuster_.GetTargetBitrateBps();
+    uint32_t adjusted_bitrate_bps = adjuster_.GetAdjustedBitrateBps();
+    uint32_t estimated_bitrate_bps = adjuster_.GetEstimatedBitrateBps();
+    uint32_t adjusted_lower_bound_bps =
+        GetTargetBitrateBpsPct(kMinAdjustedBitratePct);
+    uint32_t adjusted_upper_bound_bps =
+        GetTargetBitrateBpsPct(kMaxAdjustedBitratePct);
+    EXPECT_LE(adjusted_bitrate_bps, adjusted_upper_bound_bps);
+    EXPECT_GE(adjusted_bitrate_bps, adjusted_lower_bound_bps);
+    if (estimated_bitrate_bps > target_bitrate_bps) {
+      EXPECT_LT(adjusted_bitrate_bps, target_bitrate_bps);
+    }
+  }
+
+ protected:
+  static const float kMinAdjustedBitratePct;
+  static const float kMaxAdjustedBitratePct;
+  SimulatedClock clock_;
+  BitrateAdjuster adjuster_;
+};
+
+const float BitrateAdjusterTest::kMinAdjustedBitratePct = .5f;
+const float BitrateAdjusterTest::kMaxAdjustedBitratePct = .95f;
+
+TEST_F(BitrateAdjusterTest, VaryingBitrates) {
+  const uint32_t target_bitrate_bps = 640000;
+  adjuster_.SetTargetBitrateBps(target_bitrate_bps);
+
+  // Grossly overshoot for a little while. Adjusted bitrate should decrease.
+  uint32_t actual_bitrate_bps = 2 * target_bitrate_bps;
+  uint32_t last_adjusted_bitrate_bps = 0;
+  uint32_t adjusted_bitrate_bps = 0;
+
+  SimulateBitrateBps(actual_bitrate_bps);
+  VerifyAdjustment();
+  last_adjusted_bitrate_bps = adjuster_.GetAdjustedBitrateBps();
+
+  SimulateBitrateBps(actual_bitrate_bps);
+  VerifyAdjustment();
+  adjusted_bitrate_bps = adjuster_.GetAdjustedBitrateBps();
+  EXPECT_LT(adjusted_bitrate_bps, last_adjusted_bitrate_bps);
+  last_adjusted_bitrate_bps = adjusted_bitrate_bps;
+  // After two cycles we should've stabilized and hit the lower bound.
+  EXPECT_EQ(GetTargetBitrateBpsPct(kMinAdjustedBitratePct),
+            adjusted_bitrate_bps);
+
+  // Simulate encoder settling down. Adjusted bitrate should increase.
+  SimulateBitrateBps(target_bitrate_bps);
+  adjusted_bitrate_bps = adjuster_.GetAdjustedBitrateBps();
+  VerifyAdjustment();
+  EXPECT_GT(adjusted_bitrate_bps, last_adjusted_bitrate_bps);
+  last_adjusted_bitrate_bps = adjusted_bitrate_bps;
+
+  SimulateBitrateBps(target_bitrate_bps);
+  adjusted_bitrate_bps = adjuster_.GetAdjustedBitrateBps();
+  VerifyAdjustment();
+  EXPECT_GT(adjusted_bitrate_bps, last_adjusted_bitrate_bps);
+  last_adjusted_bitrate_bps = adjusted_bitrate_bps;
+  // After two cycles we should've stabilized and hit the upper bound.
+  EXPECT_EQ(GetTargetBitrateBpsPct(kMaxAdjustedBitratePct),
+            adjusted_bitrate_bps);
+}
+
+// Tests that large changes in target bitrate will result in immediate change
+// in adjusted bitrate.
+TEST_F(BitrateAdjusterTest, LargeTargetDelta) {
+  uint32_t target_bitrate_bps = 640000;
+  adjuster_.SetTargetBitrateBps(target_bitrate_bps);
+  EXPECT_EQ(target_bitrate_bps, adjuster_.GetAdjustedBitrateBps());
+
+  float delta_pct = BitrateAdjuster::kBitrateTolerancePct * 2;
+
+  target_bitrate_bps = (1 + delta_pct) * target_bitrate_bps;
+  adjuster_.SetTargetBitrateBps(target_bitrate_bps);
+  EXPECT_EQ(target_bitrate_bps, adjuster_.GetAdjustedBitrateBps());
+
+  target_bitrate_bps = (1 - delta_pct) * target_bitrate_bps;
+  adjuster_.SetTargetBitrateBps(target_bitrate_bps);
+  EXPECT_EQ(target_bitrate_bps, adjuster_.GetAdjustedBitrateBps());
+}
+
+// Tests that small changes in target bitrate within tolerance will not affect
+// adjusted bitrate immediately.
+TEST_F(BitrateAdjusterTest, SmallTargetDelta) {
+  const uint32_t initial_target_bitrate_bps = 640000;
+  uint32_t target_bitrate_bps = initial_target_bitrate_bps;
+  adjuster_.SetTargetBitrateBps(target_bitrate_bps);
+  EXPECT_EQ(initial_target_bitrate_bps, adjuster_.GetAdjustedBitrateBps());
+
+  float delta_pct = BitrateAdjuster::kBitrateTolerancePct / 2;
+
+  target_bitrate_bps = (1 + delta_pct) * target_bitrate_bps;
+  adjuster_.SetTargetBitrateBps(target_bitrate_bps);
+  EXPECT_EQ(initial_target_bitrate_bps, adjuster_.GetAdjustedBitrateBps());
+
+  target_bitrate_bps = (1 - delta_pct) * target_bitrate_bps;
+  adjuster_.SetTargetBitrateBps(target_bitrate_bps);
+  EXPECT_EQ(initial_target_bitrate_bps, adjuster_.GetAdjustedBitrateBps());
+}
+
+TEST_F(BitrateAdjusterTest, SmallTargetDeltaOverflow) {
+  const uint32_t initial_target_bitrate_bps = 640000;
+  uint32_t target_bitrate_bps = initial_target_bitrate_bps;
+  adjuster_.SetTargetBitrateBps(target_bitrate_bps);
+  EXPECT_EQ(initial_target_bitrate_bps, adjuster_.GetAdjustedBitrateBps());
+
+  float delta_pct = BitrateAdjuster::kBitrateTolerancePct / 2;
+
+  target_bitrate_bps = (1 + delta_pct) * target_bitrate_bps;
+  adjuster_.SetTargetBitrateBps(target_bitrate_bps);
+  EXPECT_EQ(initial_target_bitrate_bps, adjuster_.GetAdjustedBitrateBps());
+
+  // 1.05 * 1.05 is 1.1 which is greater than tolerance for the initial target
+  // bitrate. Since we didn't advance the clock the adjuster never updated.
+  target_bitrate_bps = (1 + delta_pct) * target_bitrate_bps;
+  adjuster_.SetTargetBitrateBps(target_bitrate_bps);
+  EXPECT_EQ(target_bitrate_bps, adjuster_.GetAdjustedBitrateBps());
+}
+
+}  // namespace webrtc

webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_decoder.cc

@@ -164,6 +164,10 @@ int H264VideoToolboxDecoder::RegisterDecodeCompleteCallback(
 }
 
 int H264VideoToolboxDecoder::Release() {
+  // Need to invalidate the session so that callbacks no longer occur and it
+  // is safe to null out the callback.
+  DestroyDecompressionSession();
+  SetVideoFormat(nullptr);
   callback_ = nullptr;
   return WEBRTC_VIDEO_CODEC_OK;
 }

webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.cc

@@ -21,6 +21,7 @@
 #include "webrtc/base/logging.h"
 #include "webrtc/base/scoped_ptr.h"
 #include "webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_nalu.h"
+#include "webrtc/system_wrappers/include/clock.h"
 
 namespace internal {
 
@@ -66,6 +67,22 @@ void SetVTSessionProperty(VTSessionRef session,
   }
 }
 
+// Convenience function for setting a VT property.
+void SetVTSessionProperty(VTSessionRef session,
+                          CFStringRef key,
+                          uint32_t value) {
+  int64_t value_64 = value;
+  CFNumberRef cfNum =
+      CFNumberCreate(kCFAllocatorDefault, kCFNumberSInt64Type, &value_64);
+  OSStatus status = VTSessionSetProperty(session, key, cfNum);
+  CFRelease(cfNum);
+  if (status != noErr) {
+    std::string key_string = CFStringToString(key);
+    LOG(LS_ERROR) << "VTSessionSetProperty failed to set: " << key_string
+                  << " to " << value << ": " << status;
+  }
+}
+
 // Convenience function for setting a VT property.
 void SetVTSessionProperty(VTSessionRef session, CFStringRef key, bool value) {
   CFBooleanRef cf_bool = (value) ? kCFBooleanTrue : kCFBooleanFalse;
@@ -93,20 +110,21 @@ void SetVTSessionProperty(VTSessionRef session,
 // Struct that we pass to the encoder per frame to encode. We receive it again
 // in the encoder callback.
 struct FrameEncodeParams {
-  FrameEncodeParams(webrtc::EncodedImageCallback* cb,
+  FrameEncodeParams(webrtc::H264VideoToolboxEncoder* e,
                     const webrtc::CodecSpecificInfo* csi,
                     int32_t w,
                     int32_t h,
                     int64_t rtms,
                     uint32_t ts)
-      : callback(cb), width(w), height(h), render_time_ms(rtms), timestamp(ts) {
+      : encoder(e), width(w), height(h), render_time_ms(rtms), timestamp(ts) {
     if (csi) {
       codec_specific_info = *csi;
     } else {
       codec_specific_info.codecType = webrtc::kVideoCodecH264;
     }
   }
-  webrtc::EncodedImageCallback* callback;
+
+  webrtc::H264VideoToolboxEncoder* encoder;
   webrtc::CodecSpecificInfo codec_specific_info;
   int32_t width;
   int32_t height;
@@ -153,7 +171,7 @@ bool CopyVideoFrameToPixelBuffer(const webrtc::VideoFrame& frame,
 }
 
 // This is the callback function that VideoToolbox calls when encode is
-// complete.
+// complete. From inspection this happens on its own queue.
 void VTCompressionOutputCallback(void* encoder,
                                  void* params,
                                  OSStatus status,
@@ -161,54 +179,27 @@ void VTCompressionOutputCallback(void* encoder,
                                  CMSampleBufferRef sample_buffer) {
   rtc::scoped_ptr<FrameEncodeParams> encode_params(
       reinterpret_cast<FrameEncodeParams*>(params));
-  if (status != noErr) {
+  encode_params->encoder->OnEncodedFrame(
-    LOG(LS_ERROR) << "H264 encoding failed.";
+      status, info_flags, sample_buffer, encode_params->codec_specific_info,
-    return;
+      encode_params->width, encode_params->height,
-  }
+      encode_params->render_time_ms, encode_params->timestamp);
-  if (info_flags & kVTEncodeInfo_FrameDropped) {
-    LOG(LS_INFO) << "H264 encode dropped frame.";
-  }
-
-  bool is_keyframe = false;
-  CFArrayRef attachments =
-      CMSampleBufferGetSampleAttachmentsArray(sample_buffer, 0);
-  if (attachments != nullptr && CFArrayGetCount(attachments)) {
-    CFDictionaryRef attachment =
-        static_cast<CFDictionaryRef>(CFArrayGetValueAtIndex(attachments, 0));
-    is_keyframe =
-        !CFDictionaryContainsKey(attachment, kCMSampleAttachmentKey_NotSync);
-  }
-
-  // Convert the sample buffer into a buffer suitable for RTP packetization.
-  // TODO(tkchin): Allocate buffers through a pool.
-  rtc::scoped_ptr<rtc::Buffer> buffer(new rtc::Buffer());
-  rtc::scoped_ptr<webrtc::RTPFragmentationHeader> header;
-  if (!H264CMSampleBufferToAnnexBBuffer(sample_buffer, is_keyframe,
-                                        buffer.get(), header.accept())) {
-    return;
-  }
-  webrtc::EncodedImage frame(buffer->data(), buffer->size(), buffer->size());
-  frame._encodedWidth = encode_params->width;
-  frame._encodedHeight = encode_params->height;
-  frame._completeFrame = true;
-  frame._frameType =
-      is_keyframe ? webrtc::kVideoFrameKey : webrtc::kVideoFrameDelta;
-  frame.capture_time_ms_ = encode_params->render_time_ms;
-  frame._timeStamp = encode_params->timestamp;
-
-  int result = encode_params->callback->Encoded(
-      frame, &(encode_params->codec_specific_info), header.get());
-  if (result != 0) {
-    LOG(LS_ERROR) << "Encoded callback failed: " << result;
-  }
 }
 
 }  // namespace internal
 
 namespace webrtc {
 
+// .5 is set as a mininum to prevent overcompensating for large temporary
+// overshoots. We don't want to degrade video quality too badly.
+// .95 is set to prevent oscillations. When a lower bitrate is set on the
+// encoder than previously set, its output seems to have a brief period of
+// drastically reduced bitrate, so we want to avoid that. In steady state
+// conditions, 0.95 seems to give us better overall bitrate over long periods
+// of time.
 H264VideoToolboxEncoder::H264VideoToolboxEncoder()
-    : callback_(nullptr), compression_session_(nullptr) {}
+    : callback_(nullptr),
+      compression_session_(nullptr),
+      bitrate_adjuster_(Clock::GetRealTimeClock(), .5, .95) {}
 
 H264VideoToolboxEncoder::~H264VideoToolboxEncoder() {
   DestroyCompressionSession();
@@ -224,7 +215,8 @@ int H264VideoToolboxEncoder::InitEncode(const VideoCodec* codec_settings,
   width_ = codec_settings->width;
   height_ = codec_settings->height;
   // We can only set average bitrate on the HW encoder.
-  bitrate_ = codec_settings->startBitrate * 1000;
+  target_bitrate_bps_ = codec_settings->startBitrate;
+  bitrate_adjuster_.SetTargetBitrateBps(target_bitrate_bps_);
 
   // TODO(tkchin): Try setting payload size via
   // kVTCompressionPropertyKey_MaxH264SliceBytes.
@@ -287,8 +279,12 @@ int H264VideoToolboxEncoder::Encode(
   }
   rtc::scoped_ptr<internal::FrameEncodeParams> encode_params;
   encode_params.reset(new internal::FrameEncodeParams(
-      callback_, codec_specific_info, width_, height_,
+      this, codec_specific_info, width_, height_, input_image.render_time_ms(),
-      input_image.render_time_ms(), input_image.timestamp()));
+      input_image.timestamp()));
+
+  // Update the bitrate if needed.
+  SetBitrateBps(bitrate_adjuster_.GetAdjustedBitrateBps());
+
   VTCompressionSessionEncodeFrame(
       compression_session_, pixel_buffer, presentation_time_stamp,
       kCMTimeInvalid, frame_properties, encode_params.release(), nullptr);
@@ -315,20 +311,20 @@ int H264VideoToolboxEncoder::SetChannelParameters(uint32_t packet_loss,
 
 int H264VideoToolboxEncoder::SetRates(uint32_t new_bitrate_kbit,
                                       uint32_t frame_rate) {
-  bitrate_ = new_bitrate_kbit * 1000;
+  target_bitrate_bps_ = 1000 * new_bitrate_kbit;
-  if (compression_session_) {
+  bitrate_adjuster_.SetTargetBitrateBps(target_bitrate_bps_);
-    internal::SetVTSessionProperty(compression_session_,
+  SetBitrateBps(bitrate_adjuster_.GetAdjustedBitrateBps());
-                                   kVTCompressionPropertyKey_AverageBitRate,
+
-                                   bitrate_);
-  }
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
 int H264VideoToolboxEncoder::Release() {
+  // Need to reset so that the session is invalidated and won't use the
+  // callback anymore. Do not remove callback until the session is invalidated
+  // since async encoder callbacks can occur until invalidation.
+  int ret = ResetCompressionSession();
   callback_ = nullptr;
-  // Need to reset to that the session is invalidated and won't use the
+  return ret;
-  // callback anymore.
-  return ResetCompressionSession();
 }
 
 int H264VideoToolboxEncoder::ResetCompressionSession() {
@@ -389,11 +385,10 @@ void H264VideoToolboxEncoder::ConfigureCompressionSession() {
   internal::SetVTSessionProperty(compression_session_,
                                  kVTCompressionPropertyKey_ProfileLevel,
                                  kVTProfileLevel_H264_Baseline_AutoLevel);
-  internal::SetVTSessionProperty(
-      compression_session_, kVTCompressionPropertyKey_AverageBitRate, bitrate_);
   internal::SetVTSessionProperty(compression_session_,
                                  kVTCompressionPropertyKey_AllowFrameReordering,
                                  false);
+  SetEncoderBitrateBps(target_bitrate_bps_);
   // TODO(tkchin): Look at entropy mode and colorspace matrices.
   // TODO(tkchin): Investigate to see if there's any way to make this work.
   // May need it to interop with Android. Currently this call just fails.
@@ -423,6 +418,73 @@ const char* H264VideoToolboxEncoder::ImplementationName() const {
   return "VideoToolbox";
 }
 
+void H264VideoToolboxEncoder::SetBitrateBps(uint32_t bitrate_bps) {
+  if (encoder_bitrate_bps_ != bitrate_bps) {
+    SetEncoderBitrateBps(bitrate_bps);
+  }
+}
+
+void H264VideoToolboxEncoder::SetEncoderBitrateBps(uint32_t bitrate_bps) {
+  if (compression_session_) {
+    internal::SetVTSessionProperty(compression_session_,
+                                   kVTCompressionPropertyKey_AverageBitRate,
+                                   bitrate_bps);
+    encoder_bitrate_bps_ = bitrate_bps;
+  }
+}
+
+void H264VideoToolboxEncoder::OnEncodedFrame(
+    OSStatus status,
+    VTEncodeInfoFlags info_flags,
+    CMSampleBufferRef sample_buffer,
+    CodecSpecificInfo codec_specific_info,
+    int32_t width,
+    int32_t height,
+    int64_t render_time_ms,
+    uint32_t timestamp) {
+  if (status != noErr) {
+    LOG(LS_ERROR) << "H264 encode failed.";
+    return;
+  }
+  if (info_flags & kVTEncodeInfo_FrameDropped) {
+    LOG(LS_INFO) << "H264 encode dropped frame.";
+  }
+
+  bool is_keyframe = false;
+  CFArrayRef attachments =
+      CMSampleBufferGetSampleAttachmentsArray(sample_buffer, 0);
+  if (attachments != nullptr && CFArrayGetCount(attachments)) {
+    CFDictionaryRef attachment =
+        static_cast<CFDictionaryRef>(CFArrayGetValueAtIndex(attachments, 0));
+    is_keyframe =
+        !CFDictionaryContainsKey(attachment, kCMSampleAttachmentKey_NotSync);
+  }
+
+  // Convert the sample buffer into a buffer suitable for RTP packetization.
+  // TODO(tkchin): Allocate buffers through a pool.
+  rtc::scoped_ptr<rtc::Buffer> buffer(new rtc::Buffer());
+  rtc::scoped_ptr<webrtc::RTPFragmentationHeader> header;
+  if (!H264CMSampleBufferToAnnexBBuffer(sample_buffer, is_keyframe,
+                                        buffer.get(), header.accept())) {
+    return;
+  }
+  webrtc::EncodedImage frame(buffer->data(), buffer->size(), buffer->size());
+  frame._encodedWidth = width;
+  frame._encodedHeight = height;
+  frame._completeFrame = true;
+  frame._frameType =
+      is_keyframe ? webrtc::kVideoFrameKey : webrtc::kVideoFrameDelta;
+  frame.capture_time_ms_ = render_time_ms;
+  frame._timeStamp = timestamp;
+
+  int result = callback_->Encoded(frame, &codec_specific_info, header.get());
+  if (result != 0) {
+    LOG(LS_ERROR) << "Encode callback failed: " << result;
+    return;
+  }
+  bitrate_adjuster_.Update(frame._size);
+}
+
 }  // namespace webrtc
 
 #endif  // defined(WEBRTC_VIDEO_TOOLBOX_SUPPORTED)

webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.h

@@ -13,6 +13,7 @@
 #define WEBRTC_MODULES_VIDEO_CODING_CODECS_H264_H264_VIDEO_TOOLBOX_ENCODER_H_
 
 #include "webrtc/modules/video_coding/codecs/h264/include/h264.h"
+#include "webrtc/modules/video_coding/include/bitrate_adjuster.h"
 
 #if defined(WEBRTC_VIDEO_TOOLBOX_SUPPORTED)
 
@@ -50,14 +51,27 @@ class H264VideoToolboxEncoder : public H264Encoder {
 
   const char* ImplementationName() const override;
 
+  void OnEncodedFrame(OSStatus status,
+                      VTEncodeInfoFlags info_flags,
+                      CMSampleBufferRef sample_buffer,
+                      CodecSpecificInfo codec_specific_info,
+                      int32_t width,
+                      int32_t height,
+                      int64_t render_time_ms,
+                      uint32_t timestamp);
+
  private:
   int ResetCompressionSession();
   void ConfigureCompressionSession();
   void DestroyCompressionSession();
+  void SetBitrateBps(uint32_t bitrate_bps);
+  void SetEncoderBitrateBps(uint32_t bitrate_bps);
 
-  webrtc::EncodedImageCallback* callback_;
+  EncodedImageCallback* callback_;
   VTCompressionSessionRef compression_session_;
-  int32_t bitrate_;  // Bitrate in bits per second.
+  BitrateAdjuster bitrate_adjuster_;
+  uint32_t target_bitrate_bps_;
+  uint32_t encoder_bitrate_bps_;
   int32_t width_;
   int32_t height_;
 };  // H264VideoToolboxEncoder

webrtc/modules/video_coding/include/bitrate_adjuster.h

@@ -0,0 +1,90 @@
+/*
+ *  Copyright 2016 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef WEBRTC_MODULES_VIDEO_CODING_INCLUDE_BITRATE_ADJUSTER_H_
+#define WEBRTC_MODULES_VIDEO_CODING_INCLUDE_BITRATE_ADJUSTER_H_
+
+#include <functional>
+
+#include "webrtc/base/criticalsection.h"
+#include "webrtc/base/gtest_prod_util.h"
+#include "webrtc/base/rate_statistics.h"
+
+namespace webrtc {
+
+class Clock;
+
+// Certain hardware encoders tend to consistently overshoot the bitrate that
+// they are configured to encode at. This class estimates an adjusted bitrate
+// that when set on the encoder will produce the desired bitrate.
+class BitrateAdjuster {
+ public:
+  // min_adjusted_bitrate_pct and max_adjusted_bitrate_pct are the lower and
+  // upper bound outputted adjusted bitrates as a percentage of the target
+  // bitrate.
+  BitrateAdjuster(Clock* clock,
+                  float min_adjusted_bitrate_pct,
+                  float max_adjusted_bitrate_pct);
+  virtual ~BitrateAdjuster() {}
+
+  static const uint32_t kBitrateUpdateIntervalMs;
+  static const uint32_t kBitrateUpdateFrameInterval;
+  static const float kBitrateTolerancePct;
+  static const float kBytesPerMsToBitsPerSecond;
+
+  // Sets the desired bitrate in bps (bits per second).
+  // Should be called at least once before Update.
+  void SetTargetBitrateBps(uint32_t bitrate_bps);
+  uint32_t GetTargetBitrateBps() const;
+
+  // Returns the adjusted bitrate in bps.
+  uint32_t GetAdjustedBitrateBps() const;
+
+  // Returns what we think the current bitrate is.
+  uint32_t GetEstimatedBitrateBps();
+
+  // This should be called after each frame is encoded. The timestamp at which
+  // it is called is used to estimate the output bitrate of the encoder.
+  // Should be called from only one thread.
+  void Update(size_t frame_size);
+
+ private:
+  // Returns true if the bitrate is within kBitrateTolerancePct of bitrate_bps.
+  bool IsWithinTolerance(uint32_t bitrate_bps, uint32_t target_bitrate_bps);
+
+  // Returns smallest possible adjusted value.
+  uint32_t GetMinAdjustedBitrateBps() const EXCLUSIVE_LOCKS_REQUIRED(crit_);
+  // Returns largest possible adjusted value.
+  uint32_t GetMaxAdjustedBitrateBps() const EXCLUSIVE_LOCKS_REQUIRED(crit_);
+
+  void Reset();
+  void UpdateBitrate(uint32_t current_time_ms) EXCLUSIVE_LOCKS_REQUIRED(crit_);
+
+  rtc::CriticalSection crit_;
+  Clock* const clock_;
+  const float min_adjusted_bitrate_pct_;
+  const float max_adjusted_bitrate_pct_;
+  // The bitrate we want.
+  volatile uint32_t target_bitrate_bps_ GUARDED_BY(crit_);
+  // The bitrate we use to get what we want.
+  volatile uint32_t adjusted_bitrate_bps_ GUARDED_BY(crit_);
+  // The target bitrate that the adjusted bitrate was computed from.
+  volatile uint32_t last_adjusted_target_bitrate_bps_ GUARDED_BY(crit_);
+  // Used to estimate bitrate.
+  RateStatistics bitrate_tracker_ GUARDED_BY(crit_);
+  // The last time we tried to adjust the bitrate.
+  uint32_t last_bitrate_update_time_ms_ GUARDED_BY(crit_);
+  // The number of frames since the last time we tried to adjust the bitrate.
+  uint32_t frames_since_last_update_ GUARDED_BY(crit_);
+};
+
+}  // namespace webrtc
+
+#endif  // WEBRTC_MODULES_VIDEO_CODING_INCLUDE_BITRATE_ADJUSTER_H_

webrtc/modules/video_coding/video_coding.gypi

@@ -22,6 +22,7 @@
       ],
       'sources': [
         # interfaces
+        'include/bitrate_adjuster.h',
         'include/video_coding.h',
         'include/video_coding_defines.h',
 
@@ -54,6 +55,7 @@
         'video_coding_impl.h',
 
         # sources
+        'bitrate_adjuster.cc',
         'codec_database.cc',
         'codec_timer.cc',
         'content_metrics_processing.cc',

webrtc/video/receive_statistics_proxy.h

@@ -14,11 +14,11 @@
 #include <string>
 
 #include "webrtc/base/criticalsection.h"
+#include "webrtc/base/rate_statistics.h"
 #include "webrtc/base/ratetracker.h"
 #include "webrtc/base/thread_annotations.h"
 #include "webrtc/common_types.h"
 #include "webrtc/frame_callback.h"
-#include "webrtc/modules/remote_bitrate_estimator/rate_statistics.h"
 #include "webrtc/modules/video_coding/include/video_coding_defines.h"
 #include "webrtc/video/report_block_stats.h"
 #include "webrtc/video/vie_channel.h"