Move BitrateAllocator from BitrateController logic to Call.

Move BitrateAllocator from BitrateController logic to Call.

This is a step on the way to have variable bitrate for audio and is
intended to be as much of a no-op as possible.

BUG=webrtc:5079
Committed: https://crrev.com/0e7e259ebd69993bb5670a991f43aa1b06c9bf9e
Cr-Commit-Position: refs/heads/master@{#10630}

[mflodman, 2015-11-12 13:36:17]

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

[mflodman, 2015-11-12 13:36:17]

Erik,
Would you mind taking a look at this CL?

[mflodman, 2015-11-12 13:52:50]

https://codereview.webrtc.org/1441673002/diff/1/webrtc/call/bitrate_allocator.cc
File webrtc/call/bitrate_allocator.cc (right):

https://codereview.webrtc.org/1441673002/diff/1/webrtc/call/bitrate_allocator...
webrtc/call/bitrate_allocator.cc:140:
static_cast<uint32_t>(bitrate_observers_.size());
call-folder has stricter compile settings than modules/bitrate_controller and
this fails to compile otherwise.

[sprang_webrtc, 2015-11-12 14:30:22]

https://codereview.webrtc.org/1441673002/diff/1/webrtc/call/call.cc
File webrtc/call/call.cc (right):

https://codereview.webrtc.org/1441673002/diff/1/webrtc/call/call.cc#newcode560
webrtc/call/call.cc:560: 
Wouldn't be even better to move this straight into the bitrate allocator? Feels
like the Call class is starting to get cluttered.
Though might not be straight-forward to get padding needed then..

https://codereview.webrtc.org/1441673002/diff/1/webrtc/call/congestion_contro...
File webrtc/call/congestion_controller.h (right):

https://codereview.webrtc.org/1441673002/diff/1/webrtc/call/congestion_contro...
webrtc/call/congestion_controller.h:62: int min_bitrate_kbps);
Nit: A bit redundant with the unit in the method name perhaps, at least since
the parameters have it already.

https://codereview.webrtc.org/1441673002/diff/1/webrtc/modules/remote_bitrate...
File webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc
(right):

https://codereview.webrtc.org/1441673002/diff/1/webrtc/modules/remote_bitrate...
webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc:12: 
Thanks for cleaning up after me :)

[mflodman, 2015-11-12 16:16:41]

Erik, PTAL.

https://codereview.webrtc.org/1441673002/diff/1/webrtc/call/call.cc
File webrtc/call/call.cc (right):

https://codereview.webrtc.org/1441673002/diff/1/webrtc/call/call.cc#newcode560
webrtc/call/call.cc:560: 
On 2015/11/12 14:30:22, språng wrote:
> Wouldn't be even better to move this straight into the bitrate allocator?
Feels
> like the Call class is starting to get cluttered.
> Though might not be straight-forward to get padding needed then..

I agree that is the right thing to do, but (and I should have written this in
the description...) I wanted to avoid adding a new interface to get the padding
now.

My thought was to start "including audio for real" and define this in a follow
up CL. I don't know exactly what this interface should look like yet and how to
include padding and how to weight different media bitrates yet. So I'd prefer to
keep this as a step on the right way.

WDYT?

https://codereview.webrtc.org/1441673002/diff/1/webrtc/call/congestion_contro...
File webrtc/call/congestion_controller.h (right):

https://codereview.webrtc.org/1441673002/diff/1/webrtc/call/congestion_contro...
webrtc/call/congestion_controller.h:62: int min_bitrate_kbps);
On 2015/11/12 14:30:22, språng wrote:
> Nit: A bit redundant with the unit in the method name perhaps, at least since
> the parameters have it already.

Done.

https://codereview.webrtc.org/1441673002/diff/1/webrtc/modules/remote_bitrate...
File webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc
(right):

https://codereview.webrtc.org/1441673002/diff/1/webrtc/modules/remote_bitrate...
webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc:12: 
On 2015/11/12 14:30:22, språng wrote:
> Thanks for cleaning up after me :)

[sprang_webrtc, 2015-11-12 16:30:33]

lgtm

https://codereview.webrtc.org/1441673002/diff/1/webrtc/call/call.cc
File webrtc/call/call.cc (right):

https://codereview.webrtc.org/1441673002/diff/1/webrtc/call/call.cc#newcode560
webrtc/call/call.cc:560: 
On 2015/11/12 16:16:40, mflodman wrote:
> On 2015/11/12 14:30:22, språng wrote:
> > Wouldn't be even better to move this straight into the bitrate allocator?
> Feels
> > like the Call class is starting to get cluttered.
> > Though might not be straight-forward to get padding needed then..
> 
> I agree that is the right thing to do, but (and I should have written this in
> the description...) I wanted to avoid adding a new interface to get the
padding
> now.
> 
> My thought was to start "including audio for real" and define this in a follow
> up CL. I don't know exactly what this interface should look like yet and how
to
> include padding and how to weight different media bitrates yet. So I'd prefer
to
> keep this as a step on the right way.
> 
> WDYT?

Fair enough!

[mflodman, 2015-11-12 16:33:17]

The CQ bit was checked by mflodman@webrtc.org

[commit-bot: I haz the power, 2015-11-12 16:33:36]

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

[commit-bot: I haz the power, 2015-11-12 16:34:30]

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

[commit-bot: I haz the power, 2015-11-12 16:34:31]

Try jobs failed on following builders:
  android_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_gn_rel/builds/6936)
  ios32_sim_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/3262)
  ios_arm64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/5567)
  mac_compile_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)
  mac_compile_x64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_x64_dbg/bui...)
  mac_x64_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_x64_gn_dbg/builds/5224)
  mac_x64_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_x64_gn_rel/builds/5162)

[mflodman, 2015-11-12 16:40:23]

The CQ bit was checked by mflodman@webrtc.org

[mflodman, 2015-11-12 16:40:24]

The patchset sent to the CQ was uploaded after l-g-t-m from sprang@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1441673002/#ps40001 (title:
"Rebase")

[commit-bot: I haz the power, 2015-11-12 16:40:40]

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

[commit-bot: I haz the power, 2015-11-12 18:40:50]

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

[commit-bot: I haz the power, 2015-11-12 18:40:51]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[mflodman, 2015-11-13 04:56:07]

The CQ bit was checked by mflodman@webrtc.org

[commit-bot: I haz the power, 2015-11-13 04:56:16]

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

[commit-bot: I haz the power, 2015-11-13 05:02:46]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-11-13 05:02:56]

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

[stefan-webrtc, 2015-11-13 08:29:26]

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

[stefan-webrtc, 2015-11-13 08:29:27]

nice change. i have one small request for follow-up. :)

https://codereview.webrtc.org/1441673002/diff/40001/webrtc/video/video_receiv...
File webrtc/video/video_receive_stream.cc (right):

https://codereview.webrtc.org/1441673002/diff/40001/webrtc/video/video_receiv...
webrtc/video/video_receive_stream.cc:21: #include
"webrtc/modules/bitrate_controller/include/bitrate_controller.h"
Why is this needed in video_receive_stream.cc?

https://codereview.webrtc.org/1441673002/diff/40001/webrtc/video/video_receiv...
webrtc/video/video_receive_stream.cc:165: CreateRtcpBandwidthObserver(),
Is it possible to follow up with removing or setting this ot nullptr if this is
a receive channel? Seems a bit odd that we include bitrate_controller.h in this
file.

[mflodman, 2015-11-13 08:35:36]

https://codereview.webrtc.org/1441673002/diff/40001/webrtc/video/video_receiv...
File webrtc/video/video_receive_stream.cc (right):

https://codereview.webrtc.org/1441673002/diff/40001/webrtc/video/video_receiv...
webrtc/video/video_receive_stream.cc:21: #include
"webrtc/modules/bitrate_controller/include/bitrate_controller.h"
On 2015/11/13 08:29:27, stefan-webrtc (holmer) wrote:
> Why is this needed in video_receive_stream.cc?

It is needed for the line of code you commented on below and was incorrectly not
added here previously.

https://codereview.webrtc.org/1441673002/diff/40001/webrtc/video/video_receiv...
webrtc/video/video_receive_stream.cc:165: CreateRtcpBandwidthObserver(),
On 2015/11/13 08:29:27, stefan-webrtc (holmer) wrote:
> Is it possible to follow up with removing or setting this ot nullptr if this
is
> a receive channel? Seems a bit odd that we include bitrate_controller.h in
this
> file.

Absolutely, I'll take a look at this and I totally agree about it's weird to
have it here.