fix wrong implementation of AimdRateControl::TimeToReduceFurther.

webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc

Index: webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc
diff --git a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc
index f4835f57c1f9e3e78f87bd278a1dfe9a7d6ea747..53245abd09f8ac74ee9592d446f975c1a2ebf21c 100644
--- a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc
+++ b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc
@@ -72,10 +72,9 @@ bool AimdRateControl::TimeToReduceFurther(int64_t time_now,
     return true;
   }
   if (ValidEstimate()) {
-    const int threshold = static_cast<int>(kWithinIncomingBitrateHysteresis *
-                                           incoming_bitrate_bps);
-    const int bitrate_difference = LatestEstimate() - incoming_bitrate_bps;
-    return bitrate_difference > threshold;
+    const uint32_t threshold = static_cast<uint32_t>
+        (kWithinIncomingBitrateHysteresis * LatestEstimate());
+    return incoming_bitrate_bps > threshold;

[stefan-webrtc, 2016/11/29 15:52:08]

Agree that it's wrong! I think the comment is wrong too, and that this should
say:
return incoming_bitrate_bps < 0.95 * threshold;

The reasoning is that, if the incoming bitrate is even lower than our estimate,
it's probably a good idea to further decrease the estimate.

Could I request that you add a unittest in aimd_rate_control_unittests.cc and
update the comment in the .h file?

[zhengpeng, 2016/11/30 04:33:11]

After reconsideration, I agree that if we are overusing and the actual bitrate
is very close to (or lower than) our lastest estimate, then it may be a good
opportunity to further reduce the bitrate estimate. 

"Hysteresis" implies it was originally intended to detect lags, since actual
changes always happen behind estimate changes.If this is the case, then I guess
it should be
return incoming_bitrate_bps < 1.05 * LatestEstimate;
It gives AimdRateControl early chance to reduce bitrate, which make bitrate
reduction more aggressive.

On the other hand, "return incoming_bitrate_bps < 0.95 * LatestEstimate" seems
more conservative, and the 5% losts the original meaning. 

Anyway, my patch is wrong, though it complies with the comment.
I will follow your suggested modification and re-commit patches.

   }
   return false;
 }