Implement periodic bandwidth probing in application-limited region.

Implement periodic bandwidth probing in application-limited region.

Now ProbeController can send periodic bandwidth probes when in
application-limited region. This will allow to maintain correct
bottleneck bandwidth estimate, even not all bandwidth is being used.
The feature is not enabled by default, but can be enabled with a flag.
Interval between probes is currently set to 5 seconds.

BUG=webrtc:6332
Committed: https://crrev.com/80ed35e21cf747240778f27afceca1f2a3f704cc
Cr-Commit-Position: refs/heads/master@{#15279}

[Sergey Ulanov, 2016-11-16 00:41:32]

Patchset #1 (id:1) has been deleted

[Sergey Ulanov, 2016-11-16 00:42:25]

Patchset #1 (id:20001) has been deleted

[Sergey Ulanov, 2016-11-16 00:43:30]

Patchset #1 (id:40001) has been deleted

[Sergey Ulanov, 2016-11-16 00:44:25]

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

[philipel, 2016-11-16 14:59:16]

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/media/base/mediach...
File webrtc/media/base/mediachannel.h (right):

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/media/base/mediach...
webrtc/media/base/mediachannel.h:134: bool periodic_bandwidth_probing = false;
Rename to something like |periodic_alr_probing| or
|periodic_alr_bandwidth_probing|. Should be renamed on other relevant places as
well.

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/probe_controller.cc (right):

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:152:
pacer_->GetApplicationLimitedRegionStartTime() &&
I think it's cleaner to keep the InApplicationLimitedRegion function in addition
to the new GetApplicationLimitedRegionStartTime.

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:193: int
alr_probe_period_ms =
Probing is very cheap (but jittery) so I don't think think we need exponential
backoff. I think an interval of 5-10 seconds would work well.

Have you done any tests regarding this behavior?

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:197: int
time_since_last_probe_ms =
I'm not sure why you added the |now_ms| argument to InitiateProbing (since it's
not used), but don't you think it's better if we calculate the time since the
last probing was initiated?

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:208: int64_t now_ms,
Not used?

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/video/video_send_s...
File webrtc/video/video_send_stream.cc (right):

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/video/video_send_s...
webrtc/video/video_send_stream.cc:751: if (config_->periodic_bandwidth_probing)
congestion_controller_->EnablePeriodicProbing(config_->periodic_bandwidth_probing);

[Sergey Ulanov, 2016-11-21 09:06:03]

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/media/base/mediach...
File webrtc/media/base/mediachannel.h (right):

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/media/base/mediach...
webrtc/media/base/mediachannel.h:134: bool periodic_bandwidth_probing = false;
On 2016/11/16 14:59:15, philipel wrote:
> Rename to something like |periodic_alr_probing| or
> |periodic_alr_bandwidth_probing|. Should be renamed on other relevant places
as
> well.

Done.

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/probe_controller.cc (right):

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:152:
pacer_->GetApplicationLimitedRegionStartTime() &&
On 2016/11/16 14:59:15, philipel wrote:
> I think it's cleaner to keep the InApplicationLimitedRegion function in
addition
> to the new GetApplicationLimitedRegionStartTime.

I don't think keeping InApplicationLimitedRegion() next to
GetApplicationLimitedRegionStartTime() is the right thing to do, because
separating them makes it easier to use them incorrectly. Particularly because
pacer_ runs on a different threads, and so ALR state may change between the two
calls. Another alternative here is to have GetApplicationLimitedRegionState()
that would return both the bool state and start time, e.g. as a struct:
 struct AlrState {
   bool in_alr;
   int64_t start_time;
 }

But this looks like it adds more complexity than we need here. Another option is
to return rtc::Option<int64_t> from GetApplicationLimitedRegionStartTime().
WDYT?

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:193: int
alr_probe_period_ms =
On 2016/11/16 14:59:15, philipel wrote:
> Probing is very cheap (but jittery) so I don't think think we need exponential
> backoff. I think an interval of 5-10 seconds would work well.
> 
> Have you done any tests regarding this behavior?

The idea here was that the longer the sender stays in ALR the less likely it is
that it will need the extra bandwidth, but yes, it's premature to implement
exponential backoff here. Changed it to regular interval of 5 second. Will see
how well that works.

What did you mean by "jittery"?

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:197: int
time_since_last_probe_ms =
On 2016/11/16 14:59:15, philipel wrote:
> I'm not sure why you added the |now_ms| argument to InitiateProbing (since
it's
> not used), 
That was a mistake, fixed that function to use now_ms.

> but don't you think it's better if we calculate the time since the
> last probing was initiated?

Done. Also simplified this code a bit.

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:208: int64_t now_ms,
On 2016/11/16 14:59:15, philipel wrote:
> Not used?

Done.

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/video/video_send_s...
File webrtc/video/video_send_stream.cc (right):

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/video/video_send_s...
webrtc/video/video_send_stream.cc:751: if (config_->periodic_bandwidth_probing)
On 2016/11/16 14:59:15, philipel wrote:
>
congestion_controller_->EnablePeriodicProbing(config_->periodic_bandwidth_probing);

Done.

[Sergey Ulanov, 2016-11-21 09:06:39]

Description was changed from

==========
Implement periodic bandwidth probing in application-limited region.

Now ProbeController can send periodic bandwidth probes when in
application-limited region. This will allow to maintain correct
bottleneck bandwidth estimate, even not all bandwidth is being used.
The feature is not enabled by default, but can be enabled with a flag.
Interval between probes increases exponentially from 5 seconds to 2
minutes, so probes are sent at 5s, 10s, 20s, 40s, 80s, 160s, 280s,
400s, and so on.

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

to

==========
Implement periodic bandwidth probing in application-limited region.

Now ProbeController can send periodic bandwidth probes when in
application-limited region. This will allow to maintain correct
bottleneck bandwidth estimate, even not all bandwidth is being used.
The feature is not enabled by default, but can be enabled with a flag.
Interval between probes is currently set to 5 seconds.

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

[Sergey Ulanov, 2016-11-21 09:28:34]

Patchset #2 (id:80001) has been deleted

[philipel, 2016-11-21 12:05:43]

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/probe_controller.cc (right):

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:152:
pacer_->GetApplicationLimitedRegionStartTime() &&
On 2016/11/21 09:06:03, Sergey Ulanov wrote:
> On 2016/11/16 14:59:15, philipel wrote:
> > I think it's cleaner to keep the InApplicationLimitedRegion function in
> addition
> > to the new GetApplicationLimitedRegionStartTime.
> 
> I don't think keeping InApplicationLimitedRegion() next to
> GetApplicationLimitedRegionStartTime() is the right thing to do, because
> separating them makes it easier to use them incorrectly. Particularly because
> pacer_ runs on a different threads, and so ALR state may change between the
two
> calls. Another alternative here is to have GetApplicationLimitedRegionState()
> that would return both the bool state and start time, e.g. as a struct:
>  struct AlrState {
>    bool in_alr;
>    int64_t start_time;
>  }
> 
> But this looks like it adds more complexity than we need here. Another option
is
> to return rtc::Option<int64_t> from GetApplicationLimitedRegionStartTime().
> WDYT?

I see what you mean about having both InApplicationLimitedRegion and
GetApplicationLimitedRegionStartTime, so I agree it's better to only have
GetApplicationLimitedRegionStartTime.

I think using rtc::Optional is the best option in this case (we want to move
away from magic constants).

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:193: int
alr_probe_period_ms =
On 2016/11/21 09:06:03, Sergey Ulanov wrote:
> On 2016/11/16 14:59:15, philipel wrote:
> > Probing is very cheap (but jittery) so I don't think think we need
exponential
> > backoff. I think an interval of 5-10 seconds would work well.
> > 
> > Have you done any tests regarding this behavior?
> 
> The idea here was that the longer the sender stays in ALR the less likely it
is
> that it will need the extra bandwidth, but yes, it's premature to implement
> exponential backoff here. Changed it to regular interval of 5 second. Will see
> how well that works.
> 
> What did you mean by "jittery"?

Lets say we have a stable link that does not change, probing this link will
result in estimates that are fairly jittery, i.e. you will get different results
every time (sometimes even higher than the actual available bandwidth).

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/video/video_send_s...
File webrtc/video/video_send_stream.cc (right):

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/video/video_send_s...
webrtc/video/video_send_stream.cc:751: if (config_->periodic_bandwidth_probing)
On 2016/11/21 09:06:03, Sergey Ulanov wrote:
> On 2016/11/16 14:59:15, philipel wrote:
> >
>
congestion_controller_->EnablePeriodicProbing(config_->periodic_bandwidth_probing);
> 
> Done.

Why do we make an assumption about the default value of ALR probing here? If we
change the default then you wont be able to disable the ALR probing using the
config.

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
File webrtc/modules/congestion_controller/include/congestion_controller.h
(right):

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/include/congestion_controller.h:85: void
EnablePeriodicProbing(bool enable);
EnablePeriodicAlrProbing

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
File webrtc/modules/congestion_controller/probe_controller.cc (right):

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/probe_controller.cc:185: if (now_ms -
alr_start_time >= kAlrPeriodicProbingIntervalMs &&
I think something like this is a bit clearer:
int64_t next_probe_time_ms = std::max(alr_start_time,
time_last_probing_initiated_ms_) + kAlrPeriodicProbingIntervalMs;
if (next_probe_time_ms <= now_ms) ...

not super important though...

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/probe_controller.cc:195: int64_t now_ms,
I don't see why this change is necessary or makes things cleaner.

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
File webrtc/modules/congestion_controller/probe_controller.h (right):

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/probe_controller.h:39: void
EnablePeriodicProbing(bool enable);
EnablePeriodicAlrProbing

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/probe_controller.h:69: bool
enable_periodic_probing_ GUARDED_BY(critsect_);
enable_periodic_alr_probing_

[Sergey Ulanov, 2016-11-21 19:28:19]

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/probe_controller.cc (right):

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:152:
pacer_->GetApplicationLimitedRegionStartTime() &&
On 2016/11/21 12:05:42, philipel wrote:
> On 2016/11/21 09:06:03, Sergey Ulanov wrote:
> > On 2016/11/16 14:59:15, philipel wrote:
> > > I think it's cleaner to keep the InApplicationLimitedRegion function in
> > addition
> > > to the new GetApplicationLimitedRegionStartTime.
> > 
> > I don't think keeping InApplicationLimitedRegion() next to
> > GetApplicationLimitedRegionStartTime() is the right thing to do, because
> > separating them makes it easier to use them incorrectly. Particularly
because
> > pacer_ runs on a different threads, and so ALR state may change between the
> two
> > calls. Another alternative here is to have
GetApplicationLimitedRegionState()
> > that would return both the bool state and start time, e.g. as a struct:
> >  struct AlrState {
> >    bool in_alr;
> >    int64_t start_time;
> >  }
> > 
> > But this looks like it adds more complexity than we need here. Another
option
> is
> > to return rtc::Option<int64_t> from GetApplicationLimitedRegionStartTime().
> > WDYT?
> 
> I see what you mean about having both InApplicationLimitedRegion and
> GetApplicationLimitedRegionStartTime, so I agree it's better to only have
> GetApplicationLimitedRegionStartTime.
> 
> I think using rtc::Optional is the best option in this case (we want to move
> away from magic constants).

Done.

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:193: int
alr_probe_period_ms =
On 2016/11/21 12:05:42, philipel wrote:
> Lets say we have a stable link that does not change, probing this link will
> result in estimates that are fairly jittery, i.e. you will get different
results
> every time (sometimes even higher than the actual available bandwidth).

Right, there is a lot of noise in the probes result, but I don't think it's an
issue in ALR, it won't be noticeable to the user.

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
File webrtc/modules/congestion_controller/include/congestion_controller.h
(right):

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/include/congestion_controller.h:85: void
EnablePeriodicProbing(bool enable);
On 2016/11/21 12:05:42, philipel wrote:
> EnablePeriodicAlrProbing

Done.

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
File webrtc/modules/congestion_controller/probe_controller.cc (right):

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/probe_controller.cc:185: if (now_ms -
alr_start_time >= kAlrPeriodicProbingIntervalMs &&
On 2016/11/21 12:05:42, philipel wrote:
> I think something like this is a bit clearer:
> int64_t next_probe_time_ms = std::max(alr_start_time,
> time_last_probing_initiated_ms_) + kAlrPeriodicProbingIntervalMs;
> if (next_probe_time_ms <= now_ms) ...
> 
> not super important though...

Done.

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/probe_controller.cc:195: int64_t now_ms,
On 2016/11/21 12:05:42, philipel wrote:
> I don't see why this change is necessary or makes things cleaner. 

TimeInMilliseconds() calls are not free, each of them requires a system call, so
it's wasteful to call it more then necessary.

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
File webrtc/modules/congestion_controller/probe_controller.h (right):

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/probe_controller.h:39: void
EnablePeriodicProbing(bool enable);
On 2016/11/21 12:05:43, philipel wrote:
> EnablePeriodicAlrProbing

Done.

https://codereview.webrtc.org/2504023002/diff/100001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/probe_controller.h:69: bool
enable_periodic_probing_ GUARDED_BY(critsect_);
On 2016/11/21 12:05:43, philipel wrote:
> enable_periodic_alr_probing_

Done.

[philipel, 2016-11-22 10:55:10]

lgtm when final comment has been addressed.

https://codereview.webrtc.org/2504023002/diff/120001/webrtc/modules/pacing/al...
File webrtc/modules/pacing/alr_detector.cc (right):

https://codereview.webrtc.org/2504023002/diff/120001/webrtc/modules/pacing/al...
webrtc/modules/pacing/alr_detector.cc:61: return
rtc::Optional<int64_t>(alr_started_time_ms_);
We always return a value? That doesn't look right.

if (alr_started_time_ms_ == 0)
  return rtc::Optional<int64_t>();
return rtc::Optional<int64_t>(alr_started_time_ms_);

[philipel, 2016-11-22 12:31:37]

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/video/video_send_s...
File webrtc/video/video_send_stream.cc (right):

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/video/video_send_s...
webrtc/video/video_send_stream.cc:751: if (config_->periodic_bandwidth_probing)
On 2016/11/21 12:05:42, philipel wrote:
> On 2016/11/21 09:06:03, Sergey Ulanov wrote:
> > On 2016/11/16 14:59:15, philipel wrote:
> > >
> >
>
congestion_controller_->EnablePeriodicProbing(config_->periodic_bandwidth_probing);
> > 
> > Done.
> 
> Why do we make an assumption about the default value of ALR probing here? If
we
> change the default then you wont be able to disable the ALR probing using the
> config.

Also, PTAL at this comment.

[Sergey Ulanov, 2016-11-22 23:19:52]

Address feedback

[Sergey Ulanov, 2016-11-22 23:20:31]

Address feedback

[Sergey Ulanov, 2016-11-22 23:20:41]

Patchset #5 (id:160001) has been deleted

[Sergey Ulanov, 2016-11-22 23:21:00]

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/video/video_send_s...
File webrtc/video/video_send_stream.cc (right):

https://codereview.webrtc.org/2504023002/diff/60001/webrtc/video/video_send_s...
webrtc/video/video_send_stream.cc:751: if (config_->periodic_bandwidth_probing)
On 2016/11/22 12:31:37, philipel wrote:
> On 2016/11/21 12:05:42, philipel wrote:
> > On 2016/11/21 09:06:03, Sergey Ulanov wrote:
> > > On 2016/11/16 14:59:15, philipel wrote:
> > > >
> > >
> >
>
congestion_controller_->EnablePeriodicProbing(config_->periodic_bandwidth_probing);
> > > 
> > > Done.
> > 
> > Why do we make an assumption about the default value of ALR probing here? If
> we
> > change the default then you wont be able to disable the ALR probing using
the
> > config.
> 
> Also, PTAL at this comment.

Fixed now. Sorry I missed it last time.

https://codereview.webrtc.org/2504023002/diff/120001/webrtc/modules/pacing/al...
File webrtc/modules/pacing/alr_detector.cc (right):

https://codereview.webrtc.org/2504023002/diff/120001/webrtc/modules/pacing/al...
webrtc/modules/pacing/alr_detector.cc:61: return
rtc::Optional<int64_t>(alr_started_time_ms_);
On 2016/11/22 10:55:10, philipel wrote:
> We always return a value? That doesn't look right.
> 
> if (alr_started_time_ms_ == 0)
>   return rtc::Optional<int64_t>();
> return rtc::Optional<int64_t>(alr_started_time_ms_);

Done.

[Sergey Ulanov, 2016-11-22 23:21:24]

The CQ bit was checked by sergeyu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-22 23:21:33]

Dry run: 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-22 23:24:44]

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

[commit-bot: I haz the power, 2016-11-22 23:24:45]

Dry run: Try jobs failed on following builders:
  android_more_configs on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_more_configs/bu...)

[stefan-webrtc, 2016-11-28 13:27:13]

lgtm % comments.

https://codereview.webrtc.org/2504023002/diff/140001/webrtc/modules/congestio...
File webrtc/modules/congestion_controller/probe_controller.cc (right):

https://codereview.webrtc.org/2504023002/diff/140001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/probe_controller.cc:194:
estimated_bitrate_bps_ * 5 / 4);
Make 5 / 4 a name constant and share on line 136.

https://codereview.webrtc.org/2504023002/diff/140001/webrtc/modules/congestio...
File webrtc/modules/congestion_controller/probe_controller.h (right):

https://codereview.webrtc.org/2504023002/diff/140001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/probe_controller.h:16: #include
"webrtc/base/optional.h"
Not used

https://codereview.webrtc.org/2504023002/diff/140001/webrtc/modules/pacing/al...
File webrtc/modules/pacing/alr_detector.cc (right):

https://codereview.webrtc.org/2504023002/diff/140001/webrtc/modules/pacing/al...
webrtc/modules/pacing/alr_detector.cc:62: ? rtc::Optional<int64_t>()
I would have made alr_started_time_ms_ an rtc::Optional<int64_t>. WDYT?

[Sergey Ulanov, 2016-11-28 20:39:00]

https://codereview.webrtc.org/2504023002/diff/140001/webrtc/modules/congestio...
File webrtc/modules/congestion_controller/probe_controller.cc (right):

https://codereview.webrtc.org/2504023002/diff/140001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/probe_controller.cc:194:
estimated_bitrate_bps_ * 5 / 4);
On 2016/11/28 13:27:13, stefan-webrtc (holmer) wrote:
> Make 5 / 4 a name constant and share on line 136.

Done.

https://codereview.webrtc.org/2504023002/diff/140001/webrtc/modules/congestio...
File webrtc/modules/congestion_controller/probe_controller.h (right):

https://codereview.webrtc.org/2504023002/diff/140001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/probe_controller.h:16: #include
"webrtc/base/optional.h"
On 2016/11/28 13:27:13, stefan-webrtc (holmer) wrote:
> Not used

Done.

https://codereview.webrtc.org/2504023002/diff/140001/webrtc/modules/pacing/al...
File webrtc/modules/pacing/alr_detector.cc (right):

https://codereview.webrtc.org/2504023002/diff/140001/webrtc/modules/pacing/al...
webrtc/modules/pacing/alr_detector.cc:62: ? rtc::Optional<int64_t>()
On 2016/11/28 13:27:13, stefan-webrtc (holmer) wrote:
> I would have made alr_started_time_ms_ an rtc::Optional<int64_t>. WDYT?

Done.

[Sergey Ulanov, 2016-11-28 20:39:25]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-11-28 20:39:25]

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/2504023002/#ps170001 (title:
"address feedback")

[commit-bot: I haz the power, 2016-11-28 20:39:37]

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-28 21:11:14]

CQ is committing da patch.

Bot data: {"patchset_id": 170001, "attempt_start_ts": 1480365565444260,
"parent_rev": "28a8c8ba3ca2aa818b7887a4d692a57a8e9ef4b3", "commit_rev":
"716d30a89a3f456c293a9e6f4d5b48d10ce677c7"}

[commit-bot: I haz the power, 2016-11-28 21:11:17]

Description was changed from

==========
Implement periodic bandwidth probing in application-limited region.

Now ProbeController can send periodic bandwidth probes when in
application-limited region. This will allow to maintain correct
bottleneck bandwidth estimate, even not all bandwidth is being used.
The feature is not enabled by default, but can be enabled with a flag.
Interval between probes is currently set to 5 seconds.

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

to

==========
Implement periodic bandwidth probing in application-limited region.

Now ProbeController can send periodic bandwidth probes when in
application-limited region. This will allow to maintain correct
bottleneck bandwidth estimate, even not all bandwidth is being used.
The feature is not enabled by default, but can be enabled with a flag.
Interval between probes is currently set to 5 seconds.

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

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

Committed patchset #5 (id:170001)

[commit-bot: I haz the power, 2016-11-28 21:11:29]

Description was changed from

==========
Implement periodic bandwidth probing in application-limited region.

Now ProbeController can send periodic bandwidth probes when in
application-limited region. This will allow to maintain correct
bottleneck bandwidth estimate, even not all bandwidth is being used.
The feature is not enabled by default, but can be enabled with a flag.
Interval between probes is currently set to 5 seconds.

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

to

==========
Implement periodic bandwidth probing in application-limited region.

Now ProbeController can send periodic bandwidth probes when in
application-limited region. This will allow to maintain correct
bottleneck bandwidth estimate, even not all bandwidth is being used.
The feature is not enabled by default, but can be enabled with a flag.
Interval between probes is currently set to 5 seconds.

BUG=webrtc:6332

Committed: https://crrev.com/80ed35e21cf747240778f27afceca1f2a3f704cc
Cr-Commit-Position: refs/heads/master@{#15279}
==========

[commit-bot: I haz the power, 2016-11-28 21:11:31]

Patchset 5 (id:??) landed as
https://crrev.com/80ed35e21cf747240778f27afceca1f2a3f704cc
Cr-Commit-Position: refs/heads/master@{#15279}