Move, almost, all receive side references to RTP to RtpStreamReceiver.

Move, almost, all receive side references to RTP to RtpStreamReceiver.

There are still a few places in VideoReceiveStream where the RTP module
is explicitly used, e.g. setting up a/v sync, but it's a bigger task to
change and that will be done in a follow up instead of in this CL.

BUG=webrtc:5838
Committed: https://crrev.com/dc7d0d2ef037dc02d7380925bf4e2133507539aa
Cr-Commit-Position: refs/heads/master@{#12642}

[mflodman, 2016-05-04 12:55:16]

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

[mflodman, 2016-05-04 12:55:17]

Philip,
Please take a look. This CL mainly moves code from VideoReceiveStream to
RtpStreamReceiver.

[mflodman, 2016-05-04 13:00:58]

Description was changed from

==========
Move, almost, all receive side references to RTP to RtpStreamReceiver.

There are still a few places in VideoReceiveStream where the RTP module
is explicitly used, e.g. setting up a/v sync, but it's a ;arger work to
change and that will be done in a folllow up instead of in this CL.

BUG=webrtc:5838
==========

to

==========
Move, almost, all receive side references to RTP to RtpStreamReceiver.

There are still a few places in VideoReceiveStream where the RTP module
is explicitly used, e.g. setting up a/v sync, but it's a bigger task to
change and that will be done in a follow up instead of in this CL.

BUG=webrtc:5838
==========

[mflodman, 2016-05-06 08:49:10]

Philip,
One change after I found out about the new SendNack method, I also added a
comment there to not use it incorrectly.

Did you forget to publish your comments on Wednesday? It seemed like you
reviewed it, but I didn't get any comments.

[philipel, 2016-05-06 08:56:19]

Just a few comment, mostly about cleaning up other peoples code. I can see that
you have already fixed a few of my comments so they are no longer relevant.

https://codereview.webrtc.org/1947913002/diff/20001/webrtc/video/rtp_stream_r...
File webrtc/video/rtp_stream_receiver.cc (right):

https://codereview.webrtc.org/1947913002/diff/20001/webrtc/video/rtp_stream_r...
webrtc/video/rtp_stream_receiver.cc:162: if (config_.rtp.fec.ulpfec_payload_type
!= -1) {
Check both FEC and RED and warn if one is set but not the other. Remove DCHECKS.

Maybe use IsFecEnabled()?

https://codereview.webrtc.org/1947913002/diff/20001/webrtc/video/rtp_stream_r...
webrtc/video/rtp_stream_receiver.cc:165: VideoCodec codec;
VideoCodec codec = {}; and remove memset on line 166

https://codereview.webrtc.org/1947913002/diff/20001/webrtc/video/rtp_stream_r...
webrtc/video/rtp_stream_receiver.cc:172: if (config_.rtp.fec.red_payload_type !=
-1) {
Merge this block of code with the block above.

https://codereview.webrtc.org/1947913002/diff/20001/webrtc/video/rtp_stream_r...
webrtc/video/rtp_stream_receiver.cc:359:
rtp_rtcp_->SendNack(sequence_number_vector);
Is the SendNACK function used anymore? If not it would be nice if you could
remove the SendNACK function from rtp_rtcp.h since it is deprecated and we want
it removed.

[mflodman, 2016-05-06 09:15:06]

Philip,
Comments addressed, PTAL.

https://codereview.webrtc.org/1947913002/diff/20001/webrtc/video/rtp_stream_r...
File webrtc/video/rtp_stream_receiver.cc (right):

https://codereview.webrtc.org/1947913002/diff/20001/webrtc/video/rtp_stream_r...
webrtc/video/rtp_stream_receiver.cc:162: if (config_.rtp.fec.ulpfec_payload_type
!= -1) {
On 2016/05/06 08:56:19, philipel wrote:
> Check both FEC and RED and warn if one is set but not the other. Remove
DCHECKS.
> 
> Maybe use IsFecEnabled()?

Done.

https://codereview.webrtc.org/1947913002/diff/20001/webrtc/video/rtp_stream_r...
webrtc/video/rtp_stream_receiver.cc:165: VideoCodec codec;
On 2016/05/06 08:56:18, philipel wrote:
> VideoCodec codec = {}; and remove memset on line 166

Done.

https://codereview.webrtc.org/1947913002/diff/20001/webrtc/video/rtp_stream_r...
webrtc/video/rtp_stream_receiver.cc:172: if (config_.rtp.fec.red_payload_type !=
-1) {
On 2016/05/06 08:56:18, philipel wrote:
> Merge this block of code with the block above.

Done.

https://codereview.webrtc.org/1947913002/diff/20001/webrtc/video/rtp_stream_r...
webrtc/video/rtp_stream_receiver.cc:359:
rtp_rtcp_->SendNack(sequence_number_vector);
On 2016/05/06 08:56:18, philipel wrote:
> Is the SendNACK function used anymore? If not it would be nice if you could
> remove the SendNACK function from rtp_rtcp.h since it is deprecated and we
want
> it removed.

I just learnt the functionality is a bit different re timing, so I reverted back
to using the old SendNACK here instead.

[philipel, 2016-05-06 09:45:04]

lgtm

[mflodman, 2016-05-06 10:21:14]

The CQ bit was checked by mflodman@webrtc.org

[commit-bot: I haz the power, 2016-05-06 10:21:19]

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

[commit-bot: I haz the power, 2016-05-06 12:32:26]

Description was changed from

==========
Move, almost, all receive side references to RTP to RtpStreamReceiver.

There are still a few places in VideoReceiveStream where the RTP module
is explicitly used, e.g. setting up a/v sync, but it's a bigger task to
change and that will be done in a follow up instead of in this CL.

BUG=webrtc:5838
==========

to

==========
Move, almost, all receive side references to RTP to RtpStreamReceiver.

There are still a few places in VideoReceiveStream where the RTP module
is explicitly used, e.g. setting up a/v sync, but it's a bigger task to
change and that will be done in a follow up instead of in this CL.

BUG=webrtc:5838
==========

[commit-bot: I haz the power, 2016-05-06 12:32:27]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-06 12:32:37]

Description was changed from

==========
Move, almost, all receive side references to RTP to RtpStreamReceiver.

There are still a few places in VideoReceiveStream where the RTP module
is explicitly used, e.g. setting up a/v sync, but it's a bigger task to
change and that will be done in a follow up instead of in this CL.

BUG=webrtc:5838
==========

to

==========
Move, almost, all receive side references to RTP to RtpStreamReceiver.

There are still a few places in VideoReceiveStream where the RTP module
is explicitly used, e.g. setting up a/v sync, but it's a bigger task to
change and that will be done in a follow up instead of in this CL.

BUG=webrtc:5838

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

[commit-bot: I haz the power, 2016-05-06 12:32:38]

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