Periodically update channel parameters and send TargetBitrate message.

Currently, parameters are periodically updated, but the TargetBitrate
message is only sent if a new bitrate is set. It should be sent
periodically to indicate the signaled bitrate is valid and to prevent
stale values due to loss of RTCP packets.

BUG=webrtc:6897

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

webrtc/modules/video_coding/video_sender.cc

@@ -63,12 +63,6 @@ void VideoSender::Process() {
       send_stats_callback_->SendStatistics(bitRate, frameRate);
     }
   }
-  {
-    rtc::CritScope cs(&params_crit_);
-    // Force an encoder parameters update, so that incoming frame rate is
-    // updated even if bandwidth hasn't changed.
-    encoder_params_.input_frame_rate = _mediaOpt.InputFrameRate();
-  }
 }
 
 int64_t VideoSender::TimeUntilNextProcess() {

webrtc/modules/video_coding/video_sender_unittest.cc

@@ -317,22 +317,6 @@ TEST_F(TestVideoSenderWithMockEncoder, TestSetRate) {
   AddFrame();
   clock_.AdvanceTimeMilliseconds(kFrameIntervalMs);
 
-  // Add enough frames so that input frame rate will be updated.
-  const int kFramesToSend =
-      (VCMProcessTimer::kDefaultProcessIntervalMs / kFrameIntervalMs) + 1;
-  for (int i = 0; i < kFramesToSend; ++i) {
-    AddFrame();
-    clock_.AdvanceTimeMilliseconds(kFrameIntervalMs);
-  }
-
-  EXPECT_CALL(encoder_,
-              SetRateAllocation(new_rate_allocation, kActualFrameRate))
-      .Times(1)
-      .WillOnce(Return(0));
-
-  sender_->Process();
-  AddFrame();
-
   // Expect no call to encoder_.SetRates if the new bitrate is zero.
   EXPECT_CALL(encoder_, SetRateAllocation(_, _)).Times(0);
   sender_->SetChannelParameters(0, 0, 200, rate_allocator_.get(), nullptr);
@@ -376,23 +360,6 @@ TEST_F(TestVideoSenderWithMockEncoder, TestEncoderParametersForInternalSource) {
                                 rate_allocator_.get(), nullptr);
 }
 
-TEST_F(TestVideoSenderWithMockEncoder, EncoderFramerateUpdatedViaProcess) {
-  sender_->SetChannelParameters(settings_.startBitrate * 1000, 0, 200,
-                                rate_allocator_.get(), nullptr);
-  const int64_t kRateStatsWindowMs = 2000;
-  const uint32_t kInputFps = 20;
-  int64_t start_time = clock_.TimeInMilliseconds();
-  while (clock_.TimeInMilliseconds() < start_time + kRateStatsWindowMs) {
-    AddFrame();
-    clock_.AdvanceTimeMilliseconds(1000 / kInputFps);
-  }
-  EXPECT_CALL(encoder_, SetRateAllocation(_, kInputFps))
-      .Times(1)
-      .WillOnce(Return(0));
-  sender_->Process();
-  AddFrame();
-}
-
 TEST_F(TestVideoSenderWithMockEncoder,
        NoRedundantSetChannelParameterOrSetRatesCalls) {
   const uint8_t kLossRate = 4;
@@ -412,13 +379,7 @@ TEST_F(TestVideoSenderWithMockEncoder,
     AddFrame();
     clock_.AdvanceTimeMilliseconds(1000 / kInputFps);
   }
-  // After process, input framerate should be updated but not ChannelParameters
-  // as they are the same as before.
-  EXPECT_CALL(encoder_, SetRateAllocation(_, kInputFps))
-      .Times(1)
-      .WillOnce(Return(0));
-  sender_->Process();
-  AddFrame();
+
   // Call to SetChannelParameters with changed bitrate should call encoder
   // SetRates but not encoder SetChannelParameters (that are unchanged).
   uint32_t new_bitrate_bps = 2 * settings_.startBitrate * 1000;

webrtc/video/vie_encoder.cc

@@ -545,9 +545,16 @@ void ViEEncoder::EncodeVideoFrame(const VideoFrame& video_frame,
                  << ", texture=" << last_frame_info_->is_texture;
   }
 
+  int64_t now_ms = clock_->TimeInMilliseconds();
   if (pending_encoder_reconfiguration_) {
     ReconfigureEncoder();
+  } else if (!last_parameters_update_ms_ ||
+             now_ms - *last_parameters_update_ms_ >=
+                 vcm::VCMProcessTimer::kDefaultProcessIntervalMs) {
+    video_sender_.UpdateChannelParemeters(rate_allocator_.get(),
+                                          bitrate_observer_);
   }
+  last_parameters_update_ms_.emplace(now_ms);
 
   if (EncoderPaused()) {
     TraceFrameDropStart();

webrtc/video/vie_encoder.h

@@ -235,6 +235,7 @@ class ViEEncoder : public rtc::VideoSinkInterface<VideoFrame>,
   int dropped_frame_count_ ACCESS_ON(&encoder_queue_);
 
   VideoBitrateAllocationObserver* bitrate_observer_ ACCESS_ON(&encoder_queue_);
+  rtc::Optional<int64_t> last_parameters_update_ms_ ACCESS_ON(&encoder_queue_);
 
   // All public methods are proxied to |encoder_queue_|. It must must be
   // destroyed first to make sure no tasks are run that use other members.

webrtc/video/vie_encoder_unittest.cc

@@ -13,10 +13,13 @@
 
 #include "webrtc/api/video/i420_buffer.h"
 #include "webrtc/base/logging.h"
+#include "webrtc/modules/video_coding/utility/default_video_bitrate_allocator.h"
 #include "webrtc/system_wrappers/include/metrics_default.h"
+#include "webrtc/system_wrappers/include/sleep.h"
 #include "webrtc/test/encoder_settings.h"
 #include "webrtc/test/fake_encoder.h"
 #include "webrtc/test/frame_generator.h"
+#include "webrtc/test/gmock.h"
 #include "webrtc/test/gtest.h"
 #include "webrtc/video/send_statistics_proxy.h"
 #include "webrtc/video/vie_encoder.h"
@@ -35,6 +38,8 @@ namespace webrtc {
 
 using DegredationPreference = VideoSendStream::DegradationPreference;
 using ScaleReason = ScalingObserverInterface::ScaleReason;
+using ::testing::_;
+using ::testing::Return;
 
 namespace {
 const size_t kMaxPayloadLength = 1440;
@@ -163,19 +168,20 @@ class ViEEncoderTest : public ::testing::Test {
     ConfigureEncoder(std::move(video_encoder_config), nack_enabled);
   }
 
-  VideoFrame CreateFrame(int64_t ntp_ts, rtc::Event* destruction_event) const {
+  VideoFrame CreateFrame(int64_t ntp_time_ms,
+                         rtc::Event* destruction_event) const {
     VideoFrame frame(new rtc::RefCountedObject<TestBuffer>(
                          destruction_event, codec_width_, codec_height_),
                      99, 99, kVideoRotation_0);
-    frame.set_ntp_time_ms(ntp_ts);
+    frame.set_ntp_time_ms(ntp_time_ms);
     return frame;
   }
 
-  VideoFrame CreateFrame(int64_t ntp_ts, int width, int height) const {
+  VideoFrame CreateFrame(int64_t ntp_time_ms, int width, int height) const {
     VideoFrame frame(
         new rtc::RefCountedObject<TestBuffer>(nullptr, width, height), 99, 99,
         kVideoRotation_0);
-    frame.set_ntp_time_ms(ntp_ts);
+    frame.set_ntp_time_ms(ntp_time_ms);
     return frame;
   }
 
@@ -979,4 +985,48 @@ TEST_F(ViEEncoderTest, UMACpuLimitedResolutionInPercent) {
       1, metrics::NumEvents("WebRTC.Video.CpuLimitedResolutionInPercent", 50));
 }
 
+TEST_F(ViEEncoderTest, CallsBitrateObserver) {
+  class MockBitrateObserver : public VideoBitrateAllocationObserver {
+   public:
+    MOCK_METHOD1(OnBitrateAllocationUpdated, void(const BitrateAllocation&));
+  } bitrate_observer;
+  vie_encoder_->SetBitrateObserver(&bitrate_observer);
+
+  const int kDefaultFps = 30;
+  const BitrateAllocation expected_bitrate =
+      DefaultVideoBitrateAllocator(fake_encoder_.codec_config())
+          .GetAllocation(kTargetBitrateBps, kDefaultFps);
+
+  // First called on bitrate updated, then again on first frame.
+  EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(expected_bitrate))
+      .Times(2);
+  vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
+
+  const int64_t kStartTimeMs = 1;
+  video_source_.IncomingCapturedFrame(
+      CreateFrame(kStartTimeMs, codec_width_, codec_height_));
+  sink_.WaitForEncodedFrame(kStartTimeMs);
+
+  // Not called on second frame.
+  EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(expected_bitrate))
+      .Times(0);
+  video_source_.IncomingCapturedFrame(
+      CreateFrame(kStartTimeMs + 1, codec_width_, codec_height_));
+  sink_.WaitForEncodedFrame(kStartTimeMs + 1);
+
+  // Called after a process interval.
+  const int64_t kProcessIntervalMs =
+      vcm::VCMProcessTimer::kDefaultProcessIntervalMs;
+  // TODO(sprang): ViEEncoder should die and/or get injectable clock.
+  // Sleep for one processing interval plus one frame to avoid flakiness.
+  SleepMs(kProcessIntervalMs + 1000 / kDefaultFps);
+  EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(expected_bitrate))
+      .Times(1);
+  video_source_.IncomingCapturedFrame(CreateFrame(
+      kStartTimeMs + kProcessIntervalMs, codec_width_, codec_height_));
+  sink_.WaitForEncodedFrame(kStartTimeMs + kProcessIntervalMs);
+
+  vie_encoder_->Stop();
+}
+
 }  // namespace webrtc