Set pacer target bitrate to max of bwe and bitrate allocation.

Set pacer target bitrate to max of bwe and bitrate allocation.

ChannelGroup::OnNetWorkChanged() should not configure the pacer to send
a lower bitrate than what bitrate_allocator has actually allocated (may
be the case if min_bitrate is enforced, for instance).

BUG=

Committed: https://crrev.com/2f48d9412440942133c637d1e47264d5179b9b51
Cr-Commit-Position: refs/heads/master@{#10519}

[sprang_webrtc, 2015-10-19 12:21:50]

sprang@webrtc.org changed reviewers:
+ mflodman@webrtc.org

[mflodman, 2015-10-20 10:10:45]

mflodman@webrtc.org changed reviewers:
+ stefan@webrtc.org

[mflodman, 2015-10-20 10:10:45]

A first look and looping in Stefan too.

https://codereview.webrtc.org/1413663004/diff/1/webrtc/video_engine/vie_chann...
File webrtc/video_engine/vie_channel_group.cc (right):

https://codereview.webrtc.org/1413663004/diff/1/webrtc/video_engine/vie_chann...
webrtc/video_engine/vie_channel_group.cc:385: target_bitrate_bps =
std::max(target_bitrate_bps, allocated_bitrate);
I'd prefer a new variable with another name for this one.

https://codereview.webrtc.org/1413663004/diff/1/webrtc/video_engine/vie_chann...
webrtc/video_engine/vie_channel_group.cc:385: target_bitrate_bps =
std::max(target_bitrate_bps, allocated_bitrate);
And also comment why this is done.

[stefan-webrtc, 2015-10-20 10:13:47]

Can we add a test for this too?

https://codereview.webrtc.org/1413663004/diff/1/webrtc/video_engine/vie_chann...
File webrtc/video_engine/vie_channel_group.cc (right):

https://codereview.webrtc.org/1413663004/diff/1/webrtc/video_engine/vie_chann...
webrtc/video_engine/vie_channel_group.cc:383: uint32_t allocated_bitrate =
bitrate_allocator_->OnNetworkChanged(
unit on allocated_bitrate

[sprang_webrtc, 2015-10-21 08:09:16]

https://codereview.webrtc.org/1413663004/diff/1/webrtc/video_engine/vie_chann...
File webrtc/video_engine/vie_channel_group.cc (right):

https://codereview.webrtc.org/1413663004/diff/1/webrtc/video_engine/vie_chann...
webrtc/video_engine/vie_channel_group.cc:383: uint32_t allocated_bitrate =
bitrate_allocator_->OnNetworkChanged(
On 2015/10/20 10:13:47, stefan-webrtc (holmer) wrote:
> unit on allocated_bitrate

Done.

https://codereview.webrtc.org/1413663004/diff/1/webrtc/video_engine/vie_chann...
webrtc/video_engine/vie_channel_group.cc:385: target_bitrate_bps =
std::max(target_bitrate_bps, allocated_bitrate);
On 2015/10/20 10:10:45, mflodman wrote:
> And also comment why this is done.

Done.

https://codereview.webrtc.org/1413663004/diff/1/webrtc/video_engine/vie_chann...
webrtc/video_engine/vie_channel_group.cc:385: target_bitrate_bps =
std::max(target_bitrate_bps, allocated_bitrate);
On 2015/10/20 10:10:45, mflodman wrote:
> I'd prefer a new variable with another name for this one.

Done.

[sprang_webrtc, 2015-10-21 08:11:21]

On 2015/10/20 10:13:47, stefan-webrtc (holmer) wrote:
> Can we add a test for this too?
> 

I added a vie_channel_group_unittest.cc (we had not tests for that previously?!)
and made it generic enough so that it'll be easy to add more tests in the
future.
But what's the plan for ChannelGroup? Do we want to remove the entire thing and
merge it with Call? In that case it might be overkill?

[mflodman, 2015-10-22 06:50:03]

So this code has moved quite a bit and I also plan to move bitrate allocator out
of CongestionController, formerly known as ChannelGroup. I think we should
discuss offline how we'd like to test this in the future.

[sprang_webrtc, 2015-11-04 10:44:26]

Moved this over to CongestionController and removed test for now. Let's get back
to that once we move this logic out to Call.

[stefan-webrtc, 2015-11-05 11:10:42]

lgtm

https://codereview.webrtc.org/1413663004/diff/60001/webrtc/call/congestion_co...
File webrtc/call/congestion_controller.cc (right):

https://codereview.webrtc.org/1413663004/diff/60001/webrtc/call/congestion_co...
webrtc/call/congestion_controller.cc:295: // set the pacer bitrate to the 
maxaimum of the two.
maximum

[sprang_webrtc, 2015-11-05 11:11:21]

The CQ bit was checked by sprang@webrtc.org

[commit-bot: I haz the power, 2015-11-05 11:11:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1413663004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1413663004/60001

[sprang_webrtc, 2015-11-05 11:23:57]

The CQ bit was unchecked by sprang@webrtc.org

[mflodman, 2015-11-05 11:27:16]

lgtm

[sprang_webrtc, 2015-11-05 11:35:39]

https://codereview.webrtc.org/1413663004/diff/60001/webrtc/call/congestion_co...
File webrtc/call/congestion_controller.cc (right):

https://codereview.webrtc.org/1413663004/diff/60001/webrtc/call/congestion_co...
webrtc/call/congestion_controller.cc:295: // set the pacer bitrate to the 
maxaimum of the two.
On 2015/11/05 11:10:42, stefan-webrtc (holmer) wrote:
> maximum

Done.

[sprang_webrtc, 2015-11-05 11:35:43]

The CQ bit was checked by sprang@webrtc.org

[sprang_webrtc, 2015-11-05 11:35:43]

The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org,
mflodman@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1413663004/#ps80001 (title:
"Fixed typo")

[commit-bot: I haz the power, 2015-11-05 11:35:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1413663004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1413663004/80001

[commit-bot: I haz the power, 2015-11-05 12:25:55]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-11-05 12:26:05]

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