Move RTP module activation into PayloadRouter.

Move RTP module activation into PayloadRouter.

Simplifies PayloadRouter to not accept dynamically-changing modules as
well as usage of PayloadRouter inside ViEChannel::SetSendCodec.

BUG=webrtc:5494
R=stefan@webrtc.org
TBR=mflodman@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/8b79b07a55a857370848724385f3d1f38c52213f

[pbos-webrtc, 2016-02-24 15:22:45]

Description was changed from

==========
Move RTP module registration into PayloadRouter.

Simplifies PayloadRouter to not accept dynamically-changing modules as
well as usage of PayloadRouter inside ViEChannel::SetSendCodec.

BUG=webrtc:5494
R=stefan@webrtc.org
==========

to

==========
Move RTP module activation into PayloadRouter.

Simplifies PayloadRouter to not accept dynamically-changing modules as
well as usage of PayloadRouter inside ViEChannel::SetSendCodec.

BUG=webrtc:5494
R=stefan@webrtc.org
==========

[pbos-webrtc, 2016-02-24 15:23:33]

PTAL, I think this makes PayloadRouter easier.

[pbos-webrtc, 2016-02-24 15:31:05]

PTAL

[stefan-webrtc, 2016-02-26 14:49:08]

https://codereview.webrtc.org/1725363003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc (right):

https://codereview.webrtc.org/1725363003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc:254: return
rtp_sender_.RegisterPayload(video_codec.plName, video_codec.plType,
Call RegisterVideoSendPayload()

https://codereview.webrtc.org/1725363003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/1725363003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:308:
static_cast<size_t>(RTP_PAYLOAD_NAME_SIZE));
Can you instead change the type of this const?

https://codereview.webrtc.org/1725363003/diff/1/webrtc/video/payload_router.cc
File webrtc/video/payload_router.cc (right):

https://codereview.webrtc.org/1725363003/diff/1/webrtc/video/payload_router.c...
webrtc/video/payload_router.cc:101: for (size_t i = 0; i < num_sending_modules_;
++i) {
Maybe loop over stream_bitrates rather modules? Should be OK to set bitrates on
inactive modules, right?

https://codereview.webrtc.org/1725363003/diff/1/webrtc/video/payload_router.h
File webrtc/video/payload_router.h (right):

https://codereview.webrtc.org/1725363003/diff/1/webrtc/video/payload_router.h...
webrtc/video/payload_router.h:77: // Active sending RTP modules, in layer order.
Maybe move this to before the vector?

https://codereview.webrtc.org/1725363003/diff/1/webrtc/video/vie_channel.cc
File webrtc/video/vie_channel.cc (right):

https://codereview.webrtc.org/1725363003/diff/1/webrtc/video/vie_channel.cc#n...
webrtc/video/vie_channel.cc:526: rtp_rtcp_modules_[0]->SetSendingStatus(true);
Shouldn't we rely on the payload router to do this?

[pbos-webrtc, 2016-02-26 14:58:55]

feedback

[pbos-webrtc, 2016-02-26 14:58:56]

PTAL

https://codereview.webrtc.org/1725363003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc (right):

https://codereview.webrtc.org/1725363003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc:254: return
rtp_sender_.RegisterPayload(video_codec.plName, video_codec.plType,
On 2016/02/26 14:49:07, stefan-webrtc (holmer) wrote:
> Call RegisterVideoSendPayload()

Other method is CHECKed, don't dare doing that right now.

https://codereview.webrtc.org/1725363003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/1725363003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:308:
static_cast<size_t>(RTP_PAYLOAD_NAME_SIZE));
On 2016/02/26 14:49:07, stefan-webrtc (holmer) wrote:
> Can you instead change the type of this const?

This is stored in RtpUtility::Payload which has a flat buffer of size
RTP_PAYLOAD_NAME_SIZE. :(

https://codereview.webrtc.org/1725363003/diff/1/webrtc/video/payload_router.cc
File webrtc/video/payload_router.cc (right):

https://codereview.webrtc.org/1725363003/diff/1/webrtc/video/payload_router.c...
webrtc/video/payload_router.cc:101: for (size_t i = 0; i < num_sending_modules_;
++i) {
On 2016/02/26 14:49:07, stefan-webrtc (holmer) wrote:
> Maybe loop over stream_bitrates rather modules? Should be OK to set bitrates
on
> inactive modules, right?

Done.

https://codereview.webrtc.org/1725363003/diff/1/webrtc/video/payload_router.h
File webrtc/video/payload_router.h (right):

https://codereview.webrtc.org/1725363003/diff/1/webrtc/video/payload_router.h...
webrtc/video/payload_router.h:77: // Active sending RTP modules, in layer order.
On 2016/02/26 14:49:07, stefan-webrtc (holmer) wrote:
> Maybe move this to before the vector?

Want them to be lumped with the crit_ that protects them.

https://codereview.webrtc.org/1725363003/diff/1/webrtc/video/vie_channel.cc
File webrtc/video/vie_channel.cc (right):

https://codereview.webrtc.org/1725363003/diff/1/webrtc/video/vie_channel.cc#n...
webrtc/video/vie_channel.cc:526: rtp_rtcp_modules_[0]->SetSendingStatus(true);
On 2016/02/26 14:49:07, stefan-webrtc (holmer) wrote:
> Shouldn't we rely on the payload router to do this?

Not on the receiver side. I'm removing this in a later CL though.

[pbos-webrtc, 2016-02-26 15:08:24]

DCHECK SetTargetSendBitrates size, then set all of them

[pbos-webrtc, 2016-02-26 15:15:04]

change RTP_PAYLOAD_SIZE to unsigned

[pbos-webrtc, 2016-02-26 15:19:08]

nuke comment

[stefan-webrtc, 2016-02-26 15:22:08]

lgtm

[pbos-webrtc, 2016-02-26 15:31:19]

Description was changed from

==========
Move RTP module activation into PayloadRouter.

Simplifies PayloadRouter to not accept dynamically-changing modules as
well as usage of PayloadRouter inside ViEChannel::SetSendCodec.

BUG=webrtc:5494
R=stefan@webrtc.org
==========

to

==========
Move RTP module activation into PayloadRouter.

Simplifies PayloadRouter to not accept dynamically-changing modules as
well as usage of PayloadRouter inside ViEChannel::SetSendCodec.

BUG=webrtc:5494
R=stefan@webrtc.org
TBR=mflodman@webrtc.org
==========

[pbos-webrtc, 2016-02-26 15:31:19]

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

[pbos-webrtc, 2016-02-26 15:31:29]

TBR=mflodman@ for common_types.h

[pbos-webrtc, 2016-02-26 15:31:47]

Description was changed from

==========
Move RTP module activation into PayloadRouter.

Simplifies PayloadRouter to not accept dynamically-changing modules as
well as usage of PayloadRouter inside ViEChannel::SetSendCodec.

BUG=webrtc:5494
R=stefan@webrtc.org
TBR=mflodman@webrtc.org
==========

to

==========
Move RTP module activation into PayloadRouter.

Simplifies PayloadRouter to not accept dynamically-changing modules as
well as usage of PayloadRouter inside ViEChannel::SetSendCodec.

BUG=webrtc:5494
R=stefan@webrtc.org
TBR=mflodman@webrtc.org

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

[commit-bot: I haz the power, 2016-02-26 15:31:49]

Description was changed from

==========
Move RTP module activation into PayloadRouter.

Simplifies PayloadRouter to not accept dynamically-changing modules as
well as usage of PayloadRouter inside ViEChannel::SetSendCodec.

BUG=webrtc:5494
R=stefan@webrtc.org
TBR=mflodman@webrtc.org

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

to

==========
Move RTP module activation into PayloadRouter.

Simplifies PayloadRouter to not accept dynamically-changing modules as
well as usage of PayloadRouter inside ViEChannel::SetSendCodec.

BUG=webrtc:5494
R=stefan@webrtc.org
TBR=mflodman@webrtc.org

Committed: https://crrev.com/8b79b07a55a857370848724385f3d1f38c52213f
Cr-Commit-Position: refs/heads/master@{#11787}
==========

[pbos-webrtc, 2016-02-26 15:31:49]

Committed patchset #5 (id:80001) manually as
8b79b07a55a857370848724385f3d1f38c52213f (presubmit successful).

[commit-bot: I haz the power, 2016-02-26 15:31:50]

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