Update screen simulcast config and fix periodic encoder param update

webrtc/video/vie_encoder.cc

 /*
  *  Copyright (c) 2012 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.
  */
 
@@ skipping 658 matching lines @@
   initial_rampup_ = kMaxInitialFramedrop;
 
   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);
   }

[stefan-webrtc, 2017/04/05 15:06:48]

I don't immediately understand what the problem was here. Did we get in the
(missing) else clause? Should we not set last_parameters_update_ms_ if we run
ReconfigureEncoder()? After all, it does call
video_sender_.UpdateChannelParemeters().

[sprang_webrtc, 2017/05/08 08:50:53]

The intent for |last_parameters_update_ms_| is to indicate when we last called
video_sender_.UpdateChannelParemeters().
The second check above looks if more than one second has passed since last we
did that and if so we should update the encoder. The problem is that we now
reset that timestamp on every frame, so unless there is more than one second
between incoming frames that update will never be triggered.

It's true that ReconfigureEncoder() also updates the encoder, so if you want to
I can add an update to the timer there as well, but it's probably a rare
occasion so I don't the we risk updating the parameters too often...

[stefan-webrtc, 2017/05/08 11:09:40]

I don't know. Maybe it's good to be consistent? Otherwise I think we should
comment on why we don't set it, as it'll be very hard for someone new to the
code to understand this.

[sprang_webrtc, 2017/05/08 11:12:27]

Alright, added update after ReconfigureEncoder() as well.

-  last_parameters_update_ms_.emplace(now_ms);
 
   if (EncoderPaused()) {
     TraceFrameDropStart();
     return;
   }
   TraceFrameDropEnd();
 
   TRACE_EVENT_ASYNC_STEP0("webrtc", "Video", video_frame.render_time_ms(),
                           "Encode");
 
@@ skipping 280 matching lines @@
 void ViEEncoder::IncrementScaleCounter(int reason, int delta) {
   // Get the counters and validate. This may also lazily initialize the state.
   const std::vector<int>& counter = GetScaleCounters();
   if (delta < 0) {
     RTC_DCHECK_GE(counter[reason], delta);
   }
   scale_counters_[degradation_preference_][reason] += delta;
 }
 
 }  // namespace webrtc