Log to RtcEventLog when loss based estimate is changed.

Log to RtcEventLog when loss based estimate is changed.

Also include some minor refactoring.

BUG=webrtc:7466
Review-Url: https://codereview.webrtc.org/2815843002
Cr-Commit-Position: refs/heads/master@{#17740}
Committed: https://chromium.googlesource.com/external/webrtc/+/1b96531f9eb82d3f9ccf8ec2cda20a86cfaad656

[philipel, 2017-04-12 12:37:42]

Description was changed from

==========
Log to RtcEventLog when loss based estimate is changed.

BUG=none
==========

to

==========
Log to RtcEventLog when loss based estimate is changed.

Also include some minor refactoring.

BUG=none
==========

[philipel, 2017-04-12 12:41:07]

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

[terelius, 2017-04-12 13:51:09]

https://codereview.webrtc.org/2815843002/diff/20001/webrtc/modules/bitrate_co...
File webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc
(right):

https://codereview.webrtc.org/2815843002/diff/20001/webrtc/modules/bitrate_co...
webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc:411: void
SendSideBandwidthEstimation::CapAndLogBitrate(int64_t now_ms,
Since CapBitrateToThresholds should (presumably?) never be called without
executing this code, what do you think about moving the code below into that
function instead?

[philipel, 2017-04-12 14:10:09]

https://codereview.webrtc.org/2815843002/diff/20001/webrtc/modules/bitrate_co...
File webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc
(right):

https://codereview.webrtc.org/2815843002/diff/20001/webrtc/modules/bitrate_co...
webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc:411: void
SendSideBandwidthEstimation::CapAndLogBitrate(int64_t now_ms,
On 2017/04/12 13:51:08, terelius wrote:
> Since CapBitrateToThresholds should (presumably?) never be called without
> executing this code, what do you think about moving the code below into that
> function instead?

Good idea!

[philipel, 2017-04-18 10:30:57]

The CQ bit was checked by philipel@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-18 10:31:00]

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, 2017-04-18 10:52:57]

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

[commit-bot: I haz the power, 2017-04-18 10:52:58]

Dry run: This issue passed the CQ dry run.

[philipel, 2017-04-18 11:55:17]

ptal

[terelius, 2017-04-18 12:09:58]

lgtm

https://codereview.webrtc.org/2815843002/diff/40001/webrtc/modules/bitrate_co...
File webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc
(right):

https://codereview.webrtc.org/2815843002/diff/40001/webrtc/modules/bitrate_co...
webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc:387: void
SendSideBandwidthEstimation::CapAndLogBitrate(int64_t now_ms,
nit: I would actually prefer to keep the old function name. I don't think debug
logging is sufficiently important to be mentioned in the function name.

https://codereview.webrtc.org/2815843002/diff/40001/webrtc/modules/bitrate_co...
File webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h (right):

https://codereview.webrtc.org/2815843002/diff/40001/webrtc/modules/bitrate_co...
webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h:68: void
CapAndLogBitrate(int64_t now_ms, uint32_t bitrate_bps);
nit: Please add a comment that mentions clipping the bitrate to [min_bitrate,
max_bitrate], updating current_bitrate_bps_ and logging the update to event_log.

[philipel, 2017-04-18 12:55:12]

https://codereview.webrtc.org/2815843002/diff/40001/webrtc/modules/bitrate_co...
File webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc
(right):

https://codereview.webrtc.org/2815843002/diff/40001/webrtc/modules/bitrate_co...
webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc:387: void
SendSideBandwidthEstimation::CapAndLogBitrate(int64_t now_ms,
On 2017/04/18 12:09:58, terelius wrote:
> nit: I would actually prefer to keep the old function name. I don't think
debug
> logging is sufficiently important to be mentioned in the function name.

Done.

https://codereview.webrtc.org/2815843002/diff/40001/webrtc/modules/bitrate_co...
File webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h (right):

https://codereview.webrtc.org/2815843002/diff/40001/webrtc/modules/bitrate_co...
webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h:68: void
CapAndLogBitrate(int64_t now_ms, uint32_t bitrate_bps);
On 2017/04/18 12:09:58, terelius wrote:
> nit: Please add a comment that mentions clipping the bitrate to [min_bitrate,
> max_bitrate], updating current_bitrate_bps_ and logging the update to
event_log.

Done.

[philipel, 2017-04-18 12:55:17]

The CQ bit was checked by philipel@webrtc.org

[philipel, 2017-04-18 12:55:17]

The patchset sent to the CQ was uploaded after l-g-t-m from terelius@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2815843002/#ps60001 (title:
"Feedback")

[commit-bot: I haz the power, 2017-04-18 12:55:26]

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-04-18 13:55:33]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1492520117164830,
"parent_rev": "80fba25fc394f6a603ef452f6fb0768326079ce8", "commit_rev":
"1b96531f9eb82d3f9ccf8ec2cda20a86cfaad656"}

[commit-bot: I haz the power, 2017-04-18 13:55:36]

Description was changed from

==========
Log to RtcEventLog when loss based estimate is changed.

Also include some minor refactoring.

BUG=none
==========

to

==========
Log to RtcEventLog when loss based estimate is changed.

Also include some minor refactoring.

BUG=none

Review-Url: https://codereview.webrtc.org/2815843002
Cr-Commit-Position: refs/heads/master@{#17740}
Committed:
https://chromium.googlesource.com/external/webrtc/+/1b96531f9eb82d3f9ccf8ec2c...
==========

[commit-bot: I haz the power, 2017-04-18 13:55:37]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/external/webrtc/+/1b96531f9eb82d3f9ccf8ec2c...

[philipel, 2017-06-29 11:20:58]

Description was changed from

==========
Log to RtcEventLog when loss based estimate is changed.

Also include some minor refactoring.

BUG=none

Review-Url: https://codereview.webrtc.org/2815843002
Cr-Commit-Position: refs/heads/master@{#17740}
Committed:
https://chromium.googlesource.com/external/webrtc/+/1b96531f9eb82d3f9ccf8ec2c...
==========

to

==========
Log to RtcEventLog when loss based estimate is changed.

Also include some minor refactoring.

BUG=webrtc:7466

Review-Url: https://codereview.webrtc.org/2815843002
Cr-Commit-Position: refs/heads/master@{#17740}
Committed:
https://chromium.googlesource.com/external/webrtc/+/1b96531f9eb82d3f9ccf8ec2c...
==========