Style cleanups in RtpSender.

Style cleanups in RtpSender.

- Renamed variables and some function to comply with style guide.
- Removed default argument values.
- Removed some dead code.
- Cleaned up comments formatting in rtp_rtcp.h

R=danilchap@webrtc.org, stefan@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/ec4f068bcdbc044f14162fb530b50dc45db01211

[Sergey Ulanov, 2016-06-14 23:35:19]

Patchset #1 (id:1) has been deleted

[Sergey Ulanov, 2016-06-14 23:35:32]

sergeyu@chromium.org changed reviewers:
+ pbos@chromium.org

[Sergey Ulanov, 2016-06-14 23:53:21]

The CQ bit was checked by sergeyu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-14 23:53:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2067673004/20001

[commit-bot: I haz the power, 2016-06-14 23:56:43]

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

[commit-bot: I haz the power, 2016-06-14 23:56:44]

Dry run: Try jobs failed on following builders:
  mac_compile_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)

[Sergey Ulanov, 2016-06-15 00:07:39]

The CQ bit was checked by sergeyu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-15 00:07:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2067673004/40001

[commit-bot: I haz the power, 2016-06-15 00:18:17]

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

[commit-bot: I haz the power, 2016-06-15 00:18:18]

Dry run: Try jobs failed on following builders:
  win_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_rel/builds/15810)

[danilchap, 2016-06-15 13:40:00]

danilchap@webrtc.org changed reviewers:
+ danilchap@webrtc.org
- pbos@chromium.org

[danilchap, 2016-06-15 13:40:01]

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc (right):

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc:48:
RtpRtcp::Configuration::Configuration() {}
: receive_statistics(NullObjectReceiveStatistics())

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender.h (left):

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.h:49: virtual int32_t
BuildRTPheader(uint8_t* data_buffer,
this function (with this spelling) is used outside webrtc.
may be deprecated the spelling instead of just changing it.

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.h:250: const bool timestamp_provided =
true,
there is use (outside webrtc) of this function with timestamp_provided = false,
inc_sequence_number = false

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.h:303: size_t SendPadData(size_t
bytes,
this version is used outside webrtc, (with timestamp provided), do not remove it

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h (right):

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h:46: int32_t
SetAudioPacketSize(uint16_t acket_size_samples_);
p

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc (right):

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc:38: delta_fec_params_ =
FecProtectionParams{0, 1, kFecMaskRandom};
may be initialize this variables inside class definition or in the initialize
list instead of the body of the constructor.

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender_video.h (right):

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_video.h:43: const int8_t
payload_type);
while chanching this lines, may be also change types from "const <ValueType>" to
"<ValueType>" in the declarations

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_video.h:97: RTPSenderInterface*
rtp_sender_;
RTPSenderInterface* const

[danilchap, 2016-06-15 13:41:15]

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h (right):

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h:46: int32_t
SetAudioPacketSize(uint16_t acket_size_samples_);
On 2016/06/15 13:40:01, danilchap wrote:
> p

and remove _

[Sergey Ulanov, 2016-06-15 18:26:33]

Patchset #3 (id:60001) has been deleted

[Sergey Ulanov, 2016-06-15 18:27:05]

Description was changed from

==========
Style cleanups in RtpSender.

- Renamed variables and some function to comply with style guide.
- Removed default argument values.
- Removed some dead code.
==========

to

==========
Style cleanups in RtpSender.

- Renamed variables and some function to comply with style guide.
- Removed default argument values.
- Removed some dead code.
- Cleaned up comments formatting in rtp_rtcp.h
==========

[Sergey Ulanov, 2016-06-15 18:27:53]

Also cleaned up comments format in rtp_rtcp.h now.

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc (right):

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc:48:
RtpRtcp::Configuration::Configuration() {}
On 2016/06/15 13:40:01, danilchap wrote:
> : receive_statistics(NullObjectReceiveStatistics())

Thanks for catching it - I meant to leave it, but lost it somehow anyway.

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender.h (left):

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.h:49: virtual int32_t
BuildRTPheader(uint8_t* data_buffer,
On 2016/06/15 13:40:01, danilchap wrote:
> this function (with this spelling) is used outside webrtc.
> may be deprecated the spelling instead of just changing it.

Done.

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.h:250: const bool timestamp_provided =
true,
On 2016/06/15 13:40:01, danilchap wrote:
> there is use (outside webrtc) of this function with timestamp_provided =
false,
> inc_sequence_number = false

Added deprecated version with these parameters. Note that inc_sequence_number is
currently ignored and I kept that behavior.

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.h:303: size_t SendPadData(size_t
bytes,
On 2016/06/15 13:40:01, danilchap wrote:
> this version is used outside webrtc, (with timestamp provided), do not remove
it

Done.

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h (right):

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h:46: int32_t
SetAudioPacketSize(uint16_t acket_size_samples_);
On 2016/06/15 13:41:15, danilchap wrote:
> On 2016/06/15 13:40:01, danilchap wrote:
> > p
> 
> and remove _

Done.

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc (right):

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc:38: delta_fec_params_ =
FecProtectionParams{0, 1, kFecMaskRandom};
On 2016/06/15 13:40:01, danilchap wrote:
> may be initialize this variables inside class definition or in the initialize
> list instead of the body of the constructor.

done

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender_video.h (right):

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_video.h:43: const int8_t
payload_type);
On 2016/06/15 13:40:01, danilchap wrote:
> while chanching this lines, may be also change types from "const <ValueType>"
to
> "<ValueType>" in the declarations

Done.

https://codereview.webrtc.org/2067673004/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_video.h:97: RTPSenderInterface*
rtp_sender_;
On 2016/06/15 13:40:01, danilchap wrote:
> RTPSenderInterface* const

Done.

[danilchap, 2016-06-15 20:04:19]

danilchap@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org

[danilchap, 2016-06-15 20:04:22]

+henrik for checking audio names and owner overview.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/i...
File webrtc/modules/rtp_rtcp/include/rtp_rtcp.h (right):

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:125: // Returns max data payload
length, which is acombination of the configuration
a combination

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:229: uint32_t timeStamp,
timestamp

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:233: const RTPFragmentationHeader*
fragmentation = NULL,
= nullptr

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:284: // Returns current RTT
(round-trip time) estimate .
I think it doesn't return an estimate, but a measured rtt.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:384: //                mostly because
we want a function that actually send
an extra space to align with line above would be nice

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:461: // send a request for a keyframe
Send (uppercase 1st word) and add a '.' at the end.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
File webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h (right):

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:47: int32_t(const uint8_t*
incomingPacket, size_t packetLength));
since you are fixing most variable names, maybe you want to fix all of them in
this mock

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:55:
MOCK_METHOD1(RegisterSendTransport, int32_t(Transport* outgoingTransport));
1 here

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:67: int32_t(const RTPExtensionType
type, uint8_t id));
const RTPExtensionType -> RTPExtensionType

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:93: void(uint32_t* totalRate,
4 here

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:118: void(RtcpIntraFrameObserver*
intraFrameCallback,
2 here

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:156: int32_t(uint8_t subType,
1 here

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:160:
MOCK_METHOD1(SetRTCPVoIPMetrics, int32_t(const RTCPVoIPMetric* void_metric));
voip (not void) metric

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:172: MOCK_METHOD2(SendNACK,
int32_t(const uint16_t* nackList, uint16_t size));
1 here

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:201: int32_t(const
KeyFrameRequestMethod method));
const KeyFrameRequestMethod -> KeyFrameRequestMethod

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:1259: rtc::CritScope
lock(&send_critsect_);
this is not a thread-safe way to keep old behavior.
But feel free to ignore timestamp_provided parameter.
(rtp_sender is not officially public interface, and the use I have in mind
doesn't really care what timestamp will be written)

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc (right):

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc:260: uint32_t oldTimeStamp =
rtp_sender_->Timestamp();
old_timestamp

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h (right):

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h:68: uint32_t dtmfTimeStamp,
dtmf_timestamp

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h:72: bool MarkerBit(const
FrameType frame_type, const int8_t payload_type);
const can be remove from both parameters

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc (right):

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc:163: delta_fec_params_ =
FecProtectionParams();
why revert from = FecProtectionParams{0, 1, kFecMaskRandom) this two lines?

[Sergey Ulanov, 2016-06-15 20:53:24]

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/i...
File webrtc/modules/rtp_rtcp/include/rtp_rtcp.h (right):

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:125: // Returns max data payload
length, which is acombination of the configuration
On 2016/06/15 20:04:21, danilchap wrote:
> a combination

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:229: uint32_t timeStamp,
On 2016/06/15 20:04:21, danilchap wrote:
> timestamp

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:233: const RTPFragmentationHeader*
fragmentation = NULL,
On 2016/06/15 20:04:21, danilchap wrote:
> = nullptr

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:284: // Returns current RTT
(round-trip time) estimate .
On 2016/06/15 20:04:21, danilchap wrote:
> I think it doesn't return an estimate, but a measured rtt.

AFAICT it's an average of several RTT measurements calculated here:
https://codesearch.chromium.org/chromium/src/third_party/webrtc/video/call_st...

In either case I think it's correct to call it an estimate as we don't control
the network and don't know what's the actual RTT for any given pair of packets.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:384: //                mostly because
we want a function that actually send
On 2016/06/15 20:04:21, danilchap wrote:
> an extra space to align with line above would be nice

Removed the spaces for consistency with other TODOs.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:461: // send a request for a keyframe
On 2016/06/15 20:04:21, danilchap wrote:
> Send (uppercase 1st word) and add a '.' at the end.

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
File webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h (right):

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:47: int32_t(const uint8_t*
incomingPacket, size_t packetLength));
On 2016/06/15 20:04:21, danilchap wrote:
> since you are fixing most variable names, maybe you want to fix all of them in
> this mock

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:55:
MOCK_METHOD1(RegisterSendTransport, int32_t(Transport* outgoingTransport));
On 2016/06/15 20:04:21, danilchap wrote:
> 1 here

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:67: int32_t(const RTPExtensionType
type, uint8_t id));
On 2016/06/15 20:04:21, danilchap wrote:
> const RTPExtensionType -> RTPExtensionType

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:93: void(uint32_t* totalRate,
On 2016/06/15 20:04:21, danilchap wrote:
> 4 here

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:118: void(RtcpIntraFrameObserver*
intraFrameCallback,
On 2016/06/15 20:04:21, danilchap wrote:
> 2 here

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:156: int32_t(uint8_t subType,
On 2016/06/15 20:04:21, danilchap wrote:
> 1 here

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:160:
MOCK_METHOD1(SetRTCPVoIPMetrics, int32_t(const RTCPVoIPMetric* void_metric));
On 2016/06/15 20:04:21, danilchap wrote:
> voip (not void) metric

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:172: MOCK_METHOD2(SendNACK,
int32_t(const uint16_t* nackList, uint16_t size));
On 2016/06/15 20:04:21, danilchap wrote:
> 1 here

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/m...
webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h:201: int32_t(const
KeyFrameRequestMethod method));
On 2016/06/15 20:04:21, danilchap wrote:
> const KeyFrameRequestMethod -> KeyFrameRequestMethod

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:1259: rtc::CritScope
lock(&send_critsect_);
On 2016/06/15 20:04:21, danilchap wrote:
> this is not a thread-safe way to keep old behavior.
> But feel free to ignore timestamp_provided parameter.
> (rtp_sender is not officially public interface, and the use I have in mind
> doesn't really care what timestamp will be written)

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc (right):

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.cc:260: uint32_t oldTimeStamp =
rtp_sender_->Timestamp();
On 2016/06/15 20:04:21, danilchap wrote:
> old_timestamp

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h (right):

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h:68: uint32_t dtmfTimeStamp,
On 2016/06/15 20:04:21, danilchap wrote:
> dtmf_timestamp

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h:72: bool MarkerBit(const
FrameType frame_type, const int8_t payload_type);
On 2016/06/15 20:04:22, danilchap wrote:
> const can be remove from both parameters

Done.

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc (right):

https://codereview.webrtc.org/2067673004/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc:163: delta_fec_params_ =
FecProtectionParams();
On 2016/06/15 20:04:22, danilchap wrote:
> why revert from = FecProtectionParams{0, 1, kFecMaskRandom) this two lines?

That was a mistake. Changed it back to FecProtectionParams{0, 1,
kFecMaskRandom}.

[danilchap, 2016-06-16 08:20:18]

lgtm

[Sergey Ulanov, 2016-06-20 20:41:50]

sergeyu@chromium.org changed reviewers:
+ mflodman@webrtc.org

[Sergey Ulanov, 2016-06-20 20:42:42]

+mflodman,
Magnus, can you please approve?

[Sergey Ulanov, 2016-06-24 00:52:15]

ping

[hlundin-webrtc, 2016-06-27 12:52:03]

henrik.lundin@webrtc.org changed reviewers:
- henrik.lundin@webrtc.org

[hlundin-webrtc, 2016-06-27 12:52:04]

As discussed previously offline, I'm not the one to do this review.

[Sergey Ulanov, 2016-07-01 22:06:47]

sergeyu@chromium.org changed reviewers:
+ holmer@chromium.org

[Sergey Ulanov, 2016-07-01 22:06:48]

+holmer@
Stefan, can you please approve?

[Sergey Ulanov, 2016-07-26 19:26:13]

sergeyu@chromium.org changed reviewers:
+ stefan@webrtc.org
- holmer@chromium.org

[Sergey Ulanov, 2016-07-26 19:26:51]

stefan@: ping

[stefan-webrtc, 2016-07-28 09:31:42]

Just a few nits first which I think we may want to take the opportunity to
address.

lgtm otherwise.

https://codereview.webrtc.org/2067673004/diff/100001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/include/rtp_rtcp.h (right):

https://codereview.webrtc.org/2067673004/diff/100001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:409:
***************************************************************************/
Could you clean up these mixed style comments?

https://codereview.webrtc.org/2067673004/diff/100001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtp_sender.h (right):

https://codereview.webrtc.org/2067673004/diff/100001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtp_sender.h:61: int8_t payload_type,
Maybe change this to int?

https://codereview.webrtc.org/2067673004/diff/100001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtp_sender.h:262: int8_t payload_type,
same here

https://codereview.webrtc.org/2067673004/diff/100001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h (right):

https://codereview.webrtc.org/2067673004/diff/100001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h:31: int8_t payload_type,
int here too

[Sergey Ulanov, 2016-07-28 18:01:34]

https://codereview.webrtc.org/2067673004/diff/100001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/include/rtp_rtcp.h (right):

https://codereview.webrtc.org/2067673004/diff/100001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/include/rtp_rtcp.h:409:
***************************************************************************/
On 2016/07/28 09:31:42, stefan-webrtc (holmer) wrote:
> Could you clean up these mixed style comments?

Done.

https://codereview.webrtc.org/2067673004/diff/100001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtp_sender.h (right):

https://codereview.webrtc.org/2067673004/diff/100001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtp_sender.h:61: int8_t payload_type,
On 2016/07/28 09:31:42, stefan-webrtc (holmer) wrote:
> Maybe change this to int?

There are many other places where int8_t is used payload_type. Changing them all
would make this CL much bigger, so I'd prefer to have it changed separately.

https://codereview.webrtc.org/2067673004/diff/100001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h (right):

https://codereview.webrtc.org/2067673004/diff/100001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h:31: int8_t payload_type,
On 2016/07/28 09:31:42, stefan-webrtc (holmer) wrote:
> int here too

Prefer to have it fixed in a separate CL.

[Sergey Ulanov, 2016-07-28 18:04:46]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-07-28 18:04:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
danilchap@webrtc.org, stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2067673004/#ps140001 (title:
"sync")

[commit-bot: I haz the power, 2016-07-28 18:04:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[Sergey Ulanov, 2016-07-28 18:41:56]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-07-28 18:41:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
danilchap@webrtc.org, stefan@webrtc.org
Link to the patchset: https://codereview.chromium.org/2067673004/#ps160001
(title: "fix windows")

[commit-bot: I haz the power, 2016-07-28 18:42:04]

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-07-28 19:12:29]

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

[commit-bot: I haz the power, 2016-07-28 19:12:30]

Try jobs failed on following builders:
  win_x64_gn_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_gn_rel/builds/1...)

[Sergey Ulanov, 2016-07-28 19:31:49]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-07-28 19:31:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
danilchap@webrtc.org, stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2067673004/#ps180001 (title:
"fix windows")

[commit-bot: I haz the power, 2016-07-28 19:32:00]

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-07-28 19:53:47]

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

[commit-bot: I haz the power, 2016-07-28 19:53:49]

Try jobs failed on following builders:
  win_x64_gn_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_gn_rel/builds/1...)

[Sergey Ulanov, 2016-07-28 20:30:15]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-07-28 20:30:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
danilchap@webrtc.org, stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2067673004/#ps200001 (title:
".")

[commit-bot: I haz the power, 2016-07-28 20:30:20]

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-07-28 21:56:28]

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

[commit-bot: I haz the power, 2016-07-28 21:56:30]

Try jobs failed on following builders:
  android_arm64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_arm64_rel/build...)

[commit-bot: I haz the power, 2016-07-28 22:19:21]

Description was changed from

==========
Style cleanups in RtpSender.

- Renamed variables and some function to comply with style guide.
- Removed default argument values.
- Removed some dead code.
- Cleaned up comments formatting in rtp_rtcp.h
==========

to

==========
Style cleanups in RtpSender.

- Renamed variables and some function to comply with style guide.
- Removed default argument values.
- Removed some dead code.
- Cleaned up comments formatting in rtp_rtcp.h

R=danilchap@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/ec4f068bcdbc044f14162fb530b50dc45db01211
Cr-Commit-Position: refs/heads/master@{#13565}
==========

[commit-bot: I haz the power, 2016-07-28 22:19:23]

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

[Sergey Ulanov, 2016-07-28 22:19:23]

Description was changed from

==========
Style cleanups in RtpSender.

- Renamed variables and some function to comply with style guide.
- Removed default argument values.
- Removed some dead code.
- Cleaned up comments formatting in rtp_rtcp.h

R=danilchap@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/ec4f068bcdbc044f14162fb530b50dc45db01211
Cr-Commit-Position: refs/heads/master@{#13565}
==========

to

==========
Style cleanups in RtpSender.

- Renamed variables and some function to comply with style guide.
- Removed default argument values.
- Removed some dead code.
- Cleaned up comments formatting in rtp_rtcp.h

R=danilchap@webrtc.org, stefan@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/ec4f068bcdbc044f14162fb53...
==========

[Sergey Ulanov, 2016-07-28 22:19:25]

Committed patchset #9 (id:200001) manually as
ec4f068bcdbc044f14162fb530b50dc45db01211 (presubmit successful).