Unify rtcp packet setters

Unify rtcp packet setters
Renamed setters in rtcp classes
from WithField to SetField
from WithItem to AddItem or SetItems
from From to SetSenderSsrc
from To to SetMediaSsrc
Some redundant or unsued setters removed.
Pass-by-const& replaced with pass-by-value when appropriate.

BUG=webrtc:5260
Committed: https://crrev.com/20e77c7b8a9f19942ef3c3c4f1fa3888b2cd54ea
Cr-Commit-Position: refs/heads/master@{#14393}

[danilchap, 2016-09-16 08:03:35]

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

[danilchap, 2016-09-16 08:03:36]

Erik, before I update all the rtcp classed, do you agree this rename worth
doing?

[sprang_webrtc, 2016-09-16 08:28:15]

I'm a bit torn. On one hand I like the current naming style, it makes me long
for a more declarative world. But since they are in reality just plain setters,
and we also have the inline-style getter methods, it's a little bit confusing.
I'm fine with either, really.

I do however approve of changing From() and To() to SetSenderSsrc() and
SetMediaSsrc(), since the former sort of adds another layer of indirection to
mentally penetrate, at least when comparing notes between the code and the
specs...

[danilchap, 2016-09-16 14:33:01]

Description was changed from

==========
Rename setters in rtcp classes
from WithField to SetField
from WithItem to AddItem or SetItems
from From to SetSenderSsrc
from To to SetMediaSsrc

BUG=webrtc:5260
==========

to

==========
Unify rtcp packet setters
Rename setters in rtcp classes
from WithField to SetField
from WithItem to AddItem or SetItems
from From to SetSenderSsrc
from To to SetMediaSsrc
Some redundant or unsued setters removed.

BUG=webrtc:5260
==========

[danilchap, 2016-09-16 14:56:45]

Description was changed from

==========
Unify rtcp packet setters
Rename setters in rtcp classes
from WithField to SetField
from WithItem to AddItem or SetItems
from From to SetSenderSsrc
from To to SetMediaSsrc
Some redundant or unsued setters removed.

BUG=webrtc:5260
==========

to

==========
Unify rtcp packet setters
Renamed setters in rtcp classes
from WithField to SetField
from WithItem to AddItem or SetItems
from From to SetSenderSsrc
from To to SetMediaSsrc
Some redundant or unsued setters removed.
Pass-by-const& replaced with pass-by-value when appropriate.

BUG=webrtc:5260
==========

[danilchap, 2016-09-16 14:58:18]

On 2016/09/16 08:28:15, språng wrote:
> I'm a bit torn. On one hand I like the current naming style, it makes me long
> for a more declarative world. But since they are in reality just plain
setters,
> and we also have the inline-style getter methods, it's a little bit confusing.
> I'm fine with either, really.
> 
> I do however approve of changing From() and To() to SetSenderSsrc() and
> SetMediaSsrc(), since the former sort of adds another layer of indirection to
> mentally penetrate, at least when comparing notes between the code and the
> specs...

'With' should look nice when it doesn't change the internal state of the object,

but return you a new one With an asked change.
Here functions are clear setters. Or Adders. This CL make them look different
now.

ptal

[sprang_webrtc, 2016-09-16 15:37:18]

On 2016/09/16 14:58:18, danilchap wrote:
> On 2016/09/16 08:28:15, språng wrote:
> > I'm a bit torn. On one hand I like the current naming style, it makes me
long
> > for a more declarative world. But since they are in reality just plain
> setters,
> > and we also have the inline-style getter methods, it's a little bit
confusing.
> > I'm fine with either, really.
> > 
> > I do however approve of changing From() and To() to SetSenderSsrc() and
> > SetMediaSsrc(), since the former sort of adds another layer of indirection
to
> > mentally penetrate, at least when comparing notes between the code and the
> > specs...
> 
> 'With' should look nice when it doesn't change the internal state of the
object,
> but return you a new one With an asked change.
> Here functions are clear setters. Or Adders. This CL make them look different
> now.
> 
> ptal

Yes, that was my thinking too.
lgtm with nit

[sprang_webrtc, 2016-09-16 15:37:33]

https://codereview.webrtc.org/2348623003/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h
(right):

https://codereview.webrtc.org/2348623003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h:33: bool
SetJitters(std::vector<uint32_t> jitters);
nit: "jitters" is a somewhat overloaded word. Could we use "jitter items" or
something instead?

https://codereview.webrtc.org/2348623003/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/sdes.h (right):

https://codereview.webrtc.org/2348623003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/sdes.h:39: bool AddCName(uint32_t
ssrc, std::string cname);
Nice, now it actually says what it does :)

[danilchap, 2016-09-16 16:27:48]

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

[danilchap, 2016-09-16 16:27:49]

Stefan, can you approve this renames for
modules/pacing/
modules/remote_bitrate_estimator/
video/end_to_end_tests.cc

https://codereview.webrtc.org/2348623003/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h
(right):

https://codereview.webrtc.org/2348623003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h:33: bool
SetJitters(std::vector<uint32_t> jitters);
On 2016/09/16 15:37:33, språng wrote:
> nit: "jitters" is a somewhat overloaded word. Could we use "jitter items" or
> something instead?

how about jitter_value?
(item feels too big for single value, inter_arrival_jitter - too long)

[sprang_webrtc, 2016-09-18 19:13:58]

lgtm

https://codereview.webrtc.org/2348623003/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h
(right):

https://codereview.webrtc.org/2348623003/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h:33: bool
SetJitters(std::vector<uint32_t> jitters);
On 2016/09/16 16:27:49, danilchap wrote:
> On 2016/09/16 15:37:33, språng wrote:
> > nit: "jitters" is a somewhat overloaded word. Could we use "jitter items" or
> > something instead?
> 
> how about jitter_value?
> (item feels too big for single value, inter_arrival_jitter - too long)

Sure!

[stefan-webrtc, 2016-09-26 13:14:16]

lgtm, mostly looked at code not in rtp_rtcp.

[danilchap, 2016-09-27 07:44:37]

The CQ bit was checked by danilchap@webrtc.org

[danilchap, 2016-09-27 07:44:38]

The patchset sent to the CQ was uploaded after l-g-t-m from sprang@webrtc.org,
stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2348623003/#ps130001 (title:
"+call/rtc_event_log_unittest")

[commit-bot: I haz the power, 2016-09-27 07:44:48]

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-09-27 08:37:44]

Description was changed from

==========
Unify rtcp packet setters
Renamed setters in rtcp classes
from WithField to SetField
from WithItem to AddItem or SetItems
from From to SetSenderSsrc
from To to SetMediaSsrc
Some redundant or unsued setters removed.
Pass-by-const& replaced with pass-by-value when appropriate.

BUG=webrtc:5260
==========

to

==========
Unify rtcp packet setters
Renamed setters in rtcp classes
from WithField to SetField
from WithItem to AddItem or SetItems
from From to SetSenderSsrc
from To to SetMediaSsrc
Some redundant or unsued setters removed.
Pass-by-const& replaced with pass-by-value when appropriate.

BUG=webrtc:5260
==========

[commit-bot: I haz the power, 2016-09-27 08:37:45]

Committed patchset #8 (id:130001)

[commit-bot: I haz the power, 2016-09-27 08:37:57]

Description was changed from

==========
Unify rtcp packet setters
Renamed setters in rtcp classes
from WithField to SetField
from WithItem to AddItem or SetItems
from From to SetSenderSsrc
from To to SetMediaSsrc
Some redundant or unsued setters removed.
Pass-by-const& replaced with pass-by-value when appropriate.

BUG=webrtc:5260
==========

to

==========
Unify rtcp packet setters
Renamed setters in rtcp classes
from WithField to SetField
from WithItem to AddItem or SetItems
from From to SetSenderSsrc
from To to SetMediaSsrc
Some redundant or unsued setters removed.
Pass-by-const& replaced with pass-by-value when appropriate.

BUG=webrtc:5260

Committed: https://crrev.com/20e77c7b8a9f19942ef3c3c4f1fa3888b2cd54ea
Cr-Commit-Position: refs/heads/master@{#14393}
==========

[commit-bot: I haz the power, 2016-09-27 08:37:58]

Patchset 8 (id:??) landed as
https://crrev.com/20e77c7b8a9f19942ef3c3c4f1fa3888b2cd54ea
Cr-Commit-Position: refs/heads/master@{#14393}

[kjellander_webrtc, 2016-09-27 15:39:08]

A revert of this CL (patchset #8 id:130001) has been created in
https://codereview.webrtc.org/2372713005/ by kjellander@webrtc.org.

The reason for reverting is: Breaks compilation of internal downstream project..