Remove MimdRateControl and factories for RemoteBitrateEstimor.

Remove MimdRateControl and factories for RemoteBitrateEstimor.

BUG=
R=stefan@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/468e62a97426a8d001e9187f3ca1d1e43f80b970

[sprang_webrtc, 2015-06-25 15:56:35]

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

[sprang_webrtc, 2015-06-25 15:56:35]

For your enjoyment!

[sprang_webrtc, 2015-06-26 12:26:59]

Updated test cases

[stefan-webrtc, 2015-06-30 11:38:18]

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
File webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h
(right):

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h:22:
#include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h"
Needed?

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
File
webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h
(right):

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h:40:
struct Cluster {
Would you mind forward declaring this and Probe (if possible...)?

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
File
webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
(right):

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc:170:
new (&remote_rate_) AimdRateControl(min_bitrate);
What the...(!)

I would _really_ prefer to use a scoped_ptr instead of this. :)

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
File webrtc/modules/remote_bitrate_estimator/remote_rate_control.h (left):

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
webrtc/modules/remote_bitrate_estimator/remote_rate_control.h:36: // when
over-using.
Can you verify that this comment isn't getting lost now? May want to have it in
the aimd_rate_control.h instead.

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
File webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc (right):

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc:71:
last_feedback_ms_(-1),
Uninitialized before?

[sprang_webrtc, 2015-07-06 07:54:29]

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
File webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h
(right):

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h:22:
#include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h"
On 2015/06/30 11:38:18, stefan-webrtc (holmer) wrote:
> Needed?

Done.

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
File
webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h
(right):

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h:40:
struct Cluster {
On 2015/06/30 11:38:18, stefan-webrtc (holmer) wrote:
> Would you mind forward declaring this and Probe (if possible...)?

Since we have both std::list<Probe> and std::list<Cluster> below that won't
work.
Could of course change the interface to have list of pointers instead, but I
don't think that's worth it?
Could also move just implementation of the the methods to .cc but that's such as
small part...

Could we refactor this in a separate cl if necessary?

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
File
webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
(right):

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc:170:
new (&remote_rate_) AimdRateControl(min_bitrate);
On 2015/06/30 11:38:18, stefan-webrtc (holmer) wrote:
> What the...(!)
> 
> I would _really_ prefer to use a scoped_ptr instead of this. :)

Haha, oops. I was intending to do this but forgot about it.
In place constructors are fun! ;)

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
File webrtc/modules/remote_bitrate_estimator/remote_rate_control.h (left):

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
webrtc/modules/remote_bitrate_estimator/remote_rate_control.h:36: // when
over-using.
On 2015/06/30 11:38:18, stefan-webrtc (holmer) wrote:
> Can you verify that this comment isn't getting lost now? May want to have it
in
> the aimd_rate_control.h instead.

Comment was already in aimd_rate_control.h, but fixed another comment.

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
File webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc (right):

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
webrtc/modules/remote_bitrate_estimator/test/estimators/remb.cc:71:
last_feedback_ms_(-1),
On 2015/06/30 11:38:18, stefan-webrtc (holmer) wrote:
> Uninitialized before?

Yep. Just a drive by fixit.

[stefan-webrtc, 2015-07-06 08:37:29]

lgtm

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
File
webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h
(right):

https://codereview.webrtc.org/1208083002/diff/40001/webrtc/modules/remote_bit...
webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h:40:
struct Cluster {
On 2015/07/06 07:54:29, språng wrote:
> On 2015/06/30 11:38:18, stefan-webrtc (holmer) wrote:
> > Would you mind forward declaring this and Probe (if possible...)?
> 
> Since we have both std::list<Probe> and std::list<Cluster> below that won't
> work.
> Could of course change the interface to have list of pointers instead, but I
> don't think that's worth it?
> Could also move just implementation of the the methods to .cc but that's such
as
> small part...
> 
> Could we refactor this in a separate cl if necessary?

Acknowledged.

[sprang_webrtc, 2015-07-06 08:51:07]

Committed patchset #4 (id:60001) manually as
468e62a97426a8d001e9187f3ca1d1e43f80b970 (presubmit successful).