Add WebRTC.BWE.MidCallProbing.* metrics.

Add WebRTC.BWE.MidCallProbing.* metrics.

BUG=webrtc:6984
Review-Url: https://codereview.webrtc.org/2629893003
Cr-Commit-Position: refs/heads/master@{#16100}
Committed: https://chromium.googlesource.com/external/webrtc/+/2df07342ee471f3f55bc54fb43de6d72c56bea43

[philipel, 2017-01-13 15:12:39]

philipel@webrtc.org changed reviewers:
+ terelius@webrtc.org

[terelius, 2017-01-16 13:48:01]

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

https://codereview.webrtc.org/2629893003/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:94: // max bitrate then
the probing attempt was successful.
I'd like to unify this class with the ProbeBitrateEstimator (and maybe
BitrateProber too?) so we would always know whether the probe has succeeded.
That way we could avoid this hack.

https://codereview.webrtc.org/2629893003/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:195: if (state_ ==
State::kWaitingForProbingResult) {
Can you be in mid_call_probing_waiting_for_result_ without being in
State::kWaitingForProbingResult? I.e, is there any reason to reset
mid_call_probing_waiting_for_result_ before checking state_ ==
State::kWaitingForProbingResult?

[philipel, 2017-01-16 13:56:20]

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

https://codereview.webrtc.org/2629893003/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:94: // max bitrate then
the probing attempt was successful.
On 2017/01/16 13:48:01, terelius wrote:
> I'd like to unify this class with the ProbeBitrateEstimator (and maybe
> BitrateProber too?) so we would always know whether the probe has succeeded.
> That way we could avoid this hack.

I agree that we could change the structure of the bandwidth estimation but that
would require major refactoring.

https://codereview.webrtc.org/2629893003/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:195: if (state_ ==
State::kWaitingForProbingResult) {
On 2017/01/16 13:48:01, terelius wrote:
> Can you be in mid_call_probing_waiting_for_result_ without being in
> State::kWaitingForProbingResult? I.e, is there any reason to reset
> mid_call_probing_waiting_for_result_ before checking state_ ==
> State::kWaitingForProbingResult?

The only time we are in State::kWaitingForProbingResult is when we are waiting
for a new estimate (which we interpret as a successful probing attempt) to see
if we should continue with the exponential probing.

That means |mid_call_probing_waiting_for_result_| can be true even if we are not
in State::kWaitingForProbingResult since mid call probing is not exponential.

[terelius, 2017-01-16 14:07:43]

lgtm

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

https://codereview.webrtc.org/2629893003/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/probe_controller.cc:195: if (state_ ==
State::kWaitingForProbingResult) {
On 2017/01/16 13:56:20, philipel wrote:
> On 2017/01/16 13:48:01, terelius wrote:
> > Can you be in mid_call_probing_waiting_for_result_ without being in
> > State::kWaitingForProbingResult? I.e, is there any reason to reset
> > mid_call_probing_waiting_for_result_ before checking state_ ==
> > State::kWaitingForProbingResult?
> 
> The only time we are in State::kWaitingForProbingResult is when we are waiting
> for a new estimate (which we interpret as a successful probing attempt) to see
> if we should continue with the exponential probing.
> 
> That means |mid_call_probing_waiting_for_result_| can be true even if we are
not
> in State::kWaitingForProbingResult since mid call probing is not exponential.

Acknowledged.

[philipel, 2017-01-16 14:10:39]

The CQ bit was checked by philipel@webrtc.org

[commit-bot: I haz the power, 2017-01-16 14:10:48]

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, 2017-01-16 17:31:50]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484575839741940,
"parent_rev": "00c7ad165ed8369aaa8d4975d67ae708a4a795f2", "commit_rev":
"2df07342ee471f3f55bc54fb43de6d72c56bea43"}

[commit-bot: I haz the power, 2017-01-16 17:31:56]

Description was changed from

==========
Add WebRTC.BWE.MidCallProbing.* metrics.

BUG=webrtc:6984
==========

to

==========
Add WebRTC.BWE.MidCallProbing.* metrics.

BUG=webrtc:6984

Review-Url: https://codereview.webrtc.org/2629893003
Cr-Commit-Position: refs/heads/master@{#16100}
Committed:
https://chromium.googlesource.com/external/webrtc/+/2df07342ee471f3f55bc54fb4...
==========

[commit-bot: I haz the power, 2017-01-16 17:31:57]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/external/webrtc/+/2df07342ee471f3f55bc54fb4...