diff --git a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h index 243b24c8f..6bc7dc41c 100644 --- a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h @@ -607,8 +607,7 @@ class RtpRtcp : public Module { /* * Set the target send bitrate */ - virtual void SetTargetSendBitrate( - const std::vector& stream_bitrates) = 0; + virtual void SetTargetSendBitrate(uint32_t bitrate_bps) = 0; /* * Turn on/off generic FEC diff --git a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 6c8dff41f..aaf84259d 100644 --- a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -232,7 +232,7 @@ class MockRtpRtcp : public RtpRtcp { MOCK_METHOD1(SetAudioLevel, int32_t(const uint8_t level_dBov)); MOCK_METHOD1(SetTargetSendBitrate, - void(const std::vector& stream_bitrates)); + void(uint32_t bitrate_bps)); MOCK_METHOD3(SetGenericFECStatus, int32_t(const bool enable, const uint8_t payloadTypeRED, const uint8_t payloadTypeFEC)); MOCK_METHOD3(GenericFECStatus, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index a529d097f..60e1a2721 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -894,34 +894,9 @@ int32_t ModuleRtpRtcpImpl::SendREDPayloadType( return rtp_sender_.RED(&payload_type); } -void ModuleRtpRtcpImpl::SetTargetSendBitrate( - const std::vector& stream_bitrates) { - if (IsDefaultModule()) { - CriticalSectionScoped lock(critical_section_module_ptrs_.get()); - if (simulcast_) { - std::vector::iterator it = child_modules_.begin(); - for (size_t i = 0; - it != child_modules_.end() && i < stream_bitrates.size(); ++it) { - if ((*it)->SendingMedia()) { - RTPSender& rtp_sender = (*it)->rtp_sender_; - rtp_sender.SetTargetBitrate(stream_bitrates[i]); - ++i; - } - } - } else { - if (stream_bitrates.size() > 1) - return; - std::vector::iterator it = child_modules_.begin(); - for (; it != child_modules_.end(); ++it) { - RTPSender& rtp_sender = (*it)->rtp_sender_; - rtp_sender.SetTargetBitrate(stream_bitrates[0]); - } - } - } else { - if (stream_bitrates.size() > 1) - return; - rtp_sender_.SetTargetBitrate(stream_bitrates[0]); - } +void ModuleRtpRtcpImpl::SetTargetSendBitrate(uint32_t bitrate_bps) { + DCHECK(!IsDefaultModule()); + rtp_sender_.SetTargetBitrate(bitrate_bps); } int32_t ModuleRtpRtcpImpl::SetKeyFrameRequestMethod( diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 5d5b7f4ca..d0f8aedef 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -288,8 +288,7 @@ class ModuleRtpRtcpImpl : public RtpRtcp { // Send a request for a keyframe. virtual int32_t RequestKeyFrame() OVERRIDE; - virtual void SetTargetSendBitrate( - const std::vector& stream_bitrates) OVERRIDE; + virtual void SetTargetSendBitrate(uint32_t bitrate_bps) OVERRIDE; virtual int32_t SetGenericFECStatus(bool enable, uint8_t payload_type_red, diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 68c53f69e..94a202521 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -30,7 +30,6 @@ namespace webrtc { namespace { const uint32_t kSenderSsrc = 0x12345; const uint32_t kReceiverSsrc = 0x23456; -const uint32_t kSenderRtxSsrc = 0x32345; const int64_t kOneWayNetworkDelayMs = 100; const uint8_t kBaseLayerTid = 0; const uint8_t kHigherLayerTid = 1; @@ -521,253 +520,4 @@ TEST_F(RtpRtcpImplTest, UniqueNackRequests) { EXPECT_EQ(75, sender_.RtcpReceived().UniqueNackRequestsInPercent()); } -class RtpSendingTestTransport : public Transport { - public: - void ResetCounters() { bytes_received_.clear(); } - - virtual int SendPacket(int channel, - const void* data, - size_t length) OVERRIDE { - RTPHeader header; - scoped_ptr parser(RtpHeaderParser::Create()); - EXPECT_TRUE(parser->Parse(static_cast(data), - static_cast(length), - &header)); - bytes_received_[header.ssrc] += length; - ++packets_received_[header.ssrc]; - return static_cast(length); - } - - virtual int SendRTCPPacket(int channel, - const void* data, - size_t length) OVERRIDE { - return static_cast(length); - } - - int GetPacketsReceived(uint32_t ssrc) const { - std::map::const_iterator it = packets_received_.find(ssrc); - if (it == packets_received_.end()) - return 0; - return it->second; - } - - int GetBytesReceived(uint32_t ssrc) const { - std::map::const_iterator it = bytes_received_.find(ssrc); - if (it == bytes_received_.end()) - return 0; - return it->second; - } - - int GetTotalBytesReceived() const { - int sum = 0; - for (std::map::const_iterator it = bytes_received_.begin(); - it != bytes_received_.end(); - ++it) { - sum += it->second; - } - return sum; - } - - private: - std::map bytes_received_; - std::map packets_received_; -}; - -class RtpSendingTest : public ::testing::Test { - protected: - // Map from SSRC to number of received packets and bytes. - typedef std::map > PaddingMap; - - RtpSendingTest() { - // Send module. - RtpRtcp::Configuration config; - config.audio = false; - config.clock = Clock::GetRealTimeClock(); - config.outgoing_transport = &transport_; - config.receive_statistics = receive_statistics_.get(); - config.rtt_stats = &rtt_stats_; - config.paced_sender = &pacer_; - memset(&codec_, 0, sizeof(VideoCodec)); - codec_.plType = 100; - strncpy(codec_.plName, "VP8", 3); - codec_.numberOfSimulcastStreams = 3; - codec_.simulcastStream[0].width = 320; - codec_.simulcastStream[0].height = 180; - codec_.simulcastStream[0].maxBitrate = 300; - codec_.simulcastStream[1].width = 640; - codec_.simulcastStream[1].height = 360; - codec_.simulcastStream[1].maxBitrate = 600; - codec_.simulcastStream[2].width = 1280; - codec_.simulcastStream[2].height = 720; - codec_.simulcastStream[2].maxBitrate = 1200; - // We need numberOfSimulcastStreams + 1 RTP modules since we need one - // default module. - for (int i = 0; i < codec_.numberOfSimulcastStreams + 1; ++i) { - RtpRtcp* sender = RtpRtcp::CreateRtpRtcp(config); - EXPECT_EQ(0, sender->RegisterSendPayload(codec_)); - EXPECT_EQ(0, sender->SetSendingStatus(true)); - sender->SetSendingMediaStatus(true); - sender->SetSSRC(kSenderSsrc + i); - sender->SetRemoteSSRC(kReceiverSsrc + i); - senders_.push_back(sender); - config.default_module = senders_[0]; - } - std::vector bitrates; - bitrates.push_back(codec_.simulcastStream[0].maxBitrate); - bitrates.push_back(codec_.simulcastStream[1].maxBitrate); - bitrates.push_back(codec_.simulcastStream[2].maxBitrate); - senders_[0]->SetTargetSendBitrate(bitrates); - } - - ~RtpSendingTest() { - for (int i = senders_.size() - 1; i >= 0; --i) { - delete senders_[i]; - } - } - - void SendFrameOnSender(int sender_index, - const uint8_t* payload, - size_t length) { - RTPVideoHeader rtp_video_header = { - codec_.simulcastStream[sender_index].width, - codec_.simulcastStream[sender_index].height, - true, - 0, - kRtpVideoVp8, - {}}; - uint32_t seq_num = 0; - uint32_t ssrc = 0; - int64_t capture_time_ms = 0; - bool retransmission = false; - EXPECT_CALL(pacer_, SendPacket(_, _, _, _, _, _)) - .WillRepeatedly(DoAll(SaveArg<1>(&ssrc), - SaveArg<2>(&seq_num), - SaveArg<3>(&capture_time_ms), - SaveArg<5>(&retransmission), - Return(true))); - EXPECT_EQ(0, - senders_[sender_index]->SendOutgoingData(kVideoFrameKey, - codec_.plType, - 0, - 0, - payload, - length, - NULL, - &rtp_video_header)); - EXPECT_TRUE(senders_[sender_index]->TimeToSendPacket( - ssrc, seq_num, capture_time_ms, retransmission)); - } - - void ExpectPadding(const PaddingMap& expected_padding) { - int expected_total_bytes = 0; - for (PaddingMap::const_iterator it = expected_padding.begin(); - it != expected_padding.end(); - ++it) { - int packets_received = transport_.GetBytesReceived(it->first); - if (it->second.first > 0) { - EXPECT_GE(packets_received, it->second.first) - << "On SSRC: " << it->first; - } - int bytes_received = transport_.GetBytesReceived(it->first); - expected_total_bytes += bytes_received; - if (it->second.second > 0) { - EXPECT_GE(bytes_received, it->second.second) - << "On SSRC: " << it->first; - } else { - EXPECT_EQ(0, bytes_received) << "On SSRC: " << it->first; - } - } - EXPECT_EQ(expected_total_bytes, transport_.GetTotalBytesReceived()); - } - - scoped_ptr receive_statistics_; - RtcpRttStatsTestImpl rtt_stats_; - std::vector senders_; - RtpSendingTestTransport transport_; - NiceMock pacer_; - VideoCodec codec_; -}; - -TEST_F(RtpSendingTest, DISABLED_RoundRobinPadding) { - // We have to send on an SSRC to be allowed to pad, since a marker bit must - // be sent prior to padding packets. - const uint8_t payload[200] = {0}; - for (int i = 0; i < codec_.numberOfSimulcastStreams; ++i) { - SendFrameOnSender(i + 1, payload, sizeof(payload)); - } - transport_.ResetCounters(); - senders_[0]->TimeToSendPadding(500); - PaddingMap expected_padding; - expected_padding[kSenderSsrc + 1] = std::make_pair(2, 500); - expected_padding[kSenderSsrc + 2] = std::make_pair(0, 0); - expected_padding[kSenderSsrc + 3] = std::make_pair(0, 0); - ExpectPadding(expected_padding); - senders_[0]->TimeToSendPadding(1000); - expected_padding[kSenderSsrc + 2] = std::make_pair(4, 1000); - ExpectPadding(expected_padding); - senders_[0]->TimeToSendPadding(1500); - expected_padding[kSenderSsrc + 3] = std::make_pair(6, 1500); - ExpectPadding(expected_padding); -} - -TEST_F(RtpSendingTest, DISABLED_RoundRobinPaddingRtx) { - // Enable RTX to allow padding to be sent prior to media. - for (int i = 1; i < codec_.numberOfSimulcastStreams + 1; ++i) { - // Abs-send-time is needed to be allowed to send padding prior to media, - // as otherwise the timestmap used for BWE will be broken. - senders_[i]->RegisterSendRtpHeaderExtension(kRtpExtensionAbsoluteSendTime, - 1); - senders_[i]->SetRtxSendPayloadType(96); - senders_[i]->SetRtxSsrc(kSenderRtxSsrc + i); - senders_[i]->SetRtxSendStatus(kRtxRetransmitted); - } - transport_.ResetCounters(); - senders_[0]->TimeToSendPadding(500); - PaddingMap expected_padding; - expected_padding[kSenderSsrc + 1] = std::make_pair(0, 0); - expected_padding[kSenderSsrc + 2] = std::make_pair(0, 0); - expected_padding[kSenderSsrc + 3] = std::make_pair(0, 0); - expected_padding[kSenderRtxSsrc + 1] = std::make_pair(2, 500); - expected_padding[kSenderRtxSsrc + 2] = std::make_pair(0, 0); - expected_padding[kSenderRtxSsrc + 3] = std::make_pair(0, 0); - ExpectPadding(expected_padding); - senders_[0]->TimeToSendPadding(1000); - expected_padding[kSenderRtxSsrc + 2] = std::make_pair(4, 500); - ExpectPadding(expected_padding); - senders_[0]->TimeToSendPadding(1500); - - expected_padding[kSenderRtxSsrc + 3] = std::make_pair(6, 500); - ExpectPadding(expected_padding); -} - -TEST_F(RtpSendingTest, DISABLED_RoundRobinPaddingRtxRedundantPayloads) { - for (int i = 1; i < codec_.numberOfSimulcastStreams + 1; ++i) { - senders_[i]->SetRtxSendPayloadType(96); - senders_[i]->SetRtxSsrc(kSenderRtxSsrc + i); - senders_[i]->SetRtxSendStatus(kRtxRetransmitted | kRtxRedundantPayloads); - senders_[i]->SetStorePacketsStatus(true, 100); - } - // First send payloads so that we have something to retransmit. - const size_t kPayloadSize = 500; - const uint8_t payload[kPayloadSize] = {0}; - for (int i = 0; i < codec_.numberOfSimulcastStreams; ++i) { - SendFrameOnSender(i + 1, payload, sizeof(payload)); - } - transport_.ResetCounters(); - senders_[0]->TimeToSendPadding(500); - PaddingMap expected_padding; - expected_padding[kSenderSsrc + 1] = std::make_pair(0, 0); - expected_padding[kSenderSsrc + 2] = std::make_pair(0, 0); - expected_padding[kSenderSsrc + 3] = std::make_pair(0, 0); - expected_padding[kSenderRtxSsrc + 1] = std::make_pair(1, 500); - expected_padding[kSenderRtxSsrc + 2] = std::make_pair(0, 0); - expected_padding[kSenderRtxSsrc + 3] = std::make_pair(0, 0); - ExpectPadding(expected_padding); - senders_[0]->TimeToSendPadding(1000); - expected_padding[kSenderRtxSsrc + 2] = std::make_pair(2, 1000); - ExpectPadding(expected_padding); - senders_[0]->TimeToSendPadding(1500); - expected_padding[kSenderRtxSsrc + 3] = std::make_pair(3, 1500); - ExpectPadding(expected_padding); -} } // namespace webrtc diff --git a/webrtc/video_engine/payload_router.cc b/webrtc/video_engine/payload_router.cc index 15ef0cc4f..7d25f10c9 100644 --- a/webrtc/video_engine/payload_router.cc +++ b/webrtc/video_engine/payload_router.cc @@ -57,12 +57,15 @@ bool PayloadRouter::RoutePayload(FrameType frame_type, const RTPFragmentationHeader* fragmentation, const RTPVideoHeader* rtp_video_hdr) { CriticalSectionScoped cs(crit_.get()); - DCHECK(rtp_video_hdr == NULL || - rtp_video_hdr->simulcastIdx <= rtp_modules_.size()); - if (!active_ || rtp_modules_.empty()) return false; + // The simulcast index might actually be larger than the number of modules in + // case the encoder was processing a frame during a codec reconfig. + if (rtp_video_hdr != NULL && + rtp_video_hdr->simulcastIdx >= rtp_modules_.size()) + return false; + int stream_idx = 0; if (rtp_video_hdr != NULL) stream_idx = rtp_video_hdr->simulcastIdx; @@ -85,6 +88,19 @@ bool PayloadRouter::TimeToSendPacket(uint32_t ssrc, return true; } +void PayloadRouter::SetTargetSendBitrates( + const std::vector& stream_bitrates) { + CriticalSectionScoped cs(crit_.get()); + if (stream_bitrates.size() < rtp_modules_.size()) { + // There can be a size mis-match during codec reconfiguration. + return; + } + int idx = 0; + for (auto* rtp_module : rtp_modules_) { + rtp_module->SetTargetSendBitrate(stream_bitrates[idx++]); + } +} + size_t PayloadRouter::TimeToSendPadding(size_t bytes) { CriticalSectionScoped cs(crit_.get()); for(auto* rtp_module : rtp_modules_) { diff --git a/webrtc/video_engine/payload_router.h b/webrtc/video_engine/payload_router.h index ec9de993d..35c541d9e 100644 --- a/webrtc/video_engine/payload_router.h +++ b/webrtc/video_engine/payload_router.h @@ -65,6 +65,10 @@ class PayloadRouter { // sent. size_t TimeToSendPadding(size_t bytes); + // Configures current target bitrate per module. 'stream_bitrates' is assumed + // to be in the same order as 'SetSendingRtpModules'. + void SetTargetSendBitrates(const std::vector& stream_bitrates); + // Returns the maximum allowed data payload length, given the configured MTU // and RTP headers. size_t MaxPayloadLength() const; diff --git a/webrtc/video_engine/payload_router_unittest.cc b/webrtc/video_engine/payload_router_unittest.cc index ff4f9b37d..1a479d9bc 100644 --- a/webrtc/video_engine/payload_router_unittest.cc +++ b/webrtc/video_engine/payload_router_unittest.cc @@ -116,6 +116,7 @@ TEST_F(PayloadRouterTest, SendSimulcast) { EXPECT_TRUE(payload_router_->RoutePayload(frame_type_2, payload_type_2, 0, 0, &payload_2, 1, NULL, &rtp_hdr_2)); + // Inactive. payload_router_->set_active(false); EXPECT_CALL(rtp_1, SendOutgoingData(_, _, _, _, _, _, _, _)) .Times(0); @@ -125,6 +126,16 @@ TEST_F(PayloadRouterTest, SendSimulcast) { &payload_1, 1, NULL, &rtp_hdr_1)); EXPECT_FALSE(payload_router_->RoutePayload(frame_type_2, payload_type_2, 0, 0, &payload_2, 1, NULL, &rtp_hdr_2)); + + // Invalid simulcast index. + payload_router_->set_active(true); + EXPECT_CALL(rtp_1, SendOutgoingData(_, _, _, _, _, _, _, _)) + .Times(0); + EXPECT_CALL(rtp_2, SendOutgoingData(_, _, _, _, _, _, _, _)) + .Times(0); + rtp_hdr_1.simulcastIdx = 2; + EXPECT_FALSE(payload_router_->RoutePayload(frame_type_1, payload_type_1, 0, 0, + &payload_1, 1, NULL, &rtp_hdr_1)); } TEST_F(PayloadRouterTest, MaxPayloadLength) { @@ -257,7 +268,6 @@ TEST_F(PayloadRouterTest, TimeToSendPadding) { modules.push_back(&rtp_2); payload_router_->SetSendingRtpModules(modules); - // Default configuration, sending padding on the first sending module. const size_t requested_padding_bytes = 1000; const size_t sent_padding_bytes = 890; @@ -302,4 +312,39 @@ TEST_F(PayloadRouterTest, TimeToSendPadding) { EXPECT_EQ(static_cast(0), payload_router_->TimeToSendPadding(requested_padding_bytes)); } + +TEST_F(PayloadRouterTest, SetTargetSendBitrates) { + MockRtpRtcp rtp_1; + MockRtpRtcp rtp_2; + std::list modules; + modules.push_back(&rtp_1); + modules.push_back(&rtp_2); + payload_router_->SetSendingRtpModules(modules); + + const uint32_t bitrate_1 = 10000; + const uint32_t bitrate_2 = 76543; + std::vector bitrates (2, bitrate_1); + bitrates[1] = bitrate_2; + EXPECT_CALL(rtp_1, SetTargetSendBitrate(bitrate_1)) + .Times(1); + EXPECT_CALL(rtp_2, SetTargetSendBitrate(bitrate_2)) + .Times(1); + payload_router_->SetTargetSendBitrates(bitrates); + + bitrates.resize(1); + EXPECT_CALL(rtp_1, SetTargetSendBitrate(bitrate_1)) + .Times(0); + EXPECT_CALL(rtp_2, SetTargetSendBitrate(bitrate_2)) + .Times(0); + payload_router_->SetTargetSendBitrates(bitrates); + + bitrates.resize(3); + bitrates[1] = bitrate_2; + bitrates[2] = bitrate_1 + bitrate_2; + EXPECT_CALL(rtp_1, SetTargetSendBitrate(bitrate_1)) + .Times(1); + EXPECT_CALL(rtp_2, SetTargetSendBitrate(bitrate_2)) + .Times(1); + payload_router_->SetTargetSendBitrates(bitrates); + } } // namespace webrtc diff --git a/webrtc/video_engine/vie_encoder.cc b/webrtc/video_engine/vie_encoder.cc index cecb3a2a9..832415472 100644 --- a/webrtc/video_engine/vie_encoder.cc +++ b/webrtc/video_engine/vie_encoder.cc @@ -380,7 +380,7 @@ int32_t ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec) { video_codec.startBitrate * 1000, video_codec.simulcastStream, video_codec.numberOfSimulcastStreams); - default_rtp_rtcp_->SetTargetSendBitrate(stream_bitrates); + send_payload_router_->SetTargetSendBitrates(stream_bitrates); { CriticalSectionScoped cs(data_cs_.get()); @@ -872,6 +872,7 @@ void ViEEncoder::OnNetworkChanged(uint32_t bitrate_bps, LOG(LS_VERBOSE) << "OnNetworkChanged, bitrate" << bitrate_bps << " packet loss " << fraction_lost << " rtt " << round_trip_time_ms; + DCHECK(send_payload_router_ != NULL); vcm_.SetChannelParameters(bitrate_bps, fraction_lost, round_trip_time_ms); bool video_is_suspended = vcm_.VideoSuspended(); int bitrate_kbps = bitrate_bps / 1000; @@ -924,7 +925,7 @@ void ViEEncoder::OnNetworkChanged(uint32_t bitrate_bps, bitrate_kbps, PacedSender::kDefaultPaceMultiplier * bitrate_kbps, pad_up_to_bitrate_kbps); - default_rtp_rtcp_->SetTargetSendBitrate(stream_bitrates); + send_payload_router_->SetTargetSendBitrates(stream_bitrates); if (video_suspended_ == video_is_suspended) return; video_suspended_ = video_is_suspended;