From a4ef2ce29de0c68b869f8d66276bc5acba54cc79 Mon Sep 17 00:00:00 2001 From: "mflodman@webrtc.org" Date: Thu, 12 Feb 2015 09:54:18 +0000 Subject: [PATCH] Remove getting max payload length from default module. Moving functionality to get max payload length from default RTP module to the payload router. I'll make a follow up CL changing asserts to DCHECK in rtp_rtcp_impl.cc. BUG=769 TEST=New unittest and existing sender mtu test R=stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/36119004 Cr-Commit-Position: refs/heads/master@{#8345} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8345 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 27 ++------------- webrtc/video_engine/payload_router.cc | 21 ++++++++++-- webrtc/video_engine/payload_router.h | 6 ++++ .../video_engine/payload_router_unittest.cc | 33 +++++++++++++++++++ webrtc/video_engine/vie_encoder.cc | 17 +++++----- 5 files changed, 69 insertions(+), 35 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 3dcbac5cf..e81740dcd 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -555,31 +555,8 @@ uint16_t ModuleRtpRtcpImpl::MaxPayloadLength() const { } uint16_t ModuleRtpRtcpImpl::MaxDataPayloadLength() const { - // Assuming IP/UDP. - uint16_t min_data_payload_length = IP_PACKET_SIZE - 28; - - if (IsDefaultModule()) { - // For default we need to update all child modules too. - CriticalSectionScoped lock(critical_section_module_ptrs_.get()); - std::vector::const_iterator it = child_modules_.begin(); - while (it != child_modules_.end()) { - RtpRtcp* module = *it; - if (module) { - uint16_t data_payload_length = - module->MaxDataPayloadLength(); - if (data_payload_length < min_data_payload_length) { - min_data_payload_length = data_payload_length; - } - } - it++; - } - } - - uint16_t data_payload_length = rtp_sender_.MaxDataPayloadLength(); - if (data_payload_length < min_data_payload_length) { - min_data_payload_length = data_payload_length; - } - return min_data_payload_length; + assert(!IsDefaultModule()); + return rtp_sender_.MaxDataPayloadLength(); } int32_t ModuleRtpRtcpImpl::SetTransportOverhead( diff --git a/webrtc/video_engine/payload_router.cc b/webrtc/video_engine/payload_router.cc index f815d13a6..00c64ad4b 100644 --- a/webrtc/video_engine/payload_router.cc +++ b/webrtc/video_engine/payload_router.cc @@ -12,6 +12,7 @@ #include "webrtc/base/checks.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h" +#include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h" #include "webrtc/system_wrappers/interface/critical_section_wrapper.h" namespace webrtc { @@ -22,6 +23,11 @@ PayloadRouter::PayloadRouter() PayloadRouter::~PayloadRouter() {} +size_t PayloadRouter::DefaultMaxPayloadLength() { + const size_t kIpUdpSrtpLength = 44; + return IP_PACKET_SIZE - kIpUdpSrtpLength; +} + void PayloadRouter::SetSendingRtpModules( const std::list& rtp_modules) { CriticalSectionScoped cs(crit_.get()); @@ -47,7 +53,7 @@ bool PayloadRouter::RoutePayload(FrameType frame_type, uint32_t time_stamp, int64_t capture_time_ms, const uint8_t* payload_data, - size_t payload_size, + size_t payload_length, const RTPFragmentationHeader* fragmentation, const RTPVideoHeader* rtp_video_hdr) { CriticalSectionScoped cs(crit_.get()); @@ -62,7 +68,18 @@ bool PayloadRouter::RoutePayload(FrameType frame_type, stream_idx = rtp_video_hdr->simulcastIdx; return rtp_modules_[stream_idx]->SendOutgoingData( frame_type, payload_type, time_stamp, capture_time_ms, payload_data, - payload_size, fragmentation, rtp_video_hdr) == 0 ? true : false; + payload_length, fragmentation, rtp_video_hdr) == 0 ? true : false; +} + +size_t PayloadRouter::MaxPayloadLength() const { + size_t min_payload_length = DefaultMaxPayloadLength(); + CriticalSectionScoped cs(crit_.get()); + for (auto* rtp_module : rtp_modules_) { + size_t module_payload_length = rtp_module->MaxDataPayloadLength(); + if (module_payload_length < min_payload_length) + min_payload_length = module_payload_length; + } + return min_payload_length; } } // namespace webrtc diff --git a/webrtc/video_engine/payload_router.h b/webrtc/video_engine/payload_router.h index 3cc0bb0a6..baad38f5f 100644 --- a/webrtc/video_engine/payload_router.h +++ b/webrtc/video_engine/payload_router.h @@ -33,6 +33,8 @@ class PayloadRouter { PayloadRouter(); ~PayloadRouter(); + static size_t DefaultMaxPayloadLength(); + // Rtp modules are assumed to be sorted in simulcast index order. void SetSendingRtpModules(const std::list& rtp_modules); @@ -52,6 +54,10 @@ class PayloadRouter { const RTPFragmentationHeader* fragmentation, const RTPVideoHeader* rtp_video_hdr); + // Returns the maximum allowed data payload length, given the configured MTU + // and RTP headers. + size_t MaxPayloadLength() const; + private: scoped_ptr crit_; diff --git a/webrtc/video_engine/payload_router_unittest.cc b/webrtc/video_engine/payload_router_unittest.cc index b4d20bf4e..abab965ac 100644 --- a/webrtc/video_engine/payload_router_unittest.cc +++ b/webrtc/video_engine/payload_router_unittest.cc @@ -127,5 +127,38 @@ TEST_F(PayloadRouterTest, SendSimulcast) { &payload_2, 1, NULL, &rtp_hdr_2)); } +TEST_F(PayloadRouterTest, MaxPayloadLength) { + // Without any limitations from the modules, verify we get the max payload + // length for IP/UDP/SRTP with a MTU of 150 bytes. + const size_t kDefaultMaxLength = 1500 - 20 - 8 - 12 - 4; + EXPECT_EQ(kDefaultMaxLength, payload_router_->DefaultMaxPayloadLength()); + EXPECT_EQ(kDefaultMaxLength, payload_router_->MaxPayloadLength()); + + MockRtpRtcp rtp_1; + MockRtpRtcp rtp_2; + std::list modules; + modules.push_back(&rtp_1); + modules.push_back(&rtp_2); + payload_router_->SetSendingRtpModules(modules); + + // Modules return a higher length than the default value. + EXPECT_CALL(rtp_1, MaxDataPayloadLength()) + .Times(1) + .WillOnce(Return(kDefaultMaxLength + 10)); + EXPECT_CALL(rtp_2, MaxDataPayloadLength()) + .Times(1) + .WillOnce(Return(kDefaultMaxLength + 10)); + EXPECT_EQ(kDefaultMaxLength, payload_router_->MaxPayloadLength()); + + // The modules return a value lower than default. + const size_t kTestMinPayloadLength = 1001; + EXPECT_CALL(rtp_1, MaxDataPayloadLength()) + .Times(1) + .WillOnce(Return(kTestMinPayloadLength + 10)); + EXPECT_CALL(rtp_2, MaxDataPayloadLength()) + .Times(1) + .WillOnce(Return(kTestMinPayloadLength)); + EXPECT_EQ(kTestMinPayloadLength, payload_router_->MaxPayloadLength()); +} } // namespace webrtc diff --git a/webrtc/video_engine/vie_encoder.cc b/webrtc/video_engine/vie_encoder.cc index 3afe758f9..0dd7e919b 100644 --- a/webrtc/video_engine/vie_encoder.cc +++ b/webrtc/video_engine/vie_encoder.cc @@ -217,7 +217,7 @@ bool ViEEncoder::Init() { send_padding_ = video_codec.numberOfSimulcastStreams > 1; } if (vcm_.RegisterSendCodec(&video_codec, number_of_cores_, - default_rtp_rtcp_->MaxDataPayloadLength()) != 0) { + PayloadRouter::DefaultMaxPayloadLength()) != 0) { return false; } if (default_rtp_rtcp_->RegisterSendPayload(video_codec) != 0) { @@ -324,6 +324,7 @@ int32_t ViEEncoder::RegisterExternalEncoder(webrtc::VideoEncoder* encoder, } int32_t ViEEncoder::DeRegisterExternalEncoder(uint8_t pl_type) { + DCHECK(send_payload_router_ != NULL); webrtc::VideoCodec current_send_codec; if (vcm_.SendCodec(¤t_send_codec) == VCM_OK) { uint32_t current_bitrate_bps = 0; @@ -340,8 +341,6 @@ int32_t ViEEncoder::DeRegisterExternalEncoder(uint8_t pl_type) { // If the external encoder is the current send codec, use vcm internal // encoder. if (current_send_codec.plType == pl_type) { - uint16_t max_data_payload_length = - default_rtp_rtcp_->MaxDataPayloadLength(); { CriticalSectionScoped cs(data_cs_.get()); send_padding_ = current_send_codec.numberOfSimulcastStreams > 1; @@ -351,6 +350,7 @@ int32_t ViEEncoder::DeRegisterExternalEncoder(uint8_t pl_type) { // a hack to prevent the following code from crashing. This should be fixed // for realz. https://code.google.com/p/chromium/issues/detail?id=348222 current_send_codec.extra_options = NULL; + size_t max_data_payload_length = send_payload_router_->MaxPayloadLength(); if (vcm_.RegisterSendCodec(¤t_send_codec, number_of_cores_, max_data_payload_length) != VCM_OK) { LOG(LS_INFO) << "De-registered the currently used external encoder (" @@ -363,6 +363,7 @@ int32_t ViEEncoder::DeRegisterExternalEncoder(uint8_t pl_type) { } int32_t ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec) { + DCHECK(send_payload_router_ != NULL); // Setting target width and height for VPM. if (vpm_.SetTargetResolution(video_codec.width, video_codec.height, video_codec.maxFramerate) != VPM_OK) { @@ -379,13 +380,11 @@ int32_t ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec) { video_codec.numberOfSimulcastStreams); default_rtp_rtcp_->SetTargetSendBitrate(stream_bitrates); - uint16_t max_data_payload_length = - default_rtp_rtcp_->MaxDataPayloadLength(); - { CriticalSectionScoped cs(data_cs_.get()); send_padding_ = video_codec.numberOfSimulcastStreams > 1; } + size_t max_data_payload_length = send_payload_router_->MaxPayloadLength(); if (vcm_.RegisterSendCodec(&video_codec, number_of_cores_, max_data_payload_length) != VCM_OK) { return -1; @@ -660,6 +659,7 @@ int ViEEncoder::CodecTargetBitrate(uint32_t* bitrate) const { } int32_t ViEEncoder::UpdateProtectionMethod(bool enable_nack) { + DCHECK(send_payload_router_ != NULL); bool fec_enabled = false; uint8_t dummy_ptype_red = 0; uint8_t dummy_ptypeFEC = 0; @@ -693,7 +693,6 @@ int32_t ViEEncoder::UpdateProtectionMethod(bool enable_nack) { // The send codec must be registered to set correct MTU. webrtc::VideoCodec codec; if (vcm_.SendCodec(&codec) == 0) { - uint16_t max_pay_load = default_rtp_rtcp_->MaxDataPayloadLength(); uint32_t current_bitrate_bps = 0; if (vcm_.Bitrate(¤t_bitrate_bps) != 0) { LOG_F(LS_WARNING) << @@ -701,7 +700,9 @@ int32_t ViEEncoder::UpdateProtectionMethod(bool enable_nack) { } // Convert to start bitrate in kbps. codec.startBitrate = (current_bitrate_bps + 500) / 1000; - if (vcm_.RegisterSendCodec(&codec, number_of_cores_, max_pay_load) != 0) { + size_t max_payload_length = send_payload_router_->MaxPayloadLength(); + if (vcm_.RegisterSendCodec(&codec, number_of_cores_, + max_payload_length) != 0) { return -1; } }