More reliable ALR detection

More reliable ALR detection

Previously AlrDetector was measuring amount of data sent in each 100ms
interval and would enter ALR mode after 5 consecutive intervals when
average bandwidth usage doesn't exceed 30% of the current estimate
estimate. This meant that an application that uses only slightely more
than 6% of total bandwidth may stay out of ALR mode, e.g. if it sends
a frame of size BW*30ms every 0.5 seconds. 100ms is too short interval
to average over, particularly when frame-rate falls below 10fps.

With this change AlrDetector averages BW usage over last 500ms. It then
enters ALR state when usage falls below 30% and exits it when usage
exceeds 50%.

BUG=webrtc:6332
R=philipel@webrtc.org, stefan@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/0182f85fd1f88974a3fac18679196b241a30d7c3

[Sergey Ulanov, 2016-11-15 08:33:57]

sergeyu@chromium.org changed reviewers:
+ philipel@webrtc.org, stefan@webrtc.org

[philipel, 2016-11-15 18:20:54]

lgtm with added unittests.

https://codereview.webrtc.org/2503643003/diff/1/webrtc/modules/pacing/alr_det...
File webrtc/modules/pacing/alr_detector.cc (right):

https://codereview.webrtc.org/2503643003/diff/1/webrtc/modules/pacing/alr_det...
webrtc/modules/pacing/alr_detector.cc:47: if (percentage < kAlrStartUsagePercent
&& !application_limited_) {
I think we should add a unittest to test this new behavior (also, I see that we
don't have a single unittest where we test if we are not in ALR, that would also
be good :))

[philipel, 2016-11-15 18:23:00]

https://codereview.webrtc.org/2503643003/diff/1/webrtc/modules/pacing/alr_det...
File webrtc/modules/pacing/alr_detector.cc (right):

https://codereview.webrtc.org/2503643003/diff/1/webrtc/modules/pacing/alr_det...
webrtc/modules/pacing/alr_detector.cc:47: if (percentage < kAlrStartUsagePercent
&& !application_limited_) {
On 2016/11/15 18:20:54, philipel wrote:
> I think we should add a unittest to test this new behavior (also, I see that
we
> don't have a single unittest where we test if we are not in ALR, that would
also
> be good :))

Oops, we do have a unittest where we are not in ALR. Still, we should add a
unittest for this new behavior.

[Sergey Ulanov, 2016-11-15 19:14:41]

On 2016/11/15 18:23:00, philipel wrote:
>
https://codereview.webrtc.org/2503643003/diff/1/webrtc/modules/pacing/alr_det...
> File webrtc/modules/pacing/alr_detector.cc (right):
> 
>
https://codereview.webrtc.org/2503643003/diff/1/webrtc/modules/pacing/alr_det...
> webrtc/modules/pacing/alr_detector.cc:47: if (percentage <
kAlrStartUsagePercent
> && !application_limited_) {
> On 2016/11/15 18:20:54, philipel wrote:
> > I think we should add a unittest to test this new behavior (also, I see that
> we
> > don't have a single unittest where we test if we are not in ALR, that would
> also
> > be good :))
> 
> Oops, we do have a unittest where we are not in ALR. Still, we should add a
> unittest for this new behavior.

Yes, it didn't have proper test coverage, particularly ALR->Normal transition
wasn't tested properly. Added more tests, particularly for the case when usage
spikes for short period of time.

[Sergey Ulanov, 2016-11-15 19:17:35]

Patchset #2 (id:20001) has been deleted

[stefan-webrtc, 2016-11-16 15:04:31]

rs-lgtm

https://codereview.webrtc.org/2503643003/diff/40001/webrtc/modules/pacing/alr...
File webrtc/modules/pacing/alr_detector.cc (right):

https://codereview.webrtc.org/2503643003/diff/40001/webrtc/modules/pacing/alr...
webrtc/modules/pacing/alr_detector.cc:59: bool
AlrDetector::InApplicationLimitedRegion() {
const

[Sergey Ulanov, 2016-11-16 18:23:18]

https://codereview.webrtc.org/2503643003/diff/40001/webrtc/modules/pacing/alr...
File webrtc/modules/pacing/alr_detector.cc (right):

https://codereview.webrtc.org/2503643003/diff/40001/webrtc/modules/pacing/alr...
webrtc/modules/pacing/alr_detector.cc:59: bool
AlrDetector::InApplicationLimitedRegion() {
On 2016/11/16 15:04:31, stefan-webrtc (holmer) wrote:
> const

Done.

[Sergey Ulanov, 2016-11-16 18:23:22]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-11-16 18:23:23]

The patchset sent to the CQ was uploaded after l-g-t-m from philipel@webrtc.org,
stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2503643003/#ps60001 (title:
"const")

[commit-bot: I haz the power, 2016-11-16 18:23:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-16 18:28:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-16 18:28:23]

Try jobs failed on following builders:
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/16289)

[Sergey Ulanov, 2016-11-16 20:11:37]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2016-11-16 20:11:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-16 20:22:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-16 20:22:54]

Try jobs failed on following builders:
  linux_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_baremetal/builds/...)

[Sergey Ulanov, 2016-11-16 23:28:48]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2016-11-16 23:28:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-16 23:34:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-16 23:34:32]

Try jobs failed on following builders:
  linux_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_baremetal/builds/...)

[commit-bot: I haz the power, 2016-11-16 23:42:24]

Description was changed from

==========
More reliable ALR detection

Previously AlrDetector was measuring amount of data sent in each 100ms
interval and would enter ALR mode after 5 consecutive intervals when
average bandwidth usage doesn't exceed 30% of the current estimate
estimate. This meant that an application that uses only slightely more
than 6% of total bandwidth may stay out of ALR mode, e.g. if it sends
a frame of size BW*30ms every 0.5 seconds. 100ms is too short interval
to average over, particularly when frame-rate falls below 10fps.

With this change AlrDetector averages BW usage over last 500ms. It then
enters ALR state when usage falls below 30% and exits it when usage
exceeds 50%.

BUG=webrtc:6332
==========

to

==========
More reliable ALR detection

Previously AlrDetector was measuring amount of data sent in each 100ms
interval and would enter ALR mode after 5 consecutive intervals when
average bandwidth usage doesn't exceed 30% of the current estimate
estimate. This meant that an application that uses only slightely more
than 6% of total bandwidth may stay out of ALR mode, e.g. if it sends
a frame of size BW*30ms every 0.5 seconds. 100ms is too short interval
to average over, particularly when frame-rate falls below 10fps.

With this change AlrDetector averages BW usage over last 500ms. It then
enters ALR state when usage falls below 30% and exits it when usage
exceeds 50%.

BUG=webrtc:6332
R=philipel@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/0182f85fd1f88974a3fac18679196b241a30d7c3
Cr-Commit-Position: refs/heads/master@{#15109}
==========

[commit-bot: I haz the power, 2016-11-16 23:42:26]

Patchset 3 (id:??) landed as
https://crrev.com/0182f85fd1f88974a3fac18679196b241a30d7c3
Cr-Commit-Position: refs/heads/master@{#15109}

[Sergey Ulanov, 2016-11-16 23:42:27]

Description was changed from

==========
More reliable ALR detection

Previously AlrDetector was measuring amount of data sent in each 100ms
interval and would enter ALR mode after 5 consecutive intervals when
average bandwidth usage doesn't exceed 30% of the current estimate
estimate. This meant that an application that uses only slightely more
than 6% of total bandwidth may stay out of ALR mode, e.g. if it sends
a frame of size BW*30ms every 0.5 seconds. 100ms is too short interval
to average over, particularly when frame-rate falls below 10fps.

With this change AlrDetector averages BW usage over last 500ms. It then
enters ALR state when usage falls below 30% and exits it when usage
exceeds 50%.

BUG=webrtc:6332
R=philipel@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/0182f85fd1f88974a3fac18679196b241a30d7c3
Cr-Commit-Position: refs/heads/master@{#15109}
==========

to

==========
More reliable ALR detection

Previously AlrDetector was measuring amount of data sent in each 100ms
interval and would enter ALR mode after 5 consecutive intervals when
average bandwidth usage doesn't exceed 30% of the current estimate
estimate. This meant that an application that uses only slightely more
than 6% of total bandwidth may stay out of ALR mode, e.g. if it sends
a frame of size BW*30ms every 0.5 seconds. 100ms is too short interval
to average over, particularly when frame-rate falls below 10fps.

With this change AlrDetector averages BW usage over last 500ms. It then
enters ALR state when usage falls below 30% and exits it when usage
exceeds 50%.

BUG=webrtc:6332
R=philipel@webrtc.org, stefan@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/0182f85fd1f88974a3fac1867...
==========

[Sergey Ulanov, 2016-11-16 23:42:28]

Committed patchset #3 (id:60001) manually as
0182f85fd1f88974a3fac18679196b241a30d7c3 (presubmit successful).