Remove TMMBRSet class

Remove TMMBRSet class
by cleaning RTCPReceiveInfo class
and following cleaning of RTCPReceiver::BoundingSet function.

BUG=webrtc:5565
Committed: https://crrev.com/2b616397de09f5d4cc10b4e538abccba83158df2
Cr-Commit-Position: refs/heads/master@{#13817}

[danilchap, 2016-08-17 10:57:20]

Patchset #2 (id:20001) has been deleted

[danilchap, 2016-08-17 10:57:27]

Patchset #2 (id:40001) has been deleted

[danilchap, 2016-08-17 10:57:33]

Patchset #1 (id:1) has been deleted

[danilchap, 2016-08-17 10:57:58]

Description was changed from

==========
Remove TMMBRSet class
by cleaning RTCPReceiveInfo classe
and following cleaning of RTCPReceiver::BoundingSet function.

BUG=webrtc:5565
==========

to

==========
Remove TMMBRSet class
by cleaning RTCPReceiveInfo class
and following cleaning of RTCPReceiver::BoundingSet function.

BUG=webrtc:5565
==========

[danilchap, 2016-08-17 11:04:20]

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

[danilchap, 2016-08-17 11:04:21]

Next cleanup CL.
Last one for TMMBRSet,
One of many for RTCPReceiver.
ptal.

[philipel, 2016-08-17 13:53:06]

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc (right):

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc:385: RTCPPacketInformation&
rtcpPacketInformation)
git cl format

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc:1051:
receiveInfo.tmmbn.push_back(
use emplace_back(SSRC, Max... * 1000, Measured...) instead.

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc (right):

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc:119:
RTCPReceiveInformation::~RTCPReceiveInformation() = default;
Any particular reason for not using a ctor to initialize values?

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc:125: for (auto& entry :
tmmbr_) {
How large do we expect |tmmbr_| to be? Should we maybe use a map instead?

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc:135: {sender_ssrc,
tmmbr_item.bitrate_bps(), tmmbr_item.packet_overhead()},
Implement TimedTmmbrItem(int64_t last, TmmbItem item) and use that instead.

[danilchap, 2016-08-17 14:56:06]

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc (right):

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc:385: RTCPPacketInformation&
rtcpPacketInformation)
On 2016/08/17 13:53:06, philipel wrote:
> git cl format

that would require
git cl format --full
and would clutter the CL.
better to do it in separate CL that has no manual changes.

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc:1051:
receiveInfo.tmmbn.push_back(
On 2016/08/17 13:53:05, philipel wrote:
> use emplace_back(SSRC, Max... * 1000, Measured...) instead.

Done.

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc (right):

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc:119:
RTCPReceiveInformation::~RTCPReceiveInformation() = default;
On 2016/08/17 13:53:06, philipel wrote:
> Any particular reason for not using a ctor to initialize values?

I find member initialization cleaner: harder to miss initializing a new member,
easier to change member order or name.

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc:125: for (auto& entry :
tmmbr_) {
On 2016/08/17 13:53:06, philipel wrote:
> How large do we expect |tmmbr_| to be? Should we maybe use a map instead?

Not large. One per one per participant that actively use tmmbr.
Most of the time - zero.
Using map though can make code cleaner.
(e.g. makes disagreement in next comment irrelevant)

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc:135: {sender_ssrc,
tmmbr_item.bitrate_bps(), tmmbr_item.packet_overhead()},
On 2016/08/17 13:53:06, philipel wrote:
> Implement TimedTmmbrItem(int64_t last, TmmbItem item) and use that instead.

Why?
Those strait-forward constructors were helpful when braced initialization was in
banned features.

[philipel, 2016-08-17 15:30:06]

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc (right):

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc:385: RTCPPacketInformation&
rtcpPacketInformation)
On 2016/08/17 14:56:05, danilchap wrote:
> On 2016/08/17 13:53:06, philipel wrote:
> > git cl format
> 
> that would require
> git cl format --full
> and would clutter the CL.
> better to do it in separate CL that has no manual changes.

Acknowledged.

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc (right):

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc:119:
RTCPReceiveInformation::~RTCPReceiveInformation() = default;
On 2016/08/17 14:56:05, danilchap wrote:
> On 2016/08/17 13:53:06, philipel wrote:
> > Any particular reason for not using a ctor to initialize values?
> 
> I find member initialization cleaner: harder to miss initializing a new
member,
> easier to change member order or name.

In that case remove the default ctor/dtor.

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc:125: for (auto& entry :
tmmbr_) {
On 2016/08/17 14:56:05, danilchap wrote:
> On 2016/08/17 13:53:06, philipel wrote:
> > How large do we expect |tmmbr_| to be? Should we maybe use a map instead?
> 
> Not large. One per one per participant that actively use tmmbr.
> Most of the time - zero.
> Using map though can make code cleaner.
> (e.g. makes disagreement in next comment irrelevant)

Acknowledged.

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc:135: {sender_ssrc,
tmmbr_item.bitrate_bps(), tmmbr_item.packet_overhead()},
On 2016/08/17 14:56:05, danilchap wrote:
> On 2016/08/17 13:53:06, philipel wrote:
> > Implement TimedTmmbrItem(int64_t last, TmmbItem item) and use that instead.
> 
> Why?
> Those strait-forward constructors were helpful when braced initialization was
in
> banned features.

If someone change TimedTmmbrItem or the TmmbrItem class itself then this will
still compile and the values will be incorrectly assigned, Very easy to break.

[danilchap, 2016-08-17 16:15:33]

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc (right):

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc:119:
RTCPReceiveInformation::~RTCPReceiveInformation() = default;
On 2016/08/17 15:30:06, philipel wrote:
> On 2016/08/17 14:56:05, danilchap wrote:
> > On 2016/08/17 13:53:06, philipel wrote:
> > > Any particular reason for not using a ctor to initialize values?
> > 
> > I find member initialization cleaner: harder to miss initializing a new
> member,
> > easier to change member order or name.
> 
> In that case remove the default ctor/dtor.

Ack

They are default, but complex. This kind of constructor/destuctor better be in
.cc file.
When line "configs -= [ "//build/config/clang:find_bad_constructs" ]" in
BUILD.gn file will be removed, absent or declaring this kind of constructor in
.h file will be an error.

https://codereview.webrtc.org/2254703003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc:135: {sender_ssrc,
tmmbr_item.bitrate_bps(), tmmbr_item.packet_overhead()},
On 2016/08/17 15:30:06, philipel wrote:
> On 2016/08/17 14:56:05, danilchap wrote:
> > On 2016/08/17 13:53:06, philipel wrote:
> > > Implement TimedTmmbrItem(int64_t last, TmmbItem item) and use that
instead.
> > 
> > Why?
> > Those strait-forward constructors were helpful when braced initialization
was
> in
> > banned features.
> 
> If someone change TimedTmmbrItem or the TmmbrItem class itself then this will
> still compile and the values will be incorrectly assigned, Very easy to break.

I guess you right. But this constructor doesn't look needed now.
Unless you think replacing vector with a map was a bad idea.

[philipel, 2016-08-18 11:15:07]

lgtm

[danilchap, 2016-08-18 13:09:36]

The CQ bit was checked by danilchap@webrtc.org

[commit-bot: I haz the power, 2016-08-18 13:09:43]

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, 2016-08-18 13:17:47]

Description was changed from

==========
Remove TMMBRSet class
by cleaning RTCPReceiveInfo class
and following cleaning of RTCPReceiver::BoundingSet function.

BUG=webrtc:5565
==========

to

==========
Remove TMMBRSet class
by cleaning RTCPReceiveInfo class
and following cleaning of RTCPReceiver::BoundingSet function.

BUG=webrtc:5565
==========

[commit-bot: I haz the power, 2016-08-18 13:17:48]

Committed patchset #2 (id:80001)

[commit-bot: I haz the power, 2016-08-18 13:17:56]

Description was changed from

==========
Remove TMMBRSet class
by cleaning RTCPReceiveInfo class
and following cleaning of RTCPReceiver::BoundingSet function.

BUG=webrtc:5565
==========

to

==========
Remove TMMBRSet class
by cleaning RTCPReceiveInfo class
and following cleaning of RTCPReceiver::BoundingSet function.

BUG=webrtc:5565

Committed: https://crrev.com/2b616397de09f5d4cc10b4e538abccba83158df2
Cr-Commit-Position: refs/heads/master@{#13817}
==========

[commit-bot: I haz the power, 2016-08-18 13:17:57]

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